[ofa-general] Re: [PATCH 1/2] libibumad: fix partition support

[Sean Hefty]

> Just two things:
> 1. It might be better if the ABI version 5 warning message for only
> pkey_index 0 being supported comes out at umad_init time rather than
> umad_set_pkey time so that the user is not swamped with these.

Placing the warning in umad_init would display it even if the app only 
used pkey_index 0, so keeping it in umad_set_pkey seems better to me. 
We could make it so that the warning message only displays once though.

> 2. There is one pathological combination. It would be using 2.6.23 (with
> the new user_mad ABI version 6), an updated libibumad would be required,
> but an older libvendor (osm_vendor_ibumad.c without your one line
> change). That might be the case with someone who swapped back and forth
> between OFED 1.2 and master in some scenarios.

I don't know how we can support all combinations, especially since the 
return codes aren't being checked.  We can make a special case when 
umad_set_pkey() is called with 0xffff on ABI 6, and display a warning 
message and/or convert it to the correct index.

> Also, this does not quite work as expected. An error was returned based
> on the bad pkey index but I do see a send on the IB link (with a bad
> pkey). I wouldn't have expected the latter part. Maybe this is a driver
> or firmware issue. Not sure yet. I suppose there should be some
> pkey_index validation (to make sure it is within the device's valid
> range) and that should also ultimately get added to libibumad or should
> such validation go into the user_mad kernel module ?

I think if we want to validate that the pkey_index is reasonable, the 
check should go in the kernel.

- Sean