[openib-general] Another MAD processing question

[Hal Rosenstock]

On Sat, 2006-01-14 at 12:38, Hal Rosenstock wrote:
> On Fri, 2006-01-13 at 13:32, Ralph Campbell wrote:
> > After looking at the MAD handling code some more, I am puzzled by
> > the following.
> 
> I'll respond in 2 parts to this.
> 
> > 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 "
> 
> 
> Yes, that check is redundant.
> 
> Thanks. Applied.
> 
> -- Hal
> 
> > 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:

This is correct as well. Thanks! Applied.

-- Hal

> > 
> > 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;
> >  	}
> >