[ofw] [patch] [IPoIB] Adding support to dell guid and user-define mask for guid generation

Fab Tillier ftillier at windows.microsoft.com
Fri Sep 19 08:58:11 PDT 2008


>This patch adds support for dell guid and also enables user-defined MAC
>address generation according to predefined GUID bitwise mask
>IPoIB GUID patch

Ideally these two would have been independent patches...

>Signed-off by: Alexander Naslednikov (xalex at mellanox.co.il)
>Index: ulp/ipoib/kernel/ipoib_adapter.h
>===================================================================
>--- ulp/ipoib/kernel/ipoib_adapter.h (revision 3193)
>+++ ulp/ipoib/kernel/ipoib_adapter.h (working copy)
>@@ -77,6 +77,7 @@
>  uint32_t xfer_block_size;
>  mac_addr_t conf_mac;
>  uint32_t mc_leave_rescan;
>+ uint64_t guid_mask;
> } ipoib_params_t;
> /*
> * FIELDS
>Index: ulp/ipoib/kernel/ipoib_driver.c
>===================================================================
>--- ulp/ipoib/kernel/ipoib_driver.c (revision 3193)
>+++ ulp/ipoib/kernel/ipoib_driver.c (working copy)
>@@ -128,7 +128,10 @@
>
> #define IB_INFINITE_SERVICE_LEASE 0xFFFFFFFF
>
>+//The mask is 8 bit and can't contain more than 6 non-zero bits

This comment does not match the definition of guid_mask in the ipoib_params_t structure (above).  Why not more than 6 non-zero bits?

>+#define MAX_GUID_MAX 0xFC
>
>+
> /* Global driver debug level */
> uint32_t  g_ipoib_dbg_level = TRACE_LEVEL_ERROR;
> uint32_t  g_ipoib_dbg_flags = 0x00000fff;
>Index: ulp/ipoib/kernel/ipoib_port.c
>===================================================================
>--- ulp/ipoib/kernel/ipoib_port.c (revision 3193)
>+++ ulp/ipoib/kernel/ipoib_port.c (working copy)
>@@ -1819,9 +1819,9 @@
>   {
>    status = ipoib_mac_from_guid(
> #if IPOIB_INLINE_RECV
>-    p_desc->buf.ib.grh.src_gid.unicast.interface_id, &mac );
>+    p_desc->buf.ib.grh.src_gid.unicast.interface_id,
>p_port->p_adapter->params.guid_mask, &mac );
> #else /* IPOIB_INLINE_RECV */
>-    p_desc->p_buf->ib.grh.src_gid.unicast.interface_id, &mac );
>+    p_desc->p_buf->ib.grh.src_gid.unicast.interface_id,
>p_port->p_adapter->params.guid_mask,&mac );

Need a space before '&mac'.

