[ofa-general] [PATCH V3] infiniband/core: Enable loopback ofDR SMP responses from userspace
Ralph Campbell
ralph.campbell at qlogic.com
Thu Oct 18 13:08:45 PDT 2007
If ib_mad_recv_done_handler() on a switch device gets a DR SMP with:
D == 1, smp->hop_ptr == 1, smp->dr_slid != IB_LID_PERMISSIVE,
then smi_handle_dr_smp_recv() returns IB_SMI_HANDLE,
smi_check_forward_dr_smp() returns IB_SMI_SEND,
smi_handle_dr_smp_send() does smp->hop_ptr-- and returns IB_SMI_HANDLE,
smi_check_local_smp() returns IB_SMI_DISCARD.
Instead, it should send a copy of the received packet with LRH:DLID
set to the smp->dr_slid. So I think smi_check_local_returning_smp()
needs to be called before smi_check_local_smp()
and do the appropriate code for forwarding the packet.
Alternatively, we could move the code from
smi_check_local_returning_smp() into smi_check_local_smp() and
let the device's process_mad() function do the forwarding.
On Wed, 2007-10-17 at 20:59 -0500, Steve Welch wrote:
> I believe in the case of ib_mad_recv_done_handler(), the call
> smi_check_forward_dr_smp() will return 0 indicating it should be
> handled by the local stack because the hop pointer will equal
> 0 (in the case where the DR SMP response should be delivered to
> the stack). The smi_check_local_smp() call would not be reached.
>
> The second part of the original fix is not required either
> in ib_mad_recv_done_handler(); when the device process mad
> routine does not reply or consume the MAD it uses the
> original receive mad to deliver to the MAD to the local agent,
> eliminating the need for the memcpy.
>
> Steve
> > -----Original Message-----
> > From: Ralph Campbell [mailto:ralph.campbell at qlogic.com]
> > Sent: Wednesday, October 17, 2007 7:33 PM
> > To: swelch at systemfabricworks.com
> > Cc: rdreier at cisco.com; sean.hefty at intel.com; general at lists.openfabrics.org
> > Subject: Re: [ofa-general] [PATCH V3] infiniband/core: Enable loopback
> > ofDR SMP responses from userspace
> >
> > Steve's patch plus the attached patch for ib_ipath allows loopback
> > to work and doesn't seem to obviously break anything.
> >
> > I was wondering though about adding the code from
> > smi_check_local_returning_smp() to smi_check_local_smp()
> > instead of defining a separate function.
> > That got me thinking about what happens when a return path DR SMP
> > is received and ib_mad_recv_done_handler() calls smi_check_local_smp().
> > Now I'm trying to convince myself one way or the other whether
> > the same checks inib_mad_recv_done_handler() are needed or not.
> >
> > On Wed, 2007-10-10 at 22:29 -0500, swelch at systemfabricworks.com wrote:
> > >
> > > Sean, Roland,
> > >
> > > This patch [v3] replaces the [v2] patch; it includes those changes but
> > renames
> > > the smi function testing returning SMP requests to the name Hal
> > recommends.
> > >
> > > This patch allows userspace DR SMP responses to be looped back and
> > delivered
> > > to a local mad agent by the management stack.
> > >
> > > Thanks, Steve
> > >
> > > Signed-off-by: Steve Welch <swelch at systemfabricworks.com>
> > > ---
> > > drivers/infiniband/core/mad.c | 6 +++---
> > > drivers/infiniband/core/smi.h | 18 +++++++++++++++++-
> > > 2 files changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/mad.c
> > b/drivers/infiniband/core/mad.c
> > > index 6f42877..98148d6 100644
> > > --- a/drivers/infiniband/core/mad.c
> > > +++ b/drivers/infiniband/core/mad.c
> > > @@ -701,7 +701,8 @@ static int handle_outgoing_dr_smp(struct
> > ib_mad_agent_private *mad_agent_priv,
> > > }
> > >
> > > /* Check to post send on QP or process locally */
> > > - if (smi_check_local_smp(smp, device) == IB_SMI_DISCARD)
> > > + if (smi_check_local_smp(smp, device) == IB_SMI_DISCARD &&
> > > + smi_check_local_returning_smp(smp, device) == IB_SMI_DISCARD)
> > > goto out;
> > >
> > > local = kmalloc(sizeof *local, GFP_ATOMIC);
> > > @@ -752,8 +753,7 @@ static int handle_outgoing_dr_smp(struct
> > ib_mad_agent_private *mad_agent_priv,
> > > port_priv = ib_get_mad_port(mad_agent_priv->agent.device,
> > > mad_agent_priv->agent.port_num);
> > > 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));
> > > 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..aff96ba 100644
> > > --- a/drivers/infiniband/core/smi.h
> > > +++ b/drivers/infiniband/core/smi.h
> > > @@ -59,7 +59,8 @@ extern enum smi_action smi_handle_dr_smp_send(struct
> > ib_smp *smp,
> > > u8 node_type, int port_num);
> > >
> > > /*
> > > - * Return 1 if the SMP should be handled by the local SMA/SM via
> > process_mad
> > > + * Return IB_SMI_HANDLE if the SMP should be handled by the local
> > SMA/SM
> > > + * via process_mad
> > > */
> > > static inline enum smi_action smi_check_local_smp(struct ib_smp *smp,
> > > struct ib_device *device)
> > > @@ -71,4 +72,19 @@ 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 IB_SMI_HANDLE if the SMP should be handled by the local
> > SMA/SM
> > > + * via process_mad
> > > + */
> > > +static inline enum smi_action smi_check_local_returning_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);
> > > +}
> > > +
> > > #endif /* __SMI_H_ */
> >
> >
> >
> > diff --git a/drivers/infiniband/hw/ipath/ipath_mad.c
> > b/drivers/infiniband/hw/ipath/ipath_mad.c
> > index 3d1432d..1978c34 100644
> > --- a/drivers/infiniband/hw/ipath/ipath_mad.c
> > +++ b/drivers/infiniband/hw/ipath/ipath_mad.c
> > @@ -1434,7 +1434,7 @@ static int process_subn(struct ib_device *ibdev, int
> > mad_flags,
> > * before checking for other consumers.
> > * Just tell the caller to process it normally.
> > */
> > - ret = IB_MAD_RESULT_FAILURE;
> > + ret = IB_MAD_RESULT_SUCCESS;
> > goto bail;
> > default:
> > smp->status |= IB_SMP_UNSUP_METHOD;
> > @@ -1516,7 +1516,7 @@ static int process_perf(struct ib_device *ibdev, u8
> > port_num,
> > * before checking for other consumers.
> > * Just tell the caller to process it normally.
> > */
> > - ret = IB_MAD_RESULT_FAILURE;
> > + ret = IB_MAD_RESULT_SUCCESS;
> > goto bail;
> > default:
> > pmp->status |= IB_SMP_UNSUP_METHOD;
>
>
More information about the general
mailing list