[ofa-general] Re: [ewg] [PATCH 3/9] ib_core: RDMAoE support only QP1

Hal Rosenstock hal.rosenstock at gmail.com
Mon Jun 15 11:26:42 PDT 2009


On Mon, Jun 15, 2009 at 3:34 AM, Eli Cohen<eli at mellanox.co.il> wrote:
> Since RDMAoE is using Ethernet there is no need for QP0. This patch will create
> only QP1 for RDMAoE ports.

Should ib_post_send_mad return some error on QP0 sends on RDMAoE ports ?
What QP1 sends are allowed ? Is it only SA sends which are faked ? How
are others handled ? These questions are important to what happens to
the IB management/diag tools when invoked on an RDMAoE port. We need
to be able to handle that scenario cleanly.

Also, one minor comment below.

> Signed-off-by: Eli Cohen <eli at mellanox.co.il>
> ---
>  drivers/infiniband/core/agent.c |   16 ++++++++-----
>  drivers/infiniband/core/mad.c   |   48 ++++++++++++++++++++++++++++++---------
>  2 files changed, 47 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/infiniband/core/agent.c b/drivers/infiniband/core/agent.c
> index ae7c288..658a278 100644
> --- a/drivers/infiniband/core/agent.c
> +++ b/drivers/infiniband/core/agent.c
> @@ -46,8 +46,10 @@
>  #define SPFX "ib_agent: "
>
>  struct ib_agent_port_private {
> -       struct list_head port_list;
> -       struct ib_mad_agent *agent[2];
> +       struct list_head        port_list;
> +       struct ib_mad_agent    *agent[2];
> +       struct ib_device       *device;
> +       u8                      port_num;
>  };
>
>  static DEFINE_SPINLOCK(ib_agent_port_list_lock);
> @@ -58,11 +60,10 @@ __ib_get_agent_port(struct ib_device *device, int port_num)
>  {
>        struct ib_agent_port_private *entry;
>
> -       list_for_each_entry(entry, &ib_agent_port_list, port_list) {
> -               if (entry->agent[0]->device == device &&
> -                   entry->agent[0]->port_num == port_num)
> +       list_for_each_entry(entry, &ib_agent_port_list, port_list)
> +               if (entry->device == device && entry->port_num == port_num)
>                        return entry;
> -       }
> +
>        return NULL;
>  }
>
> @@ -175,6 +176,9 @@ int ib_agent_port_open(struct ib_device *device, int port_num)
>                goto error3;
>        }
>
> +       port_priv->device = device;
> +       port_priv->port_num = port_num;
> +
>        spin_lock_irqsave(&ib_agent_port_list_lock, flags);
>        list_add_tail(&port_priv->port_list, &ib_agent_port_list);
>        spin_unlock_irqrestore(&ib_agent_port_list_lock, flags);
> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> index de922a0..aadf396 100644
> --- a/drivers/infiniband/core/mad.c
> +++ b/drivers/infiniband/core/mad.c
> @@ -199,6 +199,16 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
>        unsigned long flags;
>        u8 mgmt_class, vclass;
>
> +       /* Validate device and port */
> +       port_priv = ib_get_mad_port(device, port_num);
> +       if (!port_priv) {
> +               ret = ERR_PTR(-ENODEV);
> +               goto error1;
> +       }
> +
> +       if (!port_priv->qp_info[qp_type].qp)
> +               return NULL;
> +
>        /* Validate parameters */
>        qpn = get_spl_qp_index(qp_type);
>        if (qpn == -1)
> @@ -260,13 +270,6 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
>                        goto error1;
>        }
>
> -       /* Validate device and port */
> -       port_priv = ib_get_mad_port(device, port_num);
> -       if (!port_priv) {
> -               ret = ERR_PTR(-ENODEV);
> -               goto error1;
> -       }
> -
>        /* Allocate structures */
>        mad_agent_priv = kzalloc(sizeof *mad_agent_priv, GFP_KERNEL);
>        if (!mad_agent_priv) {
> @@ -556,6 +559,9 @@ int ib_unregister_mad_agent(struct ib_mad_agent *mad_agent)
>        struct ib_mad_agent_private *mad_agent_priv;
>        struct ib_mad_snoop_private *mad_snoop_priv;
>
> +       if (!mad_agent)
> +               return 0;
> +
>        /* If the TID is zero, the agent can only snoop. */
>        if (mad_agent->hi_tid) {
>                mad_agent_priv = container_of(mad_agent,
> @@ -2602,6 +2608,9 @@ static void cleanup_recv_queue(struct ib_mad_qp_info *qp_info)
>        struct ib_mad_private *recv;
>        struct ib_mad_list_head *mad_list;
>
> +       if (!qp_info->qp)
> +               return;
> +
>        while (!list_empty(&qp_info->recv_queue.list)) {
>
>                mad_list = list_entry(qp_info->recv_queue.list.next,
> @@ -2643,6 +2652,9 @@ static int ib_mad_port_start(struct ib_mad_port_private *port_priv)
>
>        for (i = 0; i < IB_MAD_QPS_CORE; i++) {
>                qp = port_priv->qp_info[i].qp;
> +               if (!qp)
> +                       continue;
> +
>                /*
>                 * PKey index for QP1 is irrelevant but
>                 * one is needed for the Reset to Init transition
> @@ -2684,6 +2696,9 @@ static int ib_mad_port_start(struct ib_mad_port_private *port_priv)
>        }
>
>        for (i = 0; i < IB_MAD_QPS_CORE; i++) {
> +               if (!port_priv->qp_info[i].qp)
> +                       continue;
> +
>                ret = ib_mad_post_receive_mads(&port_priv->qp_info[i], NULL);
>                if (ret) {
>                        printk(KERN_ERR PFX "Couldn't post receive WRs\n");
> @@ -2762,6 +2777,9 @@ error:
>
>  static void destroy_mad_qp(struct ib_mad_qp_info *qp_info)
>  {
> +       if (!qp_info->qp)
> +               return;
> +
>        ib_destroy_qp(qp_info->qp);
>        kfree(qp_info->snoop_table);
>  }
> @@ -2777,6 +2795,7 @@ static int ib_mad_port_open(struct ib_device *device,
>        struct ib_mad_port_private *port_priv;
>        unsigned long flags;
>        char name[sizeof "ib_mad123"];
> +       int has_smi;
>
>        /* Create new device info */
>        port_priv = kzalloc(sizeof *port_priv, GFP_KERNEL);
> @@ -2792,7 +2811,11 @@ static int ib_mad_port_open(struct ib_device *device,
>        init_mad_qp(port_priv, &port_priv->qp_info[0]);
>        init_mad_qp(port_priv, &port_priv->qp_info[1]);
>
> +       has_smi = ib_get_port_link_type(device, port_num) == PORT_LINK_IB;
>        cq_size = (IB_MAD_QP_SEND_SIZE + IB_MAD_QP_RECV_SIZE) * 2;
> +       if (has_smi)
> +               cq_size *= 2;
> +
>        port_priv->cq = ib_create_cq(port_priv->device,
>                                     ib_mad_thread_completion_handler,
>                                     NULL, port_priv, cq_size, 0);
> @@ -2816,9 +2839,11 @@ static int ib_mad_port_open(struct ib_device *device,
>                goto error5;
>        }
>
> -       ret = create_mad_qp(&port_priv->qp_info[0], IB_QPT_SMI);
> -       if (ret)
> -               goto error6;
> +       if (has_smi) {
> +               ret = create_mad_qp(&port_priv->qp_info[0], IB_QPT_SMI);
> +               if (ret)
> +                       goto error6;
> +       }
>        ret = create_mad_qp(&port_priv->qp_info[1], IB_QPT_GSI);
>        if (ret)
>                goto error7;
> @@ -2852,7 +2877,8 @@ error9:
>  error8:
>        destroy_mad_qp(&port_priv->qp_info[1]);
>  error7:
> -       destroy_mad_qp(&port_priv->qp_info[0]);
> +       if (has_smi)
> +               destroy_mad_qp(&port_priv->qp_info[0]);

Is something similar needed in ib_mad_port_close for handling no QP0
on RDMAoE ports in terms of destroy_mad_qp/cleanup_recv_queue calls ?

-- Hal

>  error6:
>        ib_dereg_mr(port_priv->mr);
>  error5:
> --
> 1.6.3.1
>
> _______________________________________________
> ewg mailing list
> ewg at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
>



More information about the general mailing list