> #endif /* IPOIB_INLINE_RECV */
>    if( status != IB_SUCCESS )
>    {
>@@ -2315,7 +2315,7 @@
>   cl_memcpy( &gid, &p_cid[7], sizeof(ib_gid_t) );
>   p_cid[1] =  HW_ADDR_LEN +1;// CID length
>   p_cid[2] =  DHCP_HW_TYPE_ETH;// CID type
>-  status = ipoib_mac_from_guid( gid.unicast.interface_id,
>(mac_addr_t*)&p_cid[3] );
>+  status = ipoib_mac_from_guid( gid.unicast.interface_id,
>p_port->p_adapter->params.guid_mask,(mac_addr_t*)&p_cid[3] );

Again, space after the comma, like the rest of the code.

>   p_cid[HW_ADDR_LEN + 3] = DHCP_OPT_END; //terminate tag
>  }
>  IPOIB_EXIT( IPOIB_DBG_RECV );
>@@ -2425,7 +2425,7 @@
>  {
>   /* Copy the src GID to allow aligned access */
>   cl_memcpy( &gid, &p_ib_arp->src_hw.gid, sizeof(ib_gid_t) );
>-  status = ipoib_mac_from_guid( gid.unicast.interface_id, &mac );
>+  status = ipoib_mac_from_guid( gid.unicast.interface_id,
>p_port->p_adapter->params.guid_mask,&mac );

And again...

>   if( status != IB_SUCCESS )
>   {
>    IPOIB_PRINT_EXIT( TRACE_LEVEL_ERROR, IPOIB_DBG_ERROR,
>Index: ulp/ipoib/kernel/ipoib_xfr_mgr.h
>===================================================================
>--- ulp/ipoib/kernel/ipoib_xfr_mgr.h (revision 3193)
>+++ ulp/ipoib/kernel/ipoib_xfr_mgr.h (working copy)
>@@ -231,6 +231,77 @@
>
>  return IB_SUCCESS;
> }
>+
>+/****f* IPOIB/ipoib_mac_from_dell_guid
>+* NAME
>+* ipoib_mac_from_dell_guid
>+*
>+* DESCRIPTION
>+* Generates an ethernet MAC address given a DELL port GUID.
>+*
>+* SYNOPSIS
>+*/
>+static inline ib_api_status_t
>+ipoib_mac_from_dell_guid(
>+ IN  const net64_t      port_guid,
>+  OUT   mac_addr_t* const   p_mac_addr )
>+{
>+ const uint8_t *p_guid = (const uint8_t*)&port_guid;
>+ uint32_t  low24;
>+ net16_t   guid_middle;
>+
>+ /* Port guid is in network byte order.  OUI is in lower 3 bytes. */
>+ ASSERT( p_guid[0] == 0x00 && p_guid[1] == 0x18 && p_guid[2] == 0x8b );
>+
>+ low24 = ((uint32_t)cl_ntoh64( port_guid ) & 0x00FFFFFF);
>+
>+ 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);

Why not use p_guid[6]?

>+ p_mac_addr->addr[4] = (uint8_t)(low24 >> 8);

Why not use p_guid[7]?

>+ p_mac_addr->addr[5] = (uint8_t)low24;

Why not use p_guid[8]?

>+
>+ return IB_SUCCESS;
>+}
>+
>+/****f* IPOIB/ipoib_mac_from_general_guid
>+* NAME
>+* ipoib_mac_from_general_guid
>+*
>+* DESCRIPTION
>+* Generates an ethernet MAC address given a general port GUID and a
>bitwise mask
>+*
>+* SYNOPSIS
>+*/
>+static inline ib_api_status_t
>+ipoib_mac_from_general_guid(
>+ IN  const net64_t      port_guid,
>+ IN     uint8_t      guid_mask,
>+  OUT   mac_addr_t* const   p_mac_addr )
>+{
>+#define MAC_ADDR_SIZE 6
>+ uint8_t i;
>+ const uint8_t *p_guid = (const uint8_t*)&port_guid;
>+ int dig_counter = 0;
>+
>+ //All non-zero bits of guid_mask indicates the number of an
>appropriate byte in
>+ // port_guid, that will be used in MAC address construction
>+ for (i = 7; guid_mask; guid_mask >>= 1, --i) {
>+  if (guid_mask & 1 ) {
>+   p_mac_addr->addr[MAC_ADDR_SIZE - dig_counter] = p_guid [i];
>+   ++dig_counter;
>+  }
>+ }
>+  // check for the mask validity: it can't have more than 6 non-zero
>bits
>+  if (dig_counter != MAC_ADDR_SIZE) {
>+   return IB_INVALID_GUID_MASK;
>+  }
>+
>+ return IB_SUCCESS;
>+}
>+
>+
> /*
> * PARAMETERS
> * port_guid
>@@ -399,12 +470,22 @@
> static inline ib_api_status_t
> ipoib_mac_from_guid(
>  IN  const net64_t      port_guid,
>-  OUT   mac_addr_t* const   p_mac_addr )
>+ IN  const  uint32_t     guid_mask,
>+  OUT   mac_addr_t* const   p_mac_addr
>+  )
> {
>  ib_api_status_t status;
>  const uint8_t *p_guid = (const uint8_t*)&port_guid;
>  uint32_t  laa;
>
>+ if (guid_mask) {

Curly brace use should follow style of existing code (as should whitespace, hard to tell if the whitespace issues are just from mailers garbling the patch).

>+  status = ipoib_mac_from_general_guid(port_guid, guid_mask,
>p_mac_addr);
>+  if( status == IB_SUCCESS )
>+    return IB_SUCCESS;
>+  //otherwise, mask was invalid, getting back to standard flow
>+ }
>+
>+
>  if( p_guid[0] == 0 )
>  {
>   if( p_guid[1] == 0x02 && p_guid[2] == 0xc9 )
>@@ -444,6 +525,13 @@
>    if( status == IB_SUCCESS )
>     return IB_SUCCESS;
>   }
>+  else if( p_guid[1] == 0x18 && p_guid[2] == 0x8b )

Why not check the first byte here?  The function below will assert that it's zero.

>+  {
>+   status = ipoib_mac_from_dell_guid( port_guid, p_mac_addr );
>+   if( status == IB_SUCCESS )
>+    return IB_SUCCESS;
>+  }
>+
>  }
>
>  /* Value of zero is reserved. */
>Index: ulp/opensm/user/include/iba/ib_types.h
>===================================================================
>--- ulp/opensm/user/include/iba/ib_types.h (revision 3193)
>+++ ulp/opensm/user/include/iba/ib_types.h (working copy)
>@@ -7928,6 +7928,7 @@
>  IB_INVALID_GID,
>  IB_INVALID_LID,
>  IB_INVALID_GUID,
>+ IB_INVALID_GUID_MASK,

I wouldn't add a new status value - the code never actually checks for this value, only checks for != IB_SUCCESS and then falls through the normal MAC generation path.

>  IB_INVALID_CA_HANDLE,
>  IB_INVALID_AV_HANDLE,
>  IB_INVALID_CQ_HANDLE,
>Index: ulp/opensm/user/include/iba/ib_types_extended.h
>===================================================================
>--- ulp/opensm/user/include/iba/ib_types_extended.h (revision 3193)
>+++ ulp/opensm/user/include/iba/ib_types_extended.h (working copy)
>@@ -137,6 +137,7 @@
>  IB_INVALID_GID,
>  IB_INVALID_LID,
>  IB_INVALID_GUID,
>+ IB_INVALID_GUID_MASK,
>  IB_INVALID_CA_HANDLE,
>  IB_INVALID_AV_HANDLE,
>  IB_INVALID_CQ_HANDLE,
>



More information about the ofw mailing list