[PATCH v2] opensm: bug in trap report for MC create(66) and delete(67) traps

["Eli Dorfman (Voltaire)" <dorfman.eli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Subject: [PATCH] Wrong handling of MC create and delete traps

For these traps the GID in the data details is the MGID and
not the source port gid.
So the SM should check that subscriber port has the pkey of the MC group.
There was also an error in comparing the subnet prefix and guid due to
host/network order mismatch.

Signed-off-by: Eli Dorfman <elid-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/opensm/osm_inform.c |  151 ++++++++++++++++++++++++++++---------------
 1 files changed, 98 insertions(+), 53 deletions(-)

diff --git a/opensm/opensm/osm_inform.c b/opensm/opensm/osm_inform.c
index 8108213..ae4fe71 100644
--- a/opensm/opensm/osm_inform.c
+++ b/opensm/opensm/osm_inform.c
@@ -341,6 +341,103 @@ Exit:
 	return status;
 }
 
+static int is_access_permitted( osm_infr_t *p_infr_rec,
+				osm_infr_match_ctxt_t *p_infr_match )
+{
+	cl_list_t *p_infr_to_remove_list = p_infr_match->p_remove_infr_list;
+	ib_inform_info_t *p_ii = &(p_infr_rec->inform_record.inform_info);
+	ib_mad_notice_attr_t *p_ntc = p_infr_match->p_ntc;
+	uint16_t trap_num = cl_ntoh16(p_ntc->g_or_v.generic.trap_num);
+	osm_subn_t *p_subn = p_infr_rec->sa->p_subn;
+	osm_log_t *p_log = p_infr_rec->sa->p_log;
+	char gid_str[INET6_ADDRSTRLEN];
+	osm_mgrp_t *p_mgrp;
+	ib_gid_t source_gid;
+	osm_port_t *p_src_port;
+	osm_port_t *p_dest_port;
+
+	/* In case of GID_IN(64) or GID_OUT(65) traps the source gid
+	   comparison should be done on the trap source (saved as the gid in the
+	   data details field).
+	   For traps MC_CREATE(66) or MC_DELETE(67) the data details gid is
+	   the MGID. We need to check whether subscriber has the pky of 
+	   the MC group.
+	   In all other cases the issuer gis is the trap source.
+	*/
+	if (trap_num >= 64 && trap_num <= 67 )
+		/* The issuer of these traps is the SM so source_gid 
+		   is the gid saved on the data details */
+		source_gid = p_ntc->data_details.ntc_64_67.gid;
+	else
+		source_gid = p_ntc->issuer_gid;
+
+	p_dest_port =
+	    cl_ptr_vector_get(&p_subn->port_lid_tbl,
+			      cl_ntoh16(p_infr_rec->report_addr.dest_lid));
+	if (!p_dest_port) {
+		OSM_LOG(p_log, OSM_LOG_INFO,
+			"Cannot find destination port with LID:%u\n",
+			cl_ntoh16(p_infr_rec->report_addr.dest_lid));
+		goto Exit;
+	}
+
+	switch (trap_num) {
+		case 66:
+		case 67:
+			p_mgrp = osm_get_mgrp_by_mgid(p_subn, &source_gid);
+			if (!p_mgrp) {
+				OSM_LOG(p_log, OSM_LOG_INFO,
+					"Cannot find MGID %s\n",
+					inet_ntop(AF_INET6, source_gid.raw, gid_str, sizeof gid_str));
+				goto Exit;
+			}
+			
+			if (!osm_physp_has_pkey(p_log, 
+						p_mgrp->mcmember_rec.pkey, 
+						p_dest_port->p_physp)) {
+				OSM_LOG(p_log, OSM_LOG_INFO,
+					"MGID %s and port GUID:0x%016" PRIx64 " do not share same pkey\n",
+					inet_ntop(AF_INET6, source_gid.raw, gid_str, sizeof gid_str),
+					cl_ntoh64(p_dest_port->guid));
+				goto Exit;
+			}
+			break;
+
+		default:
+			p_src_port =
+			    osm_get_port_by_guid(p_subn, source_gid.unicast.interface_id);
+			if (!p_src_port) {
+				OSM_LOG(p_log, OSM_LOG_INFO,
+					"Cannot find source port with GUID:0x%016" PRIx64 "\n",
+					cl_ntoh64(source_gid.unicast.interface_id));
+				goto Exit;
+			}
+
+
+			/* Check if there is a pkey match. o13-17.1.1 */
+			if (osm_port_share_pkey(p_log, p_src_port, p_dest_port) == FALSE) {
+				OSM_LOG(p_log, OSM_LOG_DEBUG, "Mismatch by Pkey\n");
+				/* According to o13-17.1.2 - If this informInfo does not have
+				   lid_range_begin of 0xFFFF, then this informInfo request
+				   should be removed from database */
+				if (p_ii->lid_range_begin != 0xFFFF) {
+					OSM_LOG(p_log, OSM_LOG_VERBOSE,
+						"Pkey mismatch on lid_range_begin != 0xFFFF. "
+						"Need to remove this informInfo from db\n");
+					/* add the informInfo record to the remove_infr list */
+					cl_list_insert_tail(p_infr_to_remove_list, p_infr_rec);
+				}
+				goto Exit;
+			}
+			break;
+	}
+
+	return 1;
+Exit:
+	return 0;	
+}
+
+
 /**********************************************************************
  * This routine compares a given Notice and a ListItem of InformInfo type.
  * PREREQUISITE:
@@ -351,15 +448,10 @@ static void match_notice_to_inf_rec(IN cl_list_item_t * p_list_item,
 {
 	osm_infr_match_ctxt_t *p_infr_match = (osm_infr_match_ctxt_t *) context;
 	ib_mad_notice_attr_t *p_ntc = p_infr_match->p_ntc;
-	cl_list_t *p_infr_to_remove_list = p_infr_match->p_remove_infr_list;
 	osm_infr_t *p_infr_rec = (osm_infr_t *) p_list_item;
 	ib_inform_info_t *p_ii = &(p_infr_rec->inform_record.inform_info);
 	cl_status_t status = CL_NOT_FOUND;
 	osm_log_t *p_log = p_infr_rec->sa->p_log;
-	osm_subn_t *p_subn = p_infr_rec->sa->p_subn;
-	ib_gid_t source_gid;
-	osm_port_t *p_src_port;
-	osm_port_t *p_dest_port;
 
 	OSM_LOG_ENTER(p_log);
 
@@ -460,55 +552,8 @@ static void match_notice_to_inf_rec(IN cl_list_item_t * p_list_item,
 		}
 	}
 
-	/* Check if there is a pkey match. o13-17.1.1 */
-	/* Check if the issuer of the trap is the SM. If it is, then the gid
-	   comparison should be done on the trap source (saved as the gid in the
-	   data details field).
-	   If the issuer gid is not the SM - then it is the guid of the trap
-	   source */
-	if ((cl_ntoh64(p_ntc->issuer_gid.unicast.prefix) ==
-	     p_subn->opt.subnet_prefix)
-	    && (cl_ntoh64(p_ntc->issuer_gid.unicast.interface_id) ==
-		p_subn->sm_port_guid))
-		/* The issuer is the SM then this is trap 64-67 - compare the gid
-		   with the gid saved on the data details */
-		source_gid = p_ntc->data_details.ntc_64_67.gid;
-	else
-		source_gid = p_ntc->issuer_gid;
-
-	p_src_port =
-	    osm_get_port_by_guid(p_subn, source_gid.unicast.interface_id);
-	if (!p_src_port) {
-		OSM_LOG(p_log, OSM_LOG_INFO,
-			"Cannot find source port with GUID:0x%016" PRIx64 "\n",
-			cl_ntoh64(source_gid.unicast.interface_id));
+	if (!is_access_permitted(p_infr_rec, p_infr_match))
 		goto Exit;
-	}
-
-	p_dest_port =
-	    cl_ptr_vector_get(&p_subn->port_lid_tbl,
-			      cl_ntoh16(p_infr_rec->report_addr.dest_lid));
-	if (!p_dest_port) {
-		OSM_LOG(p_log, OSM_LOG_INFO,
-			"Cannot find destination port with LID:%u\n",
-			cl_ntoh16(p_infr_rec->report_addr.dest_lid));
-		goto Exit;
-	}
-
-	if (osm_port_share_pkey(p_log, p_src_port, p_dest_port) == FALSE) {
-		OSM_LOG(p_log, OSM_LOG_DEBUG, "Mismatch by Pkey\n");
-		/* According to o13-17.1.2 - If this informInfo does not have
-		   lid_range_begin of 0xFFFF, then this informInfo request
-		   should be removed from database */
-		if (p_ii->lid_range_begin != 0xFFFF) {
-			OSM_LOG(p_log, OSM_LOG_VERBOSE,
-				"Pkey mismatch on lid_range_begin != 0xFFFF. "
-				"Need to remove this informInfo from db\n");
-			/* add the informInfo record to the remove_infr list */
-			cl_list_insert_tail(p_infr_to_remove_list, p_infr_rec);
-		}
-		goto Exit;
-	}
 
 	/* send the report to the address provided in the inform record */
 	OSM_LOG(p_log, OSM_LOG_DEBUG, "MATCH! Sending Report...\n");
-- 
1.5.5

There is still a problem with GID OUT trap that is not sent since source port
was already removed.
Patch will follow.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html