[ofa-general] [PATCH 1/3] libvendor: osm_vendor_get_all_port_attr() rework

[Hal Rosenstock]

On Tue, 2008-01-08 at 17:00 +0000, Sasha Khapyorsky wrote:
> On 08:22 Tue 08 Jan     , Hal Rosenstock wrote:
> > Sasha,
> > 
> > On Tue, 2008-01-08 at 16:20 +0000, Sasha Khapyorsky wrote:
> > > On 07:03 Mon 07 Jan     , Hal Rosenstock wrote:
> > > > Sasha,
> > > > 
> > > > On Thu, 2007-11-15 at 11:12 +0200, Sasha Khapyorsky wrote:
> > > > > It fixes couple of issues with this function:
> > > > > 
> > > > > - return only valid guids, don't return duplicated entries
> > > > 
> > > > What entries were duplicated ?
> > > 
> > > At index zero of the array.
> > 
> > That was the "best" port (and was intentionally duplicated).
> 
> Yes (not in general, but only for CAs) and in this sense it was "API"
> violation (following the name and the description of function
> osm_vendor_get_all_port_attr()), also seems another vendor layers never
> supported this (which was resulted in having four ugly
> #ifdef OSM_VENDOR_INTF_OPENIB in OpenSM port selection code).
> 
> > > > I think there may be a subtle "API" change in that the ib_port_attr_t
> > > > array filled in no longer has (or properly calculates the "best" port).
> > > > Not sure if it is this change or some other change which causes this.
> > > 
> > > I think it is a just a fix and API is untouched there.
> > 
> > Changing index 0 of the array is a subtle API change (and it affected
> > consumers which use this).
> 
> I checked an open source consumers available at OFA (ibutils, opensm) -
> nobody used this (OpenSM had '#ifdef OSM_VENDOR_INTF_OPENIB' just in
> order to ignore the index '0' of the array).

Yes, but it goes further than checking in tree consumers and not
everyone is paying attention all the time or running complete
regressions frequently so this wasn't found until recently.

This change broke autoselection on a machine running OpenSM with a
combination of IB and iWARP adapters as it selected an iWARP adapter and
exited.

If we care about continuing to support this feature, I suppose code
might be able to be added to OpenSM main.c to handle this rather than it
being in a lower layer as it was before.

-- Hal

> Sasha
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general