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:[EMAIL PROTECTED] > > Sent: Wednesday, October 17, 2007 7:33 PM > > To: [EMAIL PROTECTED] > > Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; [email protected] > > 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, [EMAIL PROTECTED] 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 <[EMAIL PROTECTED]> > > > --- > > > 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; > > _______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
