* [PATCH for-next 0/2] Fix for bnxt_re endianness issue
@ 2024-07-09 15:01 Jack Wang
2024-07-09 15:01 ` [PATCH for-next 1/2] bnxt_re: Fix imm_data endianness Jack Wang
2024-07-09 15:01 ` [PATCH for-next 2/2] bnxt_re: Fix inv_key endianness Jack Wang
0 siblings, 2 replies; 7+ messages in thread
From: Jack Wang @ 2024-07-09 15:01 UTC (permalink / raw)
To: linux-rdma; +Cc: bvanassche, leon, jgg, selvin.xavier, haris.iqbal, jinpu.wang
We have found an issue when map a RNBD device between MLX and BCM nics.
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,
The problem is bnxt_re do not handle endianness correctly in some cases,
hence this small patchset.
I tested it between BCM RoCE nics, also BCM and MLX NiCs for RNBD traffic.
Please review and consider for next release, the bug exists since the
beginning of bnxt_re.
Jack Wang (2):
bnxt_re: Fix imm_data endianness
bnxt_re: Fix inv_key endianness
drivers/infiniband/hw/bnxt_re/ib_verbs.c | 14 +++++++-------
drivers/infiniband/hw/bnxt_re/qplib_fp.h | 12 ++++++------
2 files changed, 13 insertions(+), 13 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH for-next 1/2] bnxt_re: Fix imm_data endianness
2024-07-09 15:01 [PATCH for-next 0/2] Fix for bnxt_re endianness issue Jack Wang
@ 2024-07-09 15:01 ` Jack Wang
2024-07-10 8:27 ` Selvin Xavier
2024-07-09 15:01 ` [PATCH for-next 2/2] bnxt_re: Fix inv_key endianness Jack Wang
1 sibling, 1 reply; 7+ messages in thread
From: Jack Wang @ 2024-07-09 15:01 UTC (permalink / raw)
To: linux-rdma; +Cc: bvanassche, leon, jgg, selvin.xavier, haris.iqbal, jinpu.wang
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@ionos.com>
---
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));
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));
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;
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH for-next 2/2] bnxt_re: Fix inv_key endianness
2024-07-09 15:01 [PATCH for-next 0/2] Fix for bnxt_re endianness issue Jack Wang
2024-07-09 15:01 ` [PATCH for-next 1/2] bnxt_re: Fix imm_data endianness Jack Wang
@ 2024-07-09 15:01 ` Jack Wang
2024-07-11 3:08 ` Greg Sword
1 sibling, 1 reply; 7+ messages in thread
From: Jack Wang @ 2024-07-09 15:01 UTC (permalink / raw)
To: linux-rdma; +Cc: bvanassche, leon, jgg, selvin.xavier, haris.iqbal, jinpu.wang
Similar like previous patch, this change the endianness for inv_key,
hw expect LE, so change the type accordingly.
Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/infiniband/hw/bnxt_re/ib_verbs.c | 6 +++---
drivers/infiniband/hw/bnxt_re/qplib_fp.h | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index c5080028247e..cdc8ebcf3a76 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -2483,7 +2483,7 @@ static int bnxt_re_build_send_wqe(struct bnxt_re_qp *qp,
break;
case IB_WR_SEND_WITH_INV:
wqe->type = BNXT_QPLIB_SWQE_TYPE_SEND_WITH_INV;
- wqe->send.inv_key = wr->ex.invalidate_rkey;
+ wqe->send.inv_key = cpu_to_le32(wr->ex.invalidate_rkey);
break;
default:
return -EINVAL;
@@ -2513,7 +2513,7 @@ static int bnxt_re_build_rdma_wqe(const struct ib_send_wr *wr,
break;
case IB_WR_RDMA_READ:
wqe->type = BNXT_QPLIB_SWQE_TYPE_RDMA_READ;
- wqe->rdma.inv_key = wr->ex.invalidate_rkey;
+ wqe->rdma.inv_key = cpu_to_le32(wr->ex.invalidate_rkey);
break;
default:
return -EINVAL;
@@ -2563,7 +2563,7 @@ static int bnxt_re_build_inv_wqe(const struct ib_send_wr *wr,
struct bnxt_qplib_swqe *wqe)
{
wqe->type = BNXT_QPLIB_SWQE_TYPE_LOCAL_INV;
- wqe->local_inv.inv_l_key = wr->ex.invalidate_rkey;
+ wqe->local_inv.inv_l_key = cpu_to_le32(wr->ex.invalidate_rkey);
if (wr->send_flags & IB_SEND_SIGNALED)
wqe->flags |= BNXT_QPLIB_SWQE_FLAGS_SIGNAL_COMP;
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_fp.h b/drivers/infiniband/hw/bnxt_re/qplib_fp.h
index 1fcaba0f680b..813332b2c872 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_fp.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_fp.h
@@ -165,7 +165,7 @@ struct bnxt_qplib_swqe {
struct {
union {
__le32 imm_data;
- u32 inv_key;
+ __le32 inv_key;
};
u32 q_key;
u32 dst_qp;
@@ -183,7 +183,7 @@ struct bnxt_qplib_swqe {
struct {
union {
__le32 imm_data;
- u32 inv_key;
+ __le32 inv_key;
};
u64 remote_va;
u32 r_key;
@@ -199,7 +199,7 @@ struct bnxt_qplib_swqe {
/* Local Invalidate */
struct {
- u32 inv_l_key;
+ __le32 inv_l_key;
} local_inv;
/* FR-PMR */
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH for-next 1/2] bnxt_re: Fix imm_data endianness
2024-07-09 15:01 ` [PATCH for-next 1/2] bnxt_re: Fix imm_data endianness Jack Wang
@ 2024-07-10 8:27 ` Selvin Xavier
2024-07-10 8:49 ` Jinpu Wang
2024-07-10 9:30 ` Jinpu Wang
0 siblings, 2 replies; 7+ messages in thread
From: Selvin Xavier @ 2024-07-10 8:27 UTC (permalink / raw)
To: Jack Wang; +Cc: linux-rdma, bvanassche, leon, jgg, haris.iqbal
[-- Attachment #1: Type: text/plain, Size: 5113 bytes --]
On Tue, Jul 9, 2024 at 8:31 PM Jack Wang <jinpu.wang@ionos.com> 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@ionos.com>
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.
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);
> 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
> 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
> 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
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4224 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-next 1/2] bnxt_re: Fix imm_data endianness
2024-07-10 8:27 ` Selvin Xavier
@ 2024-07-10 8:49 ` Jinpu Wang
2024-07-10 9:30 ` Jinpu Wang
1 sibling, 0 replies; 7+ messages in thread
From: Jinpu Wang @ 2024-07-10 8:49 UTC (permalink / raw)
To: Selvin Xavier; +Cc: linux-rdma, bvanassche, leon, jgg, haris.iqbal
Hi Selvin,
On Wed, Jul 10, 2024 at 10:28 AM Selvin Xavier
<selvin.xavier@broadcom.com> wrote:
>
> On Tue, Jul 9, 2024 at 8:31 PM Jack Wang <jinpu.wang@ionos.com> 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@ionos.com>
> 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
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-next 1/2] bnxt_re: Fix imm_data endianness
2024-07-10 8:27 ` Selvin Xavier
2024-07-10 8:49 ` Jinpu Wang
@ 2024-07-10 9:30 ` Jinpu Wang
1 sibling, 0 replies; 7+ messages in thread
From: Jinpu Wang @ 2024-07-10 9:30 UTC (permalink / raw)
To: Selvin Xavier; +Cc: linux-rdma, bvanassche, leon, jgg, haris.iqbal
On Wed, Jul 10, 2024 at 10:28 AM Selvin Xavier
<selvin.xavier@broadcom.com> wrote:
>
> On Tue, Jul 9, 2024 at 8:31 PM Jack Wang <jinpu.wang@ionos.com> 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@ionos.com>
> 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.
>
> 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);
I will also drop the second patch, as it is redundant to the above.
> > 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
> > 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
> > 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
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-next 2/2] bnxt_re: Fix inv_key endianness
2024-07-09 15:01 ` [PATCH for-next 2/2] bnxt_re: Fix inv_key endianness Jack Wang
@ 2024-07-11 3:08 ` Greg Sword
0 siblings, 0 replies; 7+ messages in thread
From: Greg Sword @ 2024-07-11 3:08 UTC (permalink / raw)
To: Jack Wang; +Cc: linux-rdma, bvanassche, leon, jgg, selvin.xavier, haris.iqbal
On Tue, Jul 9, 2024 at 11:03 PM Jack Wang <jinpu.wang@ionos.com> wrote:
>
> Similar like previous patch, this change the endianness for inv_key,
> hw expect LE, so change the type accordingly.
Too bad, the commit log. A lot of errors
>
> Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
> Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/infiniband/hw/bnxt_re/ib_verbs.c | 6 +++---
> drivers/infiniband/hw/bnxt_re/qplib_fp.h | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index c5080028247e..cdc8ebcf3a76 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -2483,7 +2483,7 @@ static int bnxt_re_build_send_wqe(struct bnxt_re_qp *qp,
> break;
> case IB_WR_SEND_WITH_INV:
> wqe->type = BNXT_QPLIB_SWQE_TYPE_SEND_WITH_INV;
> - wqe->send.inv_key = wr->ex.invalidate_rkey;
> + wqe->send.inv_key = cpu_to_le32(wr->ex.invalidate_rkey);
> break;
> default:
> return -EINVAL;
> @@ -2513,7 +2513,7 @@ static int bnxt_re_build_rdma_wqe(const struct ib_send_wr *wr,
> break;
> case IB_WR_RDMA_READ:
> wqe->type = BNXT_QPLIB_SWQE_TYPE_RDMA_READ;
> - wqe->rdma.inv_key = wr->ex.invalidate_rkey;
> + wqe->rdma.inv_key = cpu_to_le32(wr->ex.invalidate_rkey);
> break;
> default:
> return -EINVAL;
> @@ -2563,7 +2563,7 @@ static int bnxt_re_build_inv_wqe(const struct ib_send_wr *wr,
> struct bnxt_qplib_swqe *wqe)
> {
> wqe->type = BNXT_QPLIB_SWQE_TYPE_LOCAL_INV;
> - wqe->local_inv.inv_l_key = wr->ex.invalidate_rkey;
> + wqe->local_inv.inv_l_key = cpu_to_le32(wr->ex.invalidate_rkey);
>
> if (wr->send_flags & IB_SEND_SIGNALED)
> wqe->flags |= BNXT_QPLIB_SWQE_FLAGS_SIGNAL_COMP;
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_fp.h b/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> index 1fcaba0f680b..813332b2c872 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> @@ -165,7 +165,7 @@ struct bnxt_qplib_swqe {
> struct {
> union {
> __le32 imm_data;
> - u32 inv_key;
> + __le32 inv_key;
> };
> u32 q_key;
> u32 dst_qp;
> @@ -183,7 +183,7 @@ struct bnxt_qplib_swqe {
> struct {
> union {
> __le32 imm_data;
> - u32 inv_key;
> + __le32 inv_key;
> };
> u64 remote_va;
> u32 r_key;
> @@ -199,7 +199,7 @@ struct bnxt_qplib_swqe {
>
> /* Local Invalidate */
> struct {
> - u32 inv_l_key;
> + __le32 inv_l_key;
> } local_inv;
>
> /* FR-PMR */
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-07-11 3:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-09 15:01 [PATCH for-next 0/2] Fix for bnxt_re endianness issue Jack Wang
2024-07-09 15:01 ` [PATCH for-next 1/2] bnxt_re: Fix imm_data endianness Jack Wang
2024-07-10 8:27 ` Selvin Xavier
2024-07-10 8:49 ` Jinpu Wang
2024-07-10 9:30 ` Jinpu Wang
2024-07-09 15:01 ` [PATCH for-next 2/2] bnxt_re: Fix inv_key endianness Jack Wang
2024-07-11 3:08 ` Greg Sword
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox