[openib-general] ISER cleanup

Grant Grundler iod00d at hp.com
Mon Aug 22 22:18:38 PDT 2005


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.

> These include:
> C++ style comments changed to C
> Removed DAT 1.2 comments
> Reomoved DAT 1.2 API support file
> Removed all platform dependencies

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.


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.

	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.

iser_pdu.h
	sorry - Didn't have time to understand what this is about.

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.

	Kernel already defines scatter-gather lists type.

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


> Removed vi comments

yup - mostly. Some are still present in iser/include/*.h.

> 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!

thanks,
grant




More information about the general mailing list