[openib-general] [PATCH v3] osm: comparing InformInfo records

Yevgeny Kliteynik kliteyn at dev.mellanox.co.il
Wed Nov 22 23:08:37 PST 2006


Hi Hal

Third attempt :)

1. The received InformInfo struct was modified before dumping it.
2. The function that compares InformInfo structures was just 
   comparing the whole memory allocated for it, including reserved 
   fields. Fixed to compare more selectively.

As for QPN, from the IB spec, table 119 InformInfo:

    QPN : Ignored except when subscribe=0 (an unsubscribe 
    request). Queue pair to which Report()s were sent as 
    a result of a corresponding subscription. If no 
    subscription for this Report() with this QPN exists, 
    the request to unsubscribe performs no action and 
    produces GetResp() with status indicating an invalid 
    field value.


--
Yevgeny

Signed-off-by:  Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>

Index: opensm/osm_inform.c
===================================================================
--- opensm/osm_inform.c	(revision 10156)
+++ opensm/osm_inform.c	(working copy)
@@ -206,32 +206,122 @@ __match_inf_rec(
   osm_infr_t* p_infr_rec = (osm_infr_t *)context;
   osm_infr_t* p_infr = (osm_infr_t*)p_list_item;
   osm_log_t *p_log =  p_infr_rec->p_infr_rcv->p_log;
-  cl_status_t status;
-  int32_t count1, count2;
+  cl_status_t status = CL_NOT_FOUND;
+  ib_gid_t all_zero_gid;
 
   OSM_LOG_ENTER( p_log, __match_inf_rec);
 
-  count1 = memcmp(&p_infr->report_addr, &p_infr_rec->report_addr,
-		  sizeof(p_infr_rec->report_addr));
-  if (count1) 
-    osm_log( p_log, OSM_LOG_DEBUG,
-	     "__match_inf_rec: "
-	     "Differ by Address\n" );
+  if ( !memcmp(&p_infr->report_addr, 
+               &p_infr_rec->report_addr,
+               sizeof(p_infr_rec->report_addr)) )
+  {
+     osm_log( p_log, OSM_LOG_DEBUG,
+              "__match_inf_rec: "
+              "Differ by Address\n" );
+     goto Exit;
+  }
+
+  memset(&all_zero_gid, 0, sizeof(ib_gid_t));
+
+  /* if inform_info.gid is not zero, ignoring lid range */
+  if ( !memcmp(&p_infr_rec->inform_record.inform_info.gid,
+               &all_zero_gid, 
+               sizeof(p_infr_rec->inform_record.inform_info.gid)) )
+  {
+     if ( !memcmp(&p_infr->inform_record.inform_info.gid,
+                  &p_infr_rec->inform_record.inform_info.gid,
+                  sizeof(p_infr->inform_record.inform_info.gid)) )
+     {
+        osm_log( p_log, OSM_LOG_DEBUG,
+                 "__match_inf_rec: "
+                 "Differ by InformInfo.gid\n" );
+        goto Exit;
+     }
+  }
+  else
+  {
+     if ( (p_infr->inform_record.inform_info.lid_range_begin !=
+           p_infr_rec->inform_record.inform_info.lid_range_begin) ||
+          (p_infr->inform_record.inform_info.lid_range_end !=
+           p_infr_rec->inform_record.inform_info.lid_range_end) )
+     {
+        osm_log( p_log, OSM_LOG_DEBUG,
+                 "__match_inf_rec: "
+                 "Differ by InformInfo.LIDRange\n" );
+        goto Exit;
+     }
+  }
 
-  count2 = memcmp(
-    &p_infr->inform_record.inform_info,
-    &p_infr_rec->inform_record.inform_info,
-    sizeof(p_infr->inform_record.inform_info) );
-  if (count2) 
-    osm_log( p_log, OSM_LOG_DEBUG,
-	     "__match_inf_rec: "
-	     "Differ by InformInfo\n" ); 
+  if ( p_infr->inform_record.inform_info.trap_type !=
+       p_infr_rec->inform_record.inform_info.trap_type )
+  {
+     osm_log( p_log, OSM_LOG_DEBUG,
+              "__match_inf_rec: "
+              "Differ by InformInfo.TrapType\n" );
+     goto Exit;
+  }
+
+  if ( p_infr->inform_record.inform_info.is_generic != 
+       p_infr_rec->inform_record.inform_info.is_generic )
+  {     
+     osm_log( p_log, OSM_LOG_DEBUG,
+              "__match_inf_rec: "
+              "Differ by InformInfo.IsGeneric\n" );
+     goto Exit;
+  }
 
-  if ((count1 == 0) && (count2 == 0))
-    status = CL_SUCCESS;
+  if (p_infr->inform_record.inform_info.is_generic) 
+  {
+     if ( p_infr->inform_record.inform_info.g_or_v.generic.trap_num != 
+          p_infr_rec->inform_record.inform_info.g_or_v.generic.trap_num )
+        osm_log( p_log, OSM_LOG_DEBUG,
+                 "__match_inf_rec: "
+                 "Differ by InformInfo.Generic.TrapNumber\n" );
+     else if ( p_infr->inform_record.inform_info.g_or_v.generic.qpn_resp_time_val != 
+               p_infr_rec->inform_record.inform_info.g_or_v.generic.qpn_resp_time_val )
+        osm_log( p_log, OSM_LOG_DEBUG,
+                 "__match_inf_rec: "
+                 "Differ by InformInfo.Generic.QPNRespTimeVal\n" );
+     else if ( p_infr->inform_record.inform_info.g_or_v.generic.node_type_msb != 
+               p_infr_rec->inform_record.inform_info.g_or_v.generic.node_type_msb )
+        osm_log( p_log, OSM_LOG_DEBUG,
+                 "__match_inf_rec: "
+                 "Differ by InformInfo.Generic.NodeTypeMSB\n" );
+     else if ( p_infr->inform_record.inform_info.g_or_v.generic.node_type_lsb != 
+               p_infr_rec->inform_record.inform_info.g_or_v.generic.node_type_lsb )
+        osm_log( p_log, OSM_LOG_DEBUG,
+                 "__match_inf_rec: "
+                 "Differ by InformInfo.Generic.NodeTypeLSB\n" );
+     else
+           status = CL_SUCCESS;
+  }
   else
-    status = CL_NOT_FOUND;
+  {
+     if ( p_infr->inform_record.inform_info.g_or_v.vend.dev_id != 
+          p_infr_rec->inform_record.inform_info.g_or_v.vend.dev_id )
+        osm_log( p_log, OSM_LOG_DEBUG,
+                 "__match_inf_rec: "
+                 "Differ by InformInfo.Vendor.DeviceID\n" );
+     else if ( p_infr->inform_record.inform_info.g_or_v.vend.qpn_resp_time_val != 
+               p_infr_rec->inform_record.inform_info.g_or_v.vend.qpn_resp_time_val )
+        osm_log( p_log, OSM_LOG_DEBUG,
+                 "__match_inf_rec: "
+                 "Differ by InformInfo.Vendor.QPNRespTimeVal\n" );
+     else if ( p_infr->inform_record.inform_info.g_or_v.vend.vendor_id_msb != 
+               p_infr_rec->inform_record.inform_info.g_or_v.vend.vendor_id_msb )
+        osm_log( p_log, OSM_LOG_DEBUG,
+                 "__match_inf_rec: "
+                 "Differ by InformInfo.Vendor.VendorIdMSB\n" );
+     else if ( p_infr->inform_record.inform_info.g_or_v.vend.vendor_id_lsb !=
+               p_infr_rec->inform_record.inform_info.g_or_v.vend.vendor_id_lsb )
+        osm_log( p_log, OSM_LOG_DEBUG,
+                 "__match_inf_rec: "
+                 "Differ by InformInfo.Vendor.VendorIdLSB\n" );
+     else
+        status = CL_SUCCESS;
+  }
 
+ Exit:
   OSM_LOG_EXIT( p_log );
   return status;
 }
Index: opensm/osm_sa_informinfo.c
===================================================================
--- opensm/osm_sa_informinfo.c	(revision 10156)
+++ opensm/osm_sa_informinfo.c	(working copy)
@@ -345,7 +345,6 @@ osm_infr_rcv_process_set_method(
   ib_inform_info_t *p_recvd_inform_info;
   osm_infr_t inform_info_rec; /* actual inform record to be stored for reports */
   osm_infr_t *p_infr;
-  uint8_t subscribe;
   ib_net32_t qpn;
   uint8_t resp_time_val;
   ib_api_status_t res;
@@ -403,19 +402,11 @@ osm_infr_rcv_process_set_method(
    *
    * QPN:
    * internally we keep the QPN field of the InformInfo updated
-   * so we can simply compare the entire record - when finding such.
-   * IBA spec only requires the QPN field to be filled when an unsubscribe
-   * Set(InformInfo) is done. See table 119 p 740 QPN field
-   *
-   * SUBSCRIBE:
-   * For similar reasons we change the subscribe to 0 on the
-   * inserted/searched data
+   * so we can simply compare it in the record - when finding such.
    */
 
-  subscribe = p_recvd_inform_info->subscribe;
-  if (subscribe)
+  if (p_recvd_inform_info->subscribe)
   {
-    inform_info_rec.inform_record.inform_info.subscribe = 0;
     ib_inform_info_set_qpn(
       &inform_info_rec.inform_record.inform_info,
       inform_info_rec.report_addr.addr_type.gsi.remote_qp );
@@ -443,7 +434,7 @@ osm_infr_rcv_process_set_method(
   p_infr = osm_infr_get_by_rec( p_rcv->p_subn, p_rcv->p_log, &inform_info_rec );
 
   /* check to see if the request was for subscribe = 1 */
-  if (subscribe)
+  if (p_recvd_inform_info->subscribe)
   {
     /* validate the request for a new or update InformInfo */
     if (__validate_infr( p_rcv, &inform_info_rec ) != TRUE)





More information about the general mailing list