[openib-general] Another MAD processing question

[Hal Rosenstock]

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