[ofa-general] [PATCH] infiniband/core: Enable loopback of DR SMP responses from userspace

[Steve Welch]

> -----Original Message-----
> From: Hal Rosenstock [mailto:hal.rosenstock at gmail.com]
> Sent: Wednesday, September 05, 2007 12:03 PM
> To: Steve Welch
> Cc: Sean Hefty; general at lists.openfabrics.org
> Subject: Re: [ofa-general] [PATCH] infiniband/core: Enable loopback of DR
> SMP responses from userspace

> > The new code is executed when the device specific process_mad function
> > returns only IB_MAD_RESULT_SUCCESS status in the status bitmask.  Since
> > IB_MAD_RESULT_REPLY is not also set; the device is indicating it did
> > not create a response and mad_priv->mad should be as it was before the
> > process_mad call (i.e. not initialized with a response).  Since the
> > IB_MAD_RESULT_CONSUMED status was not set in the status bitmask, the
> > original MAD is still needing delivery and by definition goes to
> > the local node.
> >
> > Prior to this patch and the addition of the of the
> > smi_check_local_resp_smp() test, the only DR SMP that could have made
> > it to the device's process_mad call would have been a DR SMP Request
> > that was targeted to the local SMA.
> 
> I don't think that statement is 100% accurate as incoming DR SMInfo
> queries make it to the SM.

True, as would any DR SMP request the local device driver would not
handle, as indicated in the next paragraph.  Replace "SMA" with "SMI"
above for clarification.

"sminfo -C mthca0 -P 1 -D 0" could be used as a test case to see the
ramifications of this change.  On a 1.2 build it does not work, on 
a 1.3 patched with this change it does.  See below:

OFED 1.2 system - LID Routed
[root at tiger:]> sminfo -C mthca0 -P 1
sminfo: sm lid 1 sm guid 0x2c901078ce001, activity count 938 priority 0
state 3 SMINFO_MASTER

OFED 1.2 system - Directed Route
[root at tiger:]> sminfo -C mthca0 -P 1 -D 0
sminfo: sm lid 0 sm guid 0xffff000000000000, activity count 0 priority 0
state 0 SMINFO_NOTACT
[root at tiger:]>

OFED 1.3 patched system - Directed Route
nirvana: # sminfo -C mthca0 -P 1 -D 0
sminfo: sm lid 0 sm guid 0x2c901078ce001, activity count 769 priority 0
state 3 SMINFO_MASTER
nirvana: #

> 
> > It is possible that the device's
> > SMA would not handle the DR SMP Request and that it would return only
> > IB_MAD_RESULT_SUCCESS; however in that case the find_mad_agent() call
> > would still access the un-initialized mad_priv->mad.mad.  For
> > this reason I do not believe this code path was previously executed,
> > and I believe there will be no effect on the existing behavior.
> >
> > Running with these changes, the IB utilities built on top of DR SMP's
> > continue to operate on the host, going both to the local SMA and out
> > on the fabric to an SMA.
> > > Also, I think we can eliminate setting the tid, since the memcpy will
> > > set that as well.
> > Yes, I agree.
> >
> > >
> > > >                     recv_mad_agent = find_mad_agent(port_priv,
> > > >                                                     &mad_priv-
> >mad.mad);
> > > >             }
> > > > diff --git a/drivers/infiniband/core/smi.h
> > > b/drivers/infiniband/core/smi.h
> > > > index 1cfc298..d96fc8e 100644
> > > > --- a/drivers/infiniband/core/smi.h
> > > > +++ b/drivers/infiniband/core/smi.h
> > > > @@ -71,4 +71,18 @@ static inline enum smi_action
> > > smi_check_local_smp(struct ib_smp *smp,
> > > >             (smp->hop_ptr == smp->hop_cnt + 1)) ?
> > > >             IB_SMI_HANDLE : IB_SMI_DISCARD);
> > > >  }
> > > > +
> > > > +/*
> > > > + * Return 1 if the SMP response should be handled by the local
> > > management stack
> > > > + */
> > >
> > > The comment is off here - return IB_SMI_HANDLE.  (It's off for
> > > smi_check_local_smp() as well.)
> > Yes, I agree.  It appears I was a little over zealous in my header
> > cut and paste of the existing DR SMP request local check function.
> >
> > >
> > > > +static inline enum smi_action smi_check_local_resp_smp(struct
> ib_smp
> > > *smp,
> > > > +                                                  struct ib_device
> > *device)
> > > > +{
> > > > +   /* C14-13:3 -- We're at the end of the DR segment of path */
> > > > +   /* C14-13:4 -- Hop Pointer == 0 -> give to SM */
> > > > +   return ((device->process_mad &&
> > > > +           ib_get_smp_direction(smp) &&
> > > > +           !smp->hop_ptr) ? IB_SMI_HANDLE : IB_SMI_DISCARD);
> > > > +}
> > >
> > > - Sean
> > Thanks,
> > Steve
> >
> > _______________________________________________
> > general mailing list
> > general at lists.openfabrics.org
> > http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> >
> > To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-
> general
> >