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;
> > }
> >
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
