[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