Re: [PATCH for-next 1/2] bnxt_re: Fix imm_data endianness

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Selvin,

On Wed, Jul 10, 2024 at 10:28 AM Selvin Xavier
<selvin.xavier@xxxxxxxxxxxx> wrote:
>
> On Tue, Jul 9, 2024 at 8:31 PM Jack Wang <jinpu.wang@xxxxxxxxx> wrote:
> >
> > When map a device between servers with MLX and BCM RoCE nics, RTRS
> > server complain about unknown imm type, and can't map the device,
> >
> > After more debug, it seems bnxt_re wrongly handle the
> > imm_data, this patch fixed the compat issue with MLX for us.
> >
> > In offlist discussion, Selvin confirm HW is working in little endian
> > format and all data needs to be converted to LE while providing.
> >
> > This patch fix the endianness for imm_data
> >
> > Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
> > Signed-off-by: Jack Wang <jinpu.wang@xxxxxxxxx>
> Hi Jinpu,
>  Thank you for this patch and debugging with Broadcom devices. Couple
> of comments. Also, maybe you can clean up the commit message by moving
> the reference of our discussion to the cover letter.
sure, will do.
>
> Thanks,
> Selvin
> > ---
> >  drivers/infiniband/hw/bnxt_re/ib_verbs.c | 8 ++++----
> >  drivers/infiniband/hw/bnxt_re/qplib_fp.h | 6 +++---
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > index e453ca701e87..c5080028247e 100644
> > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > @@ -2479,7 +2479,7 @@ static int bnxt_re_build_send_wqe(struct bnxt_re_qp *qp,
> >                 break;
> >         case IB_WR_SEND_WITH_IMM:
> >                 wqe->type = BNXT_QPLIB_SWQE_TYPE_SEND_WITH_IMM;
> > -               wqe->send.imm_data = wr->ex.imm_data;
> > +               wqe->send.imm_data = cpu_to_le32(be32_to_cpu(wr->ex.imm_data));
> If you see bnxt_re/qplib_fp.c, we have the following code. This
> ensures that le32
> is passed down. So in your patch, we just need to do be32_to_cpu of
> the immediate data.
> sqe->inv_key_or_imm_data = cpu_to_le32(wqe->send.inv_key);
ok, makes sense.
> >                 break;
> >         case IB_WR_SEND_WITH_INV:
> >                 wqe->type = BNXT_QPLIB_SWQE_TYPE_SEND_WITH_INV;
> > @@ -2509,7 +2509,7 @@ static int bnxt_re_build_rdma_wqe(const struct ib_send_wr *wr,
> >                 break;
> >         case IB_WR_RDMA_WRITE_WITH_IMM:
> >                 wqe->type = BNXT_QPLIB_SWQE_TYPE_RDMA_WRITE_WITH_IMM;
> > -               wqe->rdma.imm_data = wr->ex.imm_data;
> > +               wqe->rdma.imm_data = cpu_to_le32(be32_to_cpu(wr->ex.imm_data));
> Same comment as above
ditto
> >                 break;
> >         case IB_WR_RDMA_READ:
> >                 wqe->type = BNXT_QPLIB_SWQE_TYPE_RDMA_READ;
> > @@ -3582,7 +3582,7 @@ static void bnxt_re_process_res_shadow_qp_wc(struct bnxt_re_qp *gsi_sqp,
> >         wc->byte_len = orig_cqe->length;
> >         wc->qp = &gsi_qp->ib_qp;
> >
> > -       wc->ex.imm_data = orig_cqe->immdata;
> > +       wc->ex.imm_data = cpu_to_be32(le32_to_cpu(orig_cqe->immdata));
> >         wc->src_qp = orig_cqe->src_qp;
> >         memcpy(wc->smac, orig_cqe->smac, ETH_ALEN);
> >         if (bnxt_re_is_vlan_pkt(orig_cqe, &vlan_id, &sl)) {
> > @@ -3727,7 +3727,7 @@ int bnxt_re_poll_cq(struct ib_cq *ib_cq, int num_entries, struct ib_wc *wc)
> >                                  (unsigned long)(cqe->qp_handle),
> >                                  struct bnxt_re_qp, qplib_qp);
> >                         wc->qp = &qp->ib_qp;
> > -                       wc->ex.imm_data = cqe->immdata;
> > +                       wc->ex.imm_data = cpu_to_be32(le32_to_cpu(cqe->immdata));
> >                         wc->src_qp = cqe->src_qp;
> >                         memcpy(wc->smac, cqe->smac, ETH_ALEN);
> >                         wc->port_num = 1;
> > diff --git a/drivers/infiniband/hw/bnxt_re/qplib_fp.h b/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> > index 4aaac84c1b1b..1fcaba0f680b 100644
> > --- a/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> > +++ b/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> > @@ -164,7 +164,7 @@ struct bnxt_qplib_swqe {
> >                 /* Send, with imm, inval key */
> >                 struct {
> >                         union {
> > -                               __be32  imm_data;
> > +                               __le32  imm_data;
> Once you implement according to my comment above, this can be a u32
will do.
> >                                 u32     inv_key;
> >                         };
> >                         u32             q_key;
> > @@ -182,7 +182,7 @@ struct bnxt_qplib_swqe {
> >                 /* RDMA write, with imm, read */
> >                 struct {
> >                         union {
> > -                               __be32  imm_data;
> > +                               __le32  imm_data;
> >                                 u32     inv_key;
> >                         };
> >                         u64             remote_va;
> > @@ -389,7 +389,7 @@ struct bnxt_qplib_cqe {
> >         u16                             cfa_meta;
> >         u64                             wr_id;
> >         union {
> > -               __be32                  immdata;
> > +               __le32                  immdata;
> >                 u32                     invrkey;
> >         };
> >         u64                             qp_handle;
> > --
> > 2.34.1
> >





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux