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

Yevgeny Kliteynik kliteyn at dev.mellanox.co.il
Sun Nov 5 23:00:39 PST 2006


Hi Hal

[From Vu Pham]
  1. sending InformInfo set subscribe for trap 64,65,144 - this works;
     however, osm.log outputs wrong value for "subscribe" field
  2. sending InformInfo set *unsubscribe* for
     trap 64,65,144 - I'm using/formating the same mad as (1) except the
     "subscribe" field is zero; however, opensm response with status 0x200
[/From Vu Pham]

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

Yevgeny

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

Index: opensm/osm_sa_informinfo.c
===================================================================
--- opensm/osm_sa_informinfo.c	(revision 10064)
+++ 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)
@@ -480,6 +471,8 @@ osm_infr_rcv_process_set_method(
         goto Exit;
       }
 
+      /* set the subscribe bit to 0 before adding the record */
+      p_infr->inform_record.inform_info.subscribe = 0;
       /* Add this new osm_infr_t object to subnet object */
       osm_infr_insert_to_db( p_rcv->p_subn, p_rcv->p_log, p_infr );
 
@@ -488,6 +481,8 @@ osm_infr_rcv_process_set_method(
     {
       /* Update the old instance of the osm_infr_t object */
       p_infr->inform_record = inform_info_rec.inform_record;
+      /* set the subscribe bit to 0 after updating the record */
+      p_infr->inform_record.inform_info.subscribe = 0;
     }
   }
   else
Index: opensm/osm_inform.c
===================================================================
--- opensm/osm_inform.c	(revision 10064)
+++ opensm/osm_inform.c	(working copy)
@@ -206,30 +206,133 @@ __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" );
-  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 ((count1 == 0) && (count2 == 0))
-    status = CL_SUCCESS;
+  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
-    status = CL_NOT_FOUND;
+  {
+     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;
+     }
+  }
+
+  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 ( 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" );
+  }
+  else 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" );
+        goto Exit;
+     }
+     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
+  {
+     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;
 }





More information about the general mailing list