[ofa-general] [PATCH V3] infiniband/core: Enable loopback ofDRSMP responses from userspace
Steve Welch
swelch at systemfabricworks.com
Fri Oct 19 08:05:15 PDT 2007
Ralph,
This looks like it could be a problem, but it is not directly
related to the original patch; i.e. loop back of a DR SMP
response being sent from userspace.
I think you could probably combine the routines as you suggest
or call them both as is done to solve the original problem.
If you want to combine this change with the original patch
to make a more uniform change across both problems that
would be fine with me. Hal, Sean, do you have an opinion?
Steve
> -----Original Message-----
> From: Ralph Campbell [mailto:ralph.campbell at qlogic.com]
> Sent: Thursday, October 18, 2007 3:09 PM
> To: Steve Welch
> 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
> ofDRSMP responses from userspace
>
> 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