[ofa-general] Re: [PATCH] IB/core: Enhance SMI for switch support

[Roland Dreier]

 > +		retsmi = smi_check_forward_dr_smp(&recv->mad.smp);
 > +		if (!retsmi)
 >  			goto local;
 > -		if (!smi_handle_dr_smp_send(&recv->mad.smp,
 > -					    port_priv->device->node_type,
 > -					    port_priv->port_num))
 > -			goto out;
 > -		if (!smi_check_local_smp(&recv->mad.smp, port_priv->device))
 > +
 > +		if (retsmi == 1) { /* don't forward */

 >  /*
 >   * Return 1 if the received DR SMP should be forwarded to the send queue
 >   * Return 0 if the SMP should be completed up the stack
 > + * Return 2 if the SMP should be forwarded (for switches only)
 >   */
 >  int smi_check_forward_dr_smp(struct ib_smp *smp)

I think this has now crossed the line where these magic return values
should be named enums instead.  Especially the "if (!retsmi)" is very
hard to follow.

 > +/*
 > + * Return the forwarding port number from initial_path for outgoing SMP and 
 > + * from return_path for returning SMP
 > + */
 > +static inline int smi_get_fwd_port(struct ib_smp *smp)
 > +{
 > +	return (!ib_get_smp_direction(smp) ? smp->initial_path[smp->hop_ptr+1] :
 > +		smp->return_path[smp->hop_ptr-1]);
 > +}

This has exactly one caller.  I would just put this function in the .c
file where it's called.

 - R.