[openib-general] ISER cleanup

[Dan Bar Dov]

In today's ISER commits: 
Removed files: iser_ext_api.h iser_global.[ch] 
Removed all typedefs except function pointers typedefs
All files are now using tabs for indentation and lines are 80 long max.
Down to 6K LOC + 2K .h LOC

Dan

> -----Original Message-----
> From: openib-general-bounces at openib.org 
> [mailto:openib-general-bounces at openib.org] On Behalf Of Dan Bar Dov
> Sent: Tuesday, August 23, 2005 7:11 PM
> To: Grant Grundler
> Cc: openib-general at openib.org
> Subject: RE: [openib-general] ISER cleanup
> 
> 
> > -----Original Message-----
> > On Mon, Aug 22, 2005 at 01:22:55PM +0300, Dan Bar Dov wrote:
> > > We have begun a cleanup of ISER based on the inputs we received.
> > > Mostly cosmetic cleanups were already commited.
> > 
> > yup - good progress and some more cosmetic stuff noted below.
> > Then need to start looking at addressing Christoph's (hch) comments.
> 
> Some of the comments were taken care of in today's commits 
> iovecs removed procfs removed Function entry/exit traces 
> removed Unnecessary files removed:
> kernel_dep.h
> iser_bhs.h
> iser_trace.c
> 
> > 
> > Still need to remove kernel_dep.h and probably most of the files in 
> > iser/include/.
> > 
> > Those also all have a trailing "/* DAT 1.2 */" 
> > that might mislead in the future.
> > Maybe a comment in the header about "Based on DAT 1.2" release.
> 
> All DAT 1.2 comments removed. Actually the current code is 
> not DAT 1.2 compatible, but the openIB flavor compatible.
> Since work started on a CM abstraction, I expect ISER to get 
> off of kdapl and onto ib-verbs + CM abstraction.
> > 
> > 
> > iser_api.h
> > 	Should iSCSI be providiing the jump table definitions?
> > 	struct iser_api_t
> > 	struct iser_api_cb_t
> > 
> > iser_ext_api.h
> > 	typedef void * iser_conn_request_t;
> > 	Delete stuff like this - it just obscures what is going on.
> OK
> 
> > 
> > 	I'm not sure what this file is doing.
> > 	I was expecting iSCSI framework to define the data structures
> > 	it needs to talk to a service provider.
> This is an "extended API". The ISER spec defines an ISER API, 
> but it does not consider implementation. 
> We chose to implement the extra api out of the iser_api 
> structute and in the iser_ext_api struct. 
> iSCSI is still not part of the kernel so we had first 
> modified and added the datamover framework to linux-iscsi and 
> now to open-iscsi. Once open-iscsi is in the kernel we'll use 
> it as the framework.
> 
> > 
> > iser_pdu.h
> > 	sorry - Didn't have time to understand what this is about.
> Most definitions are duplicates from the iscsi and will disappear. 
> The struct iser_send_pdu defines the ISER extensions to the iscsi pdu.
> 
> > 
> > iser_types.h
> > 	delete typdef void * iser_api_handle_t.
> > 	replace usage of iser_api_handle_t with "void *".
> > 	Ditto for all "void *" typedefs in that file.
> OK
> 
> > 
> > 	Kernel already defines scatter-gather lists type.
> The iser_data_buf struct can point to a scatterlist array but 
> can also be used to point at a single buffer. 
> It does not replicate scatterlist but allows us to deal with 
> two types of registrations - single buffer and scatter lists.
> 
> > 
> > kernel_dep.h
> > 	Delete this file.
> > 	This content belongs in a seperate patch that people can grab
> > 	and apply when they want to build iSER on an older kernel.
> > 	See src/linux/kernel/patches
> Gone.
> 
> > 
> > 
> > > Removed vi comments
> > 
> > yup - mostly. Some are still present in iser/include/*.h.
> Gone as well.
> 
> > 
> > > Removed CONFIG_INFINIBAND refrences
> > > Reorganized module
> > > Rewritten Makefile to new style
> > > Added Kconfig file
> > > Using kernel min/max
> > 
> > all very good.
> > 
> > 
> > > There are many other things to be done, including both 
> coding style 
> > > and substance, we'll proceed addressing all the technical
> > issues that
> > > were commented on.
> > 
> > great!
> We are going to simplify the local memory registrations by 
> registering all memory like in the SRP driver.
> We do not understand some of the substance issues - for 
> example, dma related comments - are taken care of by iscsi, 
> not the transport. The io_mmu comment, we completely do not 
> understand - there was some platform specific code, but its 
> all gone now.
> 
> BTW the code is now down to 7K LOC + 2K LOC of heavily 
> commented header files.
> 
> > 
> > thanks,
> > grant
> _______________________________________________
> openib-general mailing list
> openib-general at openib.org
> http://openib.org/mailman/listinfo/openib-general
> 
> To unsubscribe, please visit 
> http://openib.org/mailman/listinfo/openib-general
>