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

[Hal Rosenstock]

Hi Steve,

On 10/10/07, swelch at systemfabricworks.com <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

Looks pretty good. A few things below and a couple of nits embedded:

I think the original description was more detailed and should be added
to the above:

 The local loopback of an outgoing DR SMP response is limited to those that
 originate at the driver specific SMA implementation during the drivers
 process_mad() function.  This patch enables the DR SMP response
 originating in user space (or elsewhere) to be delivered back up the
 stack on the same node.  In this case the driver specific process_mad()
 function does not consume or process the MAD so it must be manually
 copied to the MAD buffer which is to be handed off to a local agent.

> Signed-off-by: Steve Welch <swelch at systemfabricworks.com>

My main concern is verifying this with the various HCA drivers
(Mellanox (in normal HCA mode), iPath, and eHCA) as well as switches
(Suri, can you try this ?) in addition to running this on a node where
OpenSM resides (Sasha, can you try this ?). How much of this have you
done ? Thanks.

> ---
>  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 &&

Should this routine now be named smi_check_local_outgoing_smp for consistency ?

> +           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)

Nit. Not sure this lines up properly.

> +{
> +       /* 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_ */
> _______________________________________________
> 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
>

-- Hal