[ofw][patch] fixes mac generation for unknown guids (HP)

[Anatoly Greenblatt]

Hi,

That is not the solution for all vendors, but replacement for default
case where MAC was generated from static counter. I am sure that it is
better, because in original code two consequent calls to
ipoib_mac_from_guid produced different MAC. The new code brings some
consistency, but I agree this is not solution for all cases. We'll see
more ipoib_mac_from_xxx_guid functions.

Regards,
Anatoly.

-----Original Message-----
From: Fab Tillier [mailto:ftillier at windows.microsoft.com] 
Sent: Monday, June 09, 2008 19:44
To: Anatoly Greenblatt; ofw at lists.openfabrics.org
Subject: RE: [ofw][patch] fixes mac generation for unknown guids (HP)

Hi Anatoly,

Are you sure that your conversion is valid for all GUIDs in the market?
The algorithm needs to create unique MAC addresses that doesn't conflict
with the vendor's other products (some of which may be real Ethernet
devices).

Note that if this algorithm was applied to SilverStorm of Voltaire GUIDs
it would produce the wrong MACs, but it wouldn't be clear that these
have been generated with the 'default' algorithm.  At least the original
code generates MAC addresses that are clearly locally defined and thus
not guaranteed to be unique.

I think the original code is more correct - vendors have to 'opt-in' to
convert their GUIDs to valid and unique Ethernet MAC addresses.  I don't
think it's safe to assume that everyone out there does this in the
simple way of taking the upper 24 bits and lower 24 bits of the port
GUID.

That said, the ipoib_mac_from_supermicro_guid and
ipoib_mac_from_cisco_guid could be merged into a single function that
could then be applied to other cases that follow the same simple
algorithm.

-Fab

> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org [mailto:ofw-
> bounces at lists.openfabrics.org] On Behalf Of Anatoly Greenblatt
> Sent: Thursday, June 05, 2008 7:08 AM
> To: ofw at lists.openfabrics.org
> Subject: [ofw][patch] fixes mac generation for unknown guids (HP)
>
>  <<fix_for_mac_generation_for_unknown_guids.patch>> Ignore my last
> e-mail, this is the correct patch.
> Fixes mac address generation problem reported by Fab.
>
>
> Index: kernel/ipoib_driver.h
> ===================================================================
---
> kernel/ipoib_driver.h       (revision 1245) +++ kernel/ipoib_driver.h
>    (working copy) @@ -71,8 +71,6 @@
>         cl_qlist_t              adapter_list;
>         cl_qlist_t              bundle_list;
>
> -       atomic32_t              laa_idx;
> -
>         NDIS_HANDLE             h_ndis_wrapper;
>         NDIS_HANDLE             h_ibat_dev;
>         volatile LONG   ibat_ref;
> @@ -90,9 +88,6 @@
>  *      bundle_list
>  *              List of all adapter bundles.
>  *
> -*      laa_idx
> -*              Global counter for generating LAA MACs
> -*
>  *      h_ibat_dev
>  *              Device handle returned by NdisMRegisterDevice.
>  *********/
> Index: kernel/ipoib_xfr_mgr.h
> ===================================================================
---
> kernel/ipoib_xfr_mgr.h      (revision 1245) +++ kernel/ipoib_xfr_mgr.h
>    (working copy) @@ -372,7 +372,7 @@
>  {
>         ib_api_status_t status;
>         const uint8_t   *p_guid = (const uint8_t*)&port_guid;
> -       uint32_t                laa;
> +       uint32_t                low24;
>
>         if( p_guid[0] == 0 )
>         {
> @@ -409,18 +409,16 @@
>                 }
>
>         }
> -       /* Value of zero is reserved. */
> -       laa = cl_atomic_inc( &g_ipoib.laa_idx );
>
> -       if( !laa )
> -               return IB_INVALID_GUID;
> +       // create mac based on unknown vendor guid
> +       low24 = ((uint32_t)cl_ntoh64( port_guid ) & 0x00FFFFFF);
>
> -       p_mac_addr->addr[0] = 2; /* LAA bit */
> -       p_mac_addr->addr[1] = 0;
> -       p_mac_addr->addr[2] = (uint8_t)(laa >> 24);
> -       p_mac_addr->addr[3] = (uint8_t)(laa >> 16);
> -       p_mac_addr->addr[4] = (uint8_t)(laa >> 8);
> -       p_mac_addr->addr[5] = (uint8_t)laa;
> +       p_mac_addr->addr[0] = p_guid[0];
> +       p_mac_addr->addr[1] = p_guid[1];
> +       p_mac_addr->addr[2] = p_guid[2];
> +       p_mac_addr->addr[3] = (uint8_t)(low24 >> 16);
> +       p_mac_addr->addr[4] = (uint8_t)(low24 >> 8);
> +       p_mac_addr->addr[5] = (uint8_t)low24;
>
>         return IB_SUCCESS;
>  }