[ofw] RE: [PATCH 1/4] Clean up MAC generation, add support for HP GUIDs

[Fab Tillier]

> - Eliminate use of IB_INVALID_GUID_MASK since no code checks for that actual
> value, and use IB_INVALID_GUID instead.

Ok, I was wrong, there's code that checks for this value.  That code is broken though...

> -               //otherwise, mask was invalid, getting back to standard flow
> -               /*if (status == IB_INVALID_GUID_MASK)
> -               {
> -                       IPOIB_PRINT( TRACE_LEVEL_WARNING, IPOIB_DBG_ERROR,
> -                       ("Invalid GUID mask received, rejecting it") );
> -               }*/

Ok, here's where we get in trouble...  The code falls through to see if the GUID has a known conversion, and eventually hits:

> -               if (status == IB_SUCCESS )
> +               if( status == IB_SUCCESS )
>                 {
> -                       ASSERT ( mask_status == IB_SUCCESS || mask_status ==
> IB_INVALID_GUID_MASK );
> -                       return mask_status;
> +                       ASSERT ( mask_status == IB_SUCCESS || mask_status ==
> IB_INVALID_GUID );
> +                       return mask_status;
>                 }

At this point, the mask_status could be IB_INVALID_GUID_MASK (in the current unpatched code).  If one of the vendor-specific conversion functions returned IB_SUCCESS the code still returns IB_INVALID_GUID_MASK, which is wrong.  In fact, even if the GUID has no known conversion function, the code will fall through to the locally administered MAC address generation, and then return IB_INVALID_GUID_MASK anyway...

So this comes down to whether the user-provided GUID mask should take precedent on the GUID conversion functions or not.  In the current code it does, which means that if a GUID mask is provided, LAA generation should be disabled (as should known GUID conversion attempts).

I'll update the 4th patch (that I haven't sent yet) to fix this up with the following logic - if the GUID mask parameter is provided, then no attempts to generate the GUID in other ways is attempted.

-Fab