Oof, that was a bad one and the following patch fixes the problem.

diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index f805e8a..4eaee27 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -255,7 +255,7 @@ static int send_wqe_overhead(enum ib_qp_type type, u32 
flags)
        case IB_QPT_UD:
                return sizeof (struct mlx4_wqe_ctrl_seg) +
                        sizeof (struct mlx4_wqe_datagram_seg) +
-                       (flags & MLX4_IB_QP_LSO) ? 64 : 0;
+                       ((flags & MLX4_IB_QP_LSO) ? 64 : 0);
        case IB_QPT_UC:
                return sizeof (struct mlx4_wqe_ctrl_seg) +
                        sizeof (struct mlx4_wqe_raddr_seg);


and the explanation is this. Since '+' precedes the '?' operator, the
expression evaluated is:
        sizeof (struct mlx4_wqe_ctrl_seg) + sizeof (struct 
mlx4_wqe_datagram_seg) +
                (flags & MLX4_IB_QP_LSO)

which is obviously true so the value returned is 64. The parentheses
around the '?' gives the desired result. 

On Tue, 2008-04-01 at 12:41 -0700, Roland Dreier wrote:
> > would like me to re-generate the mlx4 LSO patch to match this commit or
>  > would you do the adjustments?
> 
> Sorry for being so slow.
> 
> Anyway I did the adjustments as below.  I also removed the "reserve"
> variable and moved the 64 byte extra for LSO into send_wqe_overhead(),
> since it seemed that the only place where you used send_wqe_overhead()
> without adding in reserve was actually a bug.
> 
> I also did various changes other places, and maybe introduced a bug:
> when I try NPtcp between two systems (once running unmodified
> 2.6.25-rc8, the other running my for-2.6.26 branch, both with ConnectX
> with FW 2.3.000), on the side with the LSO patch, I eventually get a
> "local length error" or "local QP operation err" on a send.  It is an
> LSO send of length 63744 with 17 fragments and an mss of 1992, so it
> should be segmented into 32 packets.  Some of these sends complete
> successfully but eventually one fails.  I'm still debugging but maybe
> you have some idea?
> 
> When I get the local QP operation error, I get this in case it helps:
> 
> local QP operation err (QPN 000048, WQE index affa, vendor syndrome 6f, 
> opcode = 5e)
> CQE contents 00000048 00000000 00000000 00000000 00000000 00000000 affa6f02 
> 0000005e
> 
>  - R.
> 
> From 141035c707b81638659ada01f456d066f2b353f7 Mon Sep 17 00:00:00 2001
> From: Eli Cohen <[EMAIL PROTECTED]>
> Date: Tue, 25 Mar 2008 15:35:12 +0200
> Subject: [PATCH] IB/mlx4: Add IPoIB LSO support
> 
> Add TSO support to the mlx4_ib driver.
> 
> Signed-off-by: Eli Cohen <[EMAIL PROTECTED]>
> Signed-off-by: Roland Dreier <[EMAIL PROTECTED]>
> ---
>  drivers/infiniband/hw/mlx4/cq.c      |    3 +
>  drivers/infiniband/hw/mlx4/main.c    |    2 +
>  drivers/infiniband/hw/mlx4/mlx4_ib.h |    5 ++
>  drivers/infiniband/hw/mlx4/qp.c      |   72 +++++++++++++++++++++++++++++----
>  drivers/net/mlx4/fw.c                |    9 ++++
>  drivers/net/mlx4/fw.h                |    1 +
>  drivers/net/mlx4/main.c              |    1 +
>  include/linux/mlx4/device.h          |    1 +
>  include/linux/mlx4/qp.h              |    5 ++
>  9 files changed, 90 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx4/cq.c b/drivers/infiniband/hw/mlx4/cq.c
> index d2e32b0..7d70af7 100644
> --- a/drivers/infiniband/hw/mlx4/cq.c
> +++ b/drivers/infiniband/hw/mlx4/cq.c
> @@ -420,6 +420,9 @@ static int mlx4_ib_poll_one(struct mlx4_ib_cq *cq,
>               case MLX4_OPCODE_BIND_MW:
>                       wc->opcode    = IB_WC_BIND_MW;
>                       break;
> +             case MLX4_OPCODE_LSO:
> +                     wc->opcode    = IB_WC_LSO;
> +                     break;
>               }
>       } else {
>               wc->byte_len = be32_to_cpu(cqe->byte_cnt);
> diff --git a/drivers/infiniband/hw/mlx4/main.c 
> b/drivers/infiniband/hw/mlx4/main.c
> index 6ea4746..e9330a0 100644
> --- a/drivers/infiniband/hw/mlx4/main.c
> +++ b/drivers/infiniband/hw/mlx4/main.c
> @@ -101,6 +101,8 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
>               props->device_cap_flags |= IB_DEVICE_UD_AV_PORT_ENFORCE;
>       if (dev->dev->caps.flags & MLX4_DEV_CAP_FLAG_IPOIB_CSUM)
>               props->device_cap_flags |= IB_DEVICE_UD_IP_CSUM;
> +     if (dev->dev->caps.max_gso_sz)
> +             props->device_cap_flags |= IB_DEVICE_UD_TSO;
>  
>       props->vendor_id           = be32_to_cpup((__be32 *) (out_mad->data + 
> 36)) &
>               0xffffff;
> diff --git a/drivers/infiniband/hw/mlx4/mlx4_ib.h 
> b/drivers/infiniband/hw/mlx4/mlx4_ib.h
> index 3726e45..3f8bd0a 100644
> --- a/drivers/infiniband/hw/mlx4/mlx4_ib.h
> +++ b/drivers/infiniband/hw/mlx4/mlx4_ib.h
> @@ -110,6 +110,10 @@ struct mlx4_ib_wq {
>       unsigned                tail;
>  };
>  
> +enum mlx4_ib_qp_flags {
> +     MLX4_IB_QP_LSO          = 1 << 0
> +};
> +
>  struct mlx4_ib_qp {
>       struct ib_qp            ibqp;
>       struct mlx4_qp          mqp;
> @@ -129,6 +133,7 @@ struct mlx4_ib_qp {
>       struct mlx4_mtt         mtt;
>       int                     buf_size;
>       struct mutex            mutex;
> +     u32                     flags;
>       u8                      port;
>       u8                      alt_port;
>       u8                      atomic_rd_en;
> diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
> index 320c25f..8ddb97e 100644
> --- a/drivers/infiniband/hw/mlx4/qp.c
> +++ b/drivers/infiniband/hw/mlx4/qp.c
> @@ -71,6 +71,7 @@ enum {
>  
>  static const __be32 mlx4_ib_opcode[] = {
>       [IB_WR_SEND]                    = 
> __constant_cpu_to_be32(MLX4_OPCODE_SEND),
> +     [IB_WR_LSO]                     = 
> __constant_cpu_to_be32(MLX4_OPCODE_LSO),
>       [IB_WR_SEND_WITH_IMM]           = 
> __constant_cpu_to_be32(MLX4_OPCODE_SEND_IMM),
>       [IB_WR_RDMA_WRITE]              = 
> __constant_cpu_to_be32(MLX4_OPCODE_RDMA_WRITE),
>       [IB_WR_RDMA_WRITE_WITH_IMM]     = 
> __constant_cpu_to_be32(MLX4_OPCODE_RDMA_WRITE_IMM),
> @@ -242,7 +243,7 @@ static void mlx4_ib_qp_event(struct mlx4_qp *qp, enum 
> mlx4_event type)
>       }
>  }
>  
> -static int send_wqe_overhead(enum ib_qp_type type)
> +static int send_wqe_overhead(enum ib_qp_type type, u32 flags)
>  {
>       /*
>        * UD WQEs must have a datagram segment.
> @@ -253,7 +254,8 @@ static int send_wqe_overhead(enum ib_qp_type type)
>       switch (type) {
>       case IB_QPT_UD:
>               return sizeof (struct mlx4_wqe_ctrl_seg) +
> -                     sizeof (struct mlx4_wqe_datagram_seg);
> +                     sizeof (struct mlx4_wqe_datagram_seg) +
> +                     (flags & MLX4_IB_QP_LSO) ? 64 : 0;
>       case IB_QPT_UC:
>               return sizeof (struct mlx4_wqe_ctrl_seg) +
>                       sizeof (struct mlx4_wqe_raddr_seg);
> @@ -315,7 +317,7 @@ static int set_kernel_sq_size(struct mlx4_ib_dev *dev, 
> struct ib_qp_cap *cap,
>       /* Sanity check SQ size before proceeding */
>       if (cap->max_send_wr     > dev->dev->caps.max_wqes  ||
>           cap->max_send_sge    > dev->dev->caps.max_sq_sg ||
> -         cap->max_inline_data + send_wqe_overhead(type) +
> +         cap->max_inline_data + send_wqe_overhead(type, qp->flags) +
>           sizeof (struct mlx4_wqe_inline_seg) > dev->dev->caps.max_sq_desc_sz)
>               return -EINVAL;
>  
> @@ -329,7 +331,7 @@ static int set_kernel_sq_size(struct mlx4_ib_dev *dev, 
> struct ib_qp_cap *cap,
>  
>       s = max(cap->max_send_sge * sizeof (struct mlx4_wqe_data_seg),
>               cap->max_inline_data + sizeof (struct mlx4_wqe_inline_seg)) +
> -             send_wqe_overhead(type);
> +             send_wqe_overhead(type, qp->flags);
>  
>       /*
>        * Hermon supports shrinking WQEs, such that a single work
> @@ -394,7 +396,8 @@ static int set_kernel_sq_size(struct mlx4_ib_dev *dev, 
> struct ib_qp_cap *cap,
>       }
>  
>       qp->sq.max_gs = ((qp->sq_max_wqes_per_wr << qp->sq.wqe_shift) -
> -                      send_wqe_overhead(type)) / sizeof (struct 
> mlx4_wqe_data_seg);
> +                      send_wqe_overhead(type, qp->flags)) /
> +             sizeof (struct mlx4_wqe_data_seg);
>  
>       qp->buf_size = (qp->rq.wqe_cnt << qp->rq.wqe_shift) +
>               (qp->sq.wqe_cnt << qp->sq.wqe_shift);
> @@ -503,6 +506,9 @@ static int create_qp_common(struct mlx4_ib_dev *dev, 
> struct ib_pd *pd,
>       } else {
>               qp->sq_no_prefetch = 0;
>  
> +             if (init_attr->create_flags & IB_QP_CREATE_IPOIB_UD_LSO)
> +                     qp->flags |= MLX4_IB_QP_LSO;
> +
>               err = set_kernel_sq_size(dev, &init_attr->cap, 
> init_attr->qp_type, qp);
>               if (err)
>                       goto err;
> @@ -673,7 +679,11 @@ struct ib_qp *mlx4_ib_create_qp(struct ib_pd *pd,
>       struct mlx4_ib_qp *qp;
>       int err;
>  
> -     if (init_attr->create_flags)
> +     /* We only support LSO, and only for kernel UD QPs. */
> +     if (init_attr->create_flags & ~IB_QP_CREATE_IPOIB_UD_LSO)
> +             return ERR_PTR(-EINVAL);
> +     if (init_attr->create_flags & IB_QP_CREATE_IPOIB_UD_LSO &&
> +         (pd->uobject || init_attr->qp_type != IB_QPT_UD))
>               return ERR_PTR(-EINVAL);
>  
>       switch (init_attr->qp_type) {
> @@ -879,10 +889,15 @@ static int __mlx4_ib_modify_qp(struct ib_qp *ibqp,
>               }
>       }
>  
> -     if (ibqp->qp_type == IB_QPT_GSI || ibqp->qp_type == IB_QPT_SMI ||
> -         ibqp->qp_type == IB_QPT_UD)
> +     if (ibqp->qp_type == IB_QPT_GSI || ibqp->qp_type == IB_QPT_SMI)
>               context->mtu_msgmax = (IB_MTU_4096 << 5) | 11;
> -     else if (attr_mask & IB_QP_PATH_MTU) {
> +     else if (ibqp->qp_type == IB_QPT_UD) {
> +             if (qp->flags & MLX4_IB_QP_LSO)
> +                     context->mtu_msgmax = (IB_MTU_4096 << 5) |
> +                                           ilog2(dev->dev->caps.max_gso_sz);
> +             else
> +                     context->mtu_msgmax = (IB_MTU_4096 << 5) | 11;
> +     } else if (attr_mask & IB_QP_PATH_MTU) {
>               if (attr->path_mtu < IB_MTU_256 || attr->path_mtu > 
> IB_MTU_4096) {
>                       printk(KERN_ERR "path MTU (%u) is invalid\n",
>                              attr->path_mtu);
> @@ -1399,6 +1414,34 @@ static void __set_data_seg(struct mlx4_wqe_data_seg 
> *dseg, struct ib_sge *sg)
>       dseg->addr       = cpu_to_be64(sg->addr);
>  }
>  
> +static int build_lso_seg(struct mlx4_lso_seg *wqe, struct ib_send_wr *wr,
> +                      struct mlx4_ib_qp *qp, unsigned *lso_seg_len)
> +{
> +     unsigned halign = ALIGN(wr->wr.ud.hlen, 16);
> +
> +     /*
> +      * This is a temporary limitation and will be removed in
> +      * a forthcoming FW release:
> +      */
> +     if (unlikely(wr->wr.ud.hlen) > 60)
> +             return -EINVAL;
> +
> +     if (unlikely(!(qp->flags & MLX4_IB_QP_LSO) &&
> +                  wr->num_sge > qp->sq.max_gs - (halign >> 4)))
> +             return -EINVAL;
> +
> +     memcpy(wqe->header, wr->wr.ud.header, wr->wr.ud.hlen);
> +
> +     /* make sure LSO header is written before overwriting stamping */
> +     wmb();
> +
> +     wqe->mss_hdr_size = cpu_to_be32((wr->wr.ud.mss - wr->wr.ud.hlen) << 16 |
> +                                     wr->wr.ud.hlen);
> +
> +     *lso_seg_len = halign;
> +     return 0;
> +}
> +
>  int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
>                     struct ib_send_wr **bad_wr)
>  {
> @@ -1412,6 +1455,7 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct 
> ib_send_wr *wr,
>       unsigned ind;
>       int uninitialized_var(stamp);
>       int uninitialized_var(size);
> +     unsigned seglen;
>       int i;
>  
>       spin_lock_irqsave(&qp->sq.lock, flags);
> @@ -1490,6 +1534,16 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct 
> ib_send_wr *wr,
>                       set_datagram_seg(wqe, wr);
>                       wqe  += sizeof (struct mlx4_wqe_datagram_seg);
>                       size += sizeof (struct mlx4_wqe_datagram_seg) / 16;
> +
> +                     if (wr->opcode == IB_WR_LSO) {
> +                             err = build_lso_seg(wqe, wr, qp, &seglen);
> +                             if (err) {
> +                                     *bad_wr = wr;
> +                                     goto out;
> +                             }
> +                             wqe  += seglen;
> +                             size += seglen / 16;
> +                     }
>                       break;
>  
>               case IB_QPT_SMI:
> diff --git a/drivers/net/mlx4/fw.c b/drivers/net/mlx4/fw.c
> index f494c3e..d82f275 100644
> --- a/drivers/net/mlx4/fw.c
> +++ b/drivers/net/mlx4/fw.c
> @@ -133,6 +133,7 @@ int mlx4_QUERY_DEV_CAP(struct mlx4_dev *dev, struct 
> mlx4_dev_cap *dev_cap)
>  #define QUERY_DEV_CAP_MAX_AV_OFFSET          0x27
>  #define QUERY_DEV_CAP_MAX_REQ_QP_OFFSET              0x29
>  #define QUERY_DEV_CAP_MAX_RES_QP_OFFSET              0x2b
> +#define QUERY_DEV_CAP_MAX_GSO_OFFSET         0x2d
>  #define QUERY_DEV_CAP_MAX_RDMA_OFFSET                0x2f
>  #define QUERY_DEV_CAP_RSZ_SRQ_OFFSET         0x33
>  #define QUERY_DEV_CAP_ACK_DELAY_OFFSET               0x35
> @@ -215,6 +216,13 @@ int mlx4_QUERY_DEV_CAP(struct mlx4_dev *dev, struct 
> mlx4_dev_cap *dev_cap)
>       dev_cap->max_requester_per_qp = 1 << (field & 0x3f);
>       MLX4_GET(field, outbox, QUERY_DEV_CAP_MAX_RES_QP_OFFSET);
>       dev_cap->max_responder_per_qp = 1 << (field & 0x3f);
> +     MLX4_GET(field, outbox, QUERY_DEV_CAP_MAX_GSO_OFFSET);
> +     field &= 0x1f;
> +     if (!field)
> +             dev_cap->max_gso_sz = 0;
> +     else
> +             dev_cap->max_gso_sz = 1 << field;
> +
>       MLX4_GET(field, outbox, QUERY_DEV_CAP_MAX_RDMA_OFFSET);
>       dev_cap->max_rdma_global = 1 << (field & 0x3f);
>       MLX4_GET(field, outbox, QUERY_DEV_CAP_ACK_DELAY_OFFSET);
> @@ -377,6 +385,7 @@ int mlx4_QUERY_DEV_CAP(struct mlx4_dev *dev, struct 
> mlx4_dev_cap *dev_cap)
>                dev_cap->max_sq_desc_sz, dev_cap->max_sq_sg);
>       mlx4_dbg(dev, "Max RQ desc size: %d, max RQ S/G: %d\n",
>                dev_cap->max_rq_desc_sz, dev_cap->max_rq_sg);
> +     mlx4_dbg(dev, "Max GSO size: %d\n", dev_cap->max_gso_sz);
>  
>       dump_dev_cap_flags(dev, dev_cap->flags);
>  
> diff --git a/drivers/net/mlx4/fw.h b/drivers/net/mlx4/fw.h
> index e16dec8..306cb9b 100644
> --- a/drivers/net/mlx4/fw.h
> +++ b/drivers/net/mlx4/fw.h
> @@ -96,6 +96,7 @@ struct mlx4_dev_cap {
>       u8  bmme_flags;
>       u32 reserved_lkey;
>       u64 max_icm_sz;
> +     int max_gso_sz;
>  };
>  
>  struct mlx4_adapter {
> diff --git a/drivers/net/mlx4/main.c b/drivers/net/mlx4/main.c
> index 08bfc13..7cfbe75 100644
> --- a/drivers/net/mlx4/main.c
> +++ b/drivers/net/mlx4/main.c
> @@ -159,6 +159,7 @@ static int mlx4_dev_cap(struct mlx4_dev *dev, struct 
> mlx4_dev_cap *dev_cap)
>       dev->caps.page_size_cap      = ~(u32) (dev_cap->min_page_sz - 1);
>       dev->caps.flags              = dev_cap->flags;
>       dev->caps.stat_rate_support  = dev_cap->stat_rate_support;
> +     dev->caps.max_gso_sz         = dev_cap->max_gso_sz;
>  
>       return 0;
>  }
> diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
> index 6cdf813..ff7df1a 100644
> --- a/include/linux/mlx4/device.h
> +++ b/include/linux/mlx4/device.h
> @@ -186,6 +186,7 @@ struct mlx4_caps {
>       u32                     flags;
>       u16                     stat_rate_support;
>       u8                      port_width_cap[MLX4_MAX_PORTS + 1];
> +     int                     max_gso_sz;
>  };
>  
>  struct mlx4_buf_list {
> diff --git a/include/linux/mlx4/qp.h b/include/linux/mlx4/qp.h
> index 31f9eb3..cf0bf4e 100644
> --- a/include/linux/mlx4/qp.h
> +++ b/include/linux/mlx4/qp.h
> @@ -219,6 +219,11 @@ struct mlx4_wqe_datagram_seg {
>       __be32                  reservd[2];
>  };
>  
> +struct mlx4_lso_seg {
> +     __be32                  mss_hdr_size;
> +     __be32                  header[0];
> +};
> +
>  struct mlx4_wqe_bind_seg {
>       __be32                  flags1;
>       __be32                  flags2;

_______________________________________________
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

Reply via email to