[openib-general] Another MAD processing question

Ralph Campbell ralphc at pathscale.com
Fri Jan 13 10:32:54 PST 2006


After looking at the MAD handling code some more, I am puzzled by
the following.

smi_check_local_dr_smp() is called only from two places in core/mad.c
It returns 0 or 1.  In smi_check_local_dr_smp(), it checks for
a directed route SMP but this function is only called when the SMP
is a directed route so this is a NOP.  The following patch could be
applied.

Index: agent.c
===================================================================
--- agent.c	(revision 4978)
+++ agent.c	(working copy)
@@ -81,9 +81,6 @@
 {
 	struct ib_agent_port_private *port_priv;
 
-	if (smp->mgmt_class != IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE)
-		return 1;
-
 	port_priv = ib_get_agent_port(device, port_num);
 	if (!port_priv) {
 		printk(KERN_DEBUG SPFX "smi_check_local_dr_smp %s port %d "

The call to ib_get_agent_port() shouldn't be possible to fail when
smi_check_local_dr_smp() is called from ib_mad_recv_done_handler().
When it is called from handle_outgoing_dr_smp(), the device and port_num
come from mad_agent_priv so I assume the call to ib_get_agent_port()
shouldn't fail either.  In either case, smi_check_local_smp()
only uses the mad_agent pointer to check that mad_agent->device->process_mad
is not NULL.  The device pointer would have to be the same as the
one passed to smi_check_local_dr_smp() since that pointer is used later
instead of the one checked in smi_check_local_smp().

All of this leads me to think that the following patch should work:

Index: smi.h
===================================================================
--- smi.h	(revision 4978)
+++ smi.h	(working copy)
@@ -49,19 +49,16 @@
 extern int smi_handle_dr_smp_send(struct ib_smp *smp,
 				  u8 node_type,
 				  int port_num);
-extern int smi_check_local_dr_smp(struct ib_smp *smp,
-				  struct ib_device *device,
-				  int port_num);
 
 /*
  * Return 1 if the SMP should be handled by the local SMA/SM via process_mad
  */
-static inline int smi_check_local_smp(struct ib_mad_agent *mad_agent,
-                         	      struct ib_smp *smp)
+static inline int smi_check_local_smp(struct ib_smp *smp,
+				      struct ib_device *device)
 {
 	/* C14-9:3 -- We're at the end of the DR segment of path */
 	/* C14-9:4 -- Hop Pointer = Hop Count + 1 -> give to SMA/SM */
-	return ((mad_agent->device->process_mad &&
+	return ((device->process_mad &&
 		!ib_get_smp_direction(smp) &&
 		(smp->hop_ptr == smp->hop_cnt + 1)));
 }
Index: agent.c
===================================================================
--- agent.c	(revision 4978)
+++ agent.c	(working copy)
@@ -75,25 +75,6 @@
 	return entry;
 }
 
-int smi_check_local_dr_smp(struct ib_smp *smp,
-			   struct ib_device *device,
-			   int port_num)
-{
-	struct ib_agent_port_private *port_priv;
-
-	if (smp->mgmt_class != IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE)
-		return 1;
-
-	port_priv = ib_get_agent_port(device, port_num);
-	if (!port_priv) {
-		printk(KERN_DEBUG SPFX "smi_check_local_dr_smp %s port %d "
-		       "not open\n", device->name, port_num);
-		return 1;
-	}
-
-	return smi_check_local_smp(port_priv->agent[0], smp);
-}
-
 int agent_send_response(struct ib_mad *mad, struct ib_grh *grh,
 			struct ib_wc *wc, struct ib_device *device,
 			int port_num, int qpn)
Index: mad.c
===================================================================
--- mad.c	(revision 4978)
+++ mad.c	(working copy)
@@ -671,8 +671,8 @@
 		goto out;
 	}
 	/* Check to post send on QP or process locally */
-	ret = smi_check_local_dr_smp(smp, device, port_num);
-	if (!ret || !device->process_mad)
+	ret = smi_check_local_smp(smp, device);
+	if (!ret)
 		goto out;
 
 	local = kmalloc(sizeof *local, GFP_ATOMIC);
@@ -1669,9 +1669,7 @@
 					    port_priv->device->node_type,
 					    port_priv->port_num))
 			goto out;
-		if (!smi_check_local_dr_smp(&recv->mad.smp,
-					    port_priv->device,
-					    port_priv->port_num))
+		if (!smi_check_local_smp(&recv->mad.smp, port_priv->device))
 			goto out;
 	}
 

-- 
Ralph Campbell <ralphc at pathscale.com>




More information about the general mailing list