[openib-general] Re: Incorrect endian in GUID comparison/SM master selection

[Brian Eng]

Hello again,

I found another place in OpenSM where it compares two SM's.  I suggest
the following both to fix it and to form a common comparison routine:

--- osm_sminfo_rcv.c	4 Mar 2005 23:21:53 -0000	1.2.2.4
+++ osm_sminfo_rcv.c	5 Mar 2005 00:45:03 -0000
@@ -156,30 +156,15 @@ osm_sminfo_rcv_init(
  By higher - we mean: SM with higher priority or with same priority
  and lower GUID.
 **********************************************************************/
-boolean_t
+inline boolean_t
 __osm_sminfo_rcv_remote_sm_is_higher (
   IN const osm_sminfo_rcv_t* p_rcv,
   IN const ib_sm_info_t*     p_remote_sm )
 {
-
-
-  if( ib_sminfo_get_priority( p_remote_sm ) >
-      p_rcv->p_subn->opt.sm_priority )
-  {
-    return( TRUE );
-  }
-  else
-  {
-    if( ib_sminfo_get_priority( p_remote_sm ) ==
-        p_rcv->p_subn->opt.sm_priority )
-    {
-      if( p_remote_sm->guid < p_rcv->p_subn->sm_port_guid )
-      {
-        return( TRUE );
-      }
-    }
-  }
-  return( FALSE );
+  return( osm_sm_is_greater_than( ib_sminfo_get_priority( p_remote_sm
),
+      p_remote_sm->guid, 
+      p_rcv->p_subn->opt.sm_priority, 
+      p_rcv->p_subn->sm_port_guid) );
 }
 
 /**********************************************************************
--- osm_state_mgr.c	4 Mar 2005 23:15:42 -0000	1.2.2.6
+++ osm_state_mgr.c	5 Mar 2005 00:45:03 -0000
@@ -1563,12 +1563,19 @@ __osm_state_mgr_get_highest_sm(
 {
   cl_qmap_t*               p_sm_tbl;
   osm_remote_sm_t*         p_sm = NULL;
-  osm_remote_sm_t*        p_highest_sm = NULL;
+  osm_remote_sm_t*         p_highest_sm;
+  uint8_t                  highest_sm_priority;
+  ib_net64_t               highest_sm_guid;
 
   OSM_LOG_ENTER( p_mgr->p_log, __osm_state_mgr_get_highest_sm );
 
   p_sm_tbl = &p_mgr->p_subn->sm_guid_tbl;
 
+  /* Start with the local sm as the standard */
+  p_highest_sm          = NULL;
+  highest_sm_priority   = p_mgr->p_subn->opt.sm_priority;
+  highest_sm_guid       = p_mgr->p_subn->sm_port_guid;
+
   /*  go over all the remote SMs */
   for(   p_sm = (osm_remote_sm_t*)cl_qmap_head( p_sm_tbl );
          p_sm != (osm_remote_sm_t*)cl_qmap_end( p_sm_tbl );
@@ -1579,55 +1586,19 @@ __osm_state_mgr_get_highest_sm(
     if (ib_sminfo_get_state(&p_sm->smi) == IB_SMINFO_STATE_NOTACTIVE )
       continue;
 
-    if ( p_highest_sm == NULL)
+    if ( osm_sm_is_greater_than( ib_sminfo_get_priority(&p_sm->smi),
+         p_sm->smi.guid, highest_sm_priority, highest_sm_guid ) )
     {
+      /*  the new p_sm is with higher priority - update the highest_sm
*/
+      /*  to this sm */
       p_highest_sm = p_sm;
-    }
-    else
-    {
-      if ( ib_sminfo_get_priority(&p_sm->smi) >
-           ib_sminfo_get_priority(&p_highest_sm->smi) )
-      {
-        /*  the new p_sm is with higher priority - update the
highest_sm */
-        /*  to this sm */
-        p_highest_sm = p_sm;
-      }
-      else
-      {
-        if ( ib_sminfo_get_priority(&p_sm->smi) ==
-             ib_sminfo_get_priority(&p_highest_sm->smi) )
-        {
-          /*  both SMs are with same priority - compare GUIDs */
-          if ( p_sm->smi.guid < p_highest_sm->smi.guid )
-          {
-            /*  the new p_sm is with same priority but lower GUID - */
-            /*  update the highest sm to this sm */
-            p_highest_sm = p_sm;
-          }
-        }
-      }
+      highest_sm_priority = ib_sminfo_get_priority(&p_sm->smi);
+      highest_sm_guid = p_sm->smi.guid;
     }
   }
-  /*  compare the p_highest_sm to the local sm */
+
   if ( p_highest_sm != NULL )
   {
-    /*  check if this SM is higher then us */
-    if (  p_mgr->p_subn->opt.sm_priority >
-          ib_sminfo_get_priority(&p_highest_sm->smi) )
-    {
-      /*  the local SM has higher priority */
-      return( NULL );
-    }
-    else
-    {
-      if(   ib_sminfo_get_priority(&p_highest_sm->smi) ==
-            p_mgr->p_subn->opt.sm_priority &&
-            p_highest_sm->smi.guid > p_mgr->p_subn->sm_port_guid )
-      {
-        /*  they have same priority. Local SM has lower GUID */
-        return( NULL );
-      }
-    }
     osm_log( p_mgr->p_log, OSM_LOG_DEBUG,
              "__osm_state_mgr_get_highest_sm: "
              "Found higher SM with guid: %016" PRIx64 "\n",
--- osm_state_mgr.h	23 Sep 2004 18:43:08 -0000	1.2
+++ osm_state_mgr.h	5 Mar 2005 00:45:03 -0000
@@ -495,6 +495,62 @@ osm_state_mgr_process(
 * SEE ALSO
 *	State Manager
 *********/
+/****f* OpenSM: State Manager/osm_sm_is_greater_than
+* NAME
+*	osm_sm_is_greater_than
+*
+* DESCRIPTION
+*  Compares two SM's (14.4.1.2)
+*
+* SYNOPSIS
+*/
+static inline boolean_t
+osm_sm_is_greater_than (
+  IN const uint8_t    l_priority,
+  IN const ib_net64_t l_guid,
+  IN const uint8_t    r_priority,
+  IN const ib_net64_t r_guid )
+{
+  if( l_priority > r_priority )
+  {
+    return( TRUE );
+  }
+  else
+  {
+    if( l_priority == r_priority )
+    {
+      if( cl_ntoh64(l_guid) <  cl_ntoh64(r_guid) )
+      {
+        return( TRUE );
+      }
+    }
+  }
+  return( FALSE );
+}
+/*
+* PARAMETERS
+*	l_priority
+*		[in] Priority of the SM on the "left"
+*
+*	l_guid
+*		[in] GUID of the SM on the "left"
+*
+*	r_priority
+*		[in] Priority of the SM on the "right"
+*
+*	r_guid
+*		[in] GUID of the SM on the "right"
+*
+* RETURN VALUES
+*  Return TRUE if an sm with l_priority and l_guid is higher than an sm
+*  with r_priority and r_guid,
+*  return FALSE otherwise.
+*
+* NOTES
+*
+* SEE ALSO
+*	State Manager
+*********/