[ofa-general] [PATCH] infiniband/core: Enable loopback of DR SMP responses from userspace
Hal Rosenstock
hal.rosenstock at gmail.com
Wed Sep 5 10:03:17 PDT 2007
On 9/4/07, Steve Welch <swelch at systemfabricworks.com> wrote:
> Hi Sean,
>
> > -----Original Message-----
> > From: Sean Hefty [mailto:mshefty at ichips.intel.com]
> > Sent: Tuesday, September 04, 2007 7:55 PM
> > To: swelch at systemfabricworks.com
> > Cc: general at lists.openfabrics.org; sean.hefty at intel.com
> > Subject: Re: [ofa-general] [PATCH] infiniband/core: Enable loopback of DR
> > SMP responses from userspace
> >
> > > @@ -754,6 +755,7 @@ static int handle_outgoing_dr_smp(struct
> > ib_mad_agent_private *mad_agent_priv,
> > > if (port_priv) {
> > > mad_priv->mad.mad.mad_hdr.tid =
> > > ((struct ib_mad *)smp)->mad_hdr.tid;
> > > + memcpy(&mad_priv->mad.mad, smp, sizeof(struct
> ib_mad));
> >
> > I'm having a hard time understanding the impact of this change. If I'm
> > reading the code correctly, mad_priv->mad should contain the response
> > from the device process_mad() routine. This changes that response. Can
> > you provide more details describing the effect this change has on the
> > existing behavior?
>
> 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.
> 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
>
More information about the general
mailing list