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