[ofw] port_attr structure

Tue Feb 20 01:01:25 PST 2007

 

________________________________

	From: Jan Bottorff [mailto:jbottorff at xsigo.com] 
	Sent: Tuesday, February 20, 2007 9:58 AM
	To: Yossi Leybovich; ofw at lists.openfabrics.org
	Subject: RE: [ofw] port_attr structure
	
	
	Hi,

	 
	I'd like to suggest that we try to have STABLE API interfaces to
the IB stack. What I mean by stable is that IB clients will continue to
work with new revisions of the IB stack. Currently, there are a number
of structures that are used as parameters to IB functions. If you add a
field to those structures, the size changes in a new build. This means
clients (kernel mode clients of the IBAL interface are what I work with)
are tied to exact revisions of the IB stack. Even worse, there are no
version or size attributes in many structures, and I offhand don't know
of a way for a client to even query the IB stack for a build revision.
This means if I don't compile my IB clients with the exact build that is
present at runtime, I may be corrupting memory, and not know it. 
	[Yossi Leybovich] I agree with the need of stable API, but as
the WinIB stack is under heavy development we encounter the need to add
missing function and fields to structures.

	For example the IB spec changed the mad packets lately to spec
1.2 and we wanted to update our stack .Or the FMR we wanted for storage
clients

	I will be happy to follow your guidelines to make the changes
more compatible with IBAL clients

	 
	Some simple areas that would help preserve revision
compatibility would be:

	 
	1)       Have a function to allocate memory in non-paged pool or
query the size of assorted structures. This would allow clients to
allocate structure parameters without having compiled in sizeof values. 
	[Yossi Leybovich] That is done in query_ca_attr and help us in
the past to add vendor specific values, and you right we should add this
to the other queries. 

	 
	2)       Have a size or version field in structures that are
passed as parameters. This would allow both the IB stack and clients to
at least know if there was a version skew problem, instead of corrupting
memory. When adding new fields, put them at the END. If fields become
orphaned, don't' take them out, just rename them to orphanedField1 or
something. LOTS of the Microsoft API's use this strategy to preserver
upward compatibility. In an ideal world, I'd actually not let clients
have direct access to fields; I'd have an object oriented function
interface to get and set values. There might (or might not) be cases
where performance is so critical, using setter/getters degrades
performance, and in those specific cases alternatives might be
appropriate (like a query of the field offsets, which a client can cache
and use to directly access the fields). This could be turned into an
inline function that queries the field offsets not yet cached, and then
does the pointer math and the value assignment. From a source code
viewpoint, it could look like SetConnectAttributes(ibalHandle,
connectStructurePtr, value1, value2, value3). Trying to architect things
based on an ideal, and then finding solutions to special cases when the
ideal has a tradoff is often a good strategy.
	[Yossi Leybovich] In most of the mad structure we use
getter/setter this was done because the IB spec was evolving and many
field where changed/added to the structures.

	        But we don't have setter/getter to most of the verb
structures.

	        IBAL does not have version on each structure but on the
whole interface.

	        We should change  the version of the interface each time
we change/add function or change one of the structure in the interface
files (i.e ib_types.h ....)

	 
	3)       Don't add new error codes in the MIDDLE of the
enumeration, as it causes previous values to be reassigned and breaks
binary compatibility. If you want to group things, then explicitly
assign values in a group in the enumeration definition. I'd actually
prefer a single error code namespace. Right now I translate all the
overlapping IB error enumerations into a single unique custom NTSTATUS.
I frequently DO want to return error details up the call chain, but
don't want to return multiple error types in parallel (which is what I
would have to do to tell the different between say a remote reject and a
NTSTATUS value for allocation failure in a connect function if I don't
map IB errors into a single NTSTATUS namespace). 

	 
	4)       Please be careful about what #includes are required by
clients. For example, some of the API's (I believe inline ones) use
complib functions that are built as part of the IB stack. At the moment,
the IB stack will not compile under the 32-bit checked WDK 6000/6001
compiler because the compiler seems unable to cope with a function
pointer in a structure with a __ptr64 attribute. We would like to be
using the WDK, but basically can't, because our kernel clients MUST be
linked with a matching build of the IB stack (i.e. an IB stack compiled
with the WDK), and we can't mix versions because some of the API's need
complib functions.
	[Yossi Leybovich] Complib is historical library from the time
IBAL was targeted to Linux and Windows. The IB stack use complib all
over the code (spinlocks,events...)

	       So I don't see how we can prevent from including complib 

	        We use to link complib dynamically but we move it to
link statically into ibbus.sys and ibal.dll 

	 
	5)       You might want to consider updating the IBAL
QUERY_INTERFACE version number if ANY structure or error value changes
in an incompatible way. At least then, clients can detect a version
problem, instead of the system crashing when memory is corrupted.
Generally in the past, I believe this version number tended to only get
updated if the interface function pointer array was updated. Changing a
parameter structure doesn't change the source definition of the
interface, but very much can change the binary definition.
	[Yossi Leybovich] Agree , I will do that. 

	 
	I'm not trying to stifle ongoing development and improvement in
the IB stack. I am trying to discuss ways the IB stack can evolve that
will cause fewer problems for those of us using the IB stack for
commercial products. Decoupling the IB stack build version from IB
client build version would help a lot. This would take a clear
definition of the binary API, and a little bit of ongoing developer
discipline.
	[Yossi Leybovich] Pls note that we are on the process of getting
WHQL on the WInIB stack and I hope that after we will get the signature
the changes will be less frequently 

	 
	Jan Bottorff

	Windows Systems Architect

	Xsigo Systems

	 
________________________________


	From: ofw-bounces at lists.openfabrics.org
[mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Yossi Leybovich
	Sent: Sunday, February 18, 2007 4:52 AM
	To: Fab Tillier
	Cc: ofw at lists.openfabrics.org
	Subject: [ofw] port_attr structure

	 
	Hi

	 
	I want to add more info to the port_attr structure (like link
width enable, link width enable).

	I notice that the port_attr include only specific fields from
the port info structure of the IB spec.

	Is there any reason why the port_attr structure in IBAL is
different from the IB spec port info (or at least include all its
fields)

	 
	Yossi 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20070220/b088e494/attachment.html>