* [PATCH] RDMA/bnxt_re: Fix IB_SEND_IP_CSUM handling in post_send
@ 2025-12-19 9:32 Alok Tiwari
2025-12-21 9:42 ` Leon Romanovsky
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Alok Tiwari @ 2025-12-19 9:32 UTC (permalink / raw)
To: somnath.kotur, sriharsha.basavapatna, leon, jgg,
kalesh-anakkur.purayil, selvin.xavier, linux-rdma
Cc: alok.a.tiwarilinux, alok.a.tiwari
The bnxt_re SEND path checks wr->send_flags to enable features such as
IP checksum offload. However, send_flags is a bitmask and may contain
multiple flags (e.g. IB_SEND_SIGNALED | IB_SEND_IP_CSUM), while the
existing code uses a switch() statement that only matches when
send_flags is exactly IB_SEND_IP_CSUM.
As a result, checksum offload is not enabled when additional SEND
flags are present.
Replace the switch() with a bitmask test:
if (wr->send_flags & IB_SEND_IP_CSUM)
This ensures IP checksum offload is enabled correctly when multiple
SEND flags are used.
Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
---
drivers/infiniband/hw/bnxt_re/ib_verbs.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index f19b55c13d58..ff91511bd338 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -2919,14 +2919,9 @@ int bnxt_re_post_send(struct ib_qp *ib_qp, const struct ib_send_wr *wr,
wqe.rawqp1.lflags |=
SQ_SEND_RAWETH_QP1_LFLAGS_ROCE_CRC;
}
- switch (wr->send_flags) {
- case IB_SEND_IP_CSUM:
+ if (wr->send_flags & IB_SEND_IP_CSUM)
wqe.rawqp1.lflags |=
SQ_SEND_RAWETH_QP1_LFLAGS_IP_CHKSUM;
- break;
- default:
- break;
- }
fallthrough;
case IB_WR_SEND_WITH_INV:
rc = bnxt_re_build_send_wqe(qp, wr, &wqe);
--
2.50.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] RDMA/bnxt_re: Fix IB_SEND_IP_CSUM handling in post_send
2025-12-19 9:32 [PATCH] RDMA/bnxt_re: Fix IB_SEND_IP_CSUM handling in post_send Alok Tiwari
@ 2025-12-21 9:42 ` Leon Romanovsky
2025-12-21 16:03 ` ALOK TIWARI
2025-12-21 15:56 ` Kalesh Anakkur Purayil
2025-12-22 8:04 ` Leon Romanovsky
2 siblings, 1 reply; 5+ messages in thread
From: Leon Romanovsky @ 2025-12-21 9:42 UTC (permalink / raw)
To: Alok Tiwari
Cc: somnath.kotur, sriharsha.basavapatna, jgg, kalesh-anakkur.purayil,
selvin.xavier, linux-rdma, alok.a.tiwarilinux
On Fri, Dec 19, 2025 at 01:32:57AM -0800, Alok Tiwari wrote:
> The bnxt_re SEND path checks wr->send_flags to enable features such as
> IP checksum offload. However, send_flags is a bitmask and may contain
> multiple flags (e.g. IB_SEND_SIGNALED | IB_SEND_IP_CSUM), while the
> existing code uses a switch() statement that only matches when
> send_flags is exactly IB_SEND_IP_CSUM.
>
> As a result, checksum offload is not enabled when additional SEND
> flags are present.
>
> Replace the switch() with a bitmask test:
>
> if (wr->send_flags & IB_SEND_IP_CSUM)
>
> This ensures IP checksum offload is enabled correctly when multiple
> SEND flags are used.
>
> Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
> Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> ---
> drivers/infiniband/hw/bnxt_re/ib_verbs.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index f19b55c13d58..ff91511bd338 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -2919,14 +2919,9 @@ int bnxt_re_post_send(struct ib_qp *ib_qp, const struct ib_send_wr *wr,
> wqe.rawqp1.lflags |=
> SQ_SEND_RAWETH_QP1_LFLAGS_ROCE_CRC;
> }
> - switch (wr->send_flags) {
> - case IB_SEND_IP_CSUM:
> + if (wr->send_flags & IB_SEND_IP_CSUM)
> wqe.rawqp1.lflags |=
> SQ_SEND_RAWETH_QP1_LFLAGS_IP_CHKSUM;
> - break;
> - default:
> - break;
> - }
> fallthrough;
The combination of "default with break" and "fallthrough" doesn't make
any sense. Are you sure that we should keep "fallthrough"?
Thanks
> case IB_WR_SEND_WITH_INV:
> rc = bnxt_re_build_send_wqe(qp, wr, &wqe);
> --
> 2.50.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] RDMA/bnxt_re: Fix IB_SEND_IP_CSUM handling in post_send
2025-12-21 9:42 ` Leon Romanovsky
@ 2025-12-21 16:03 ` ALOK TIWARI
0 siblings, 0 replies; 5+ messages in thread
From: ALOK TIWARI @ 2025-12-21 16:03 UTC (permalink / raw)
To: Leon Romanovsky
Cc: somnath.kotur, sriharsha.basavapatna, jgg, kalesh-anakkur.purayil,
selvin.xavier, linux-rdma, alok.a.tiwarilinux
On 12/21/2025 3:12 PM, Leon Romanovsky wrote:
>> - switch (wr->send_flags) {
>> - case IB_SEND_IP_CSUM:
>> + if (wr->send_flags & IB_SEND_IP_CSUM)
>> wqe.rawqp1.lflags |=
>> SQ_SEND_RAWETH_QP1_LFLAGS_IP_CHKSUM;
>> - break;
>> - default:
>> - break;
>> - }
>> fallthrough;
> The combination of "default with break" and "fallthrough" doesn't make
> any sense. Are you sure that we should keep "fallthrough"?
>
> Thanks
I removed the break.
For clarity, in the old code the break only exited the inner switch
(wr->send_flags):
switch (wr->send_flags) {
case IB_SEND_IP_CSUM:
...
break;
default:
break;
}
fallthrough;
Control then continued to the opcode fallthrough. The fallthrough is
required to keep the same behavior as before. IB_WR_SEND,
IB_WR_SEND_WITH_IMM, and IB_WR_SEND_WITH_INV all ultimately require
bnxt_re_build_send_wqe() to build the common SEND WQE, which is the core
SEND WQE builder.
Thanks,
Alok
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RDMA/bnxt_re: Fix IB_SEND_IP_CSUM handling in post_send
2025-12-19 9:32 [PATCH] RDMA/bnxt_re: Fix IB_SEND_IP_CSUM handling in post_send Alok Tiwari
2025-12-21 9:42 ` Leon Romanovsky
@ 2025-12-21 15:56 ` Kalesh Anakkur Purayil
2025-12-22 8:04 ` Leon Romanovsky
2 siblings, 0 replies; 5+ messages in thread
From: Kalesh Anakkur Purayil @ 2025-12-21 15:56 UTC (permalink / raw)
To: Alok Tiwari
Cc: somnath.kotur, sriharsha.basavapatna, leon, jgg, selvin.xavier,
linux-rdma, alok.a.tiwarilinux
[-- Attachment #1: Type: text/plain, Size: 2352 bytes --]
On Fri, Dec 19, 2025 at 3:03 PM Alok Tiwari <alok.a.tiwari@oracle.com> wrote:
>
> The bnxt_re SEND path checks wr->send_flags to enable features such as
> IP checksum offload. However, send_flags is a bitmask and may contain
> multiple flags (e.g. IB_SEND_SIGNALED | IB_SEND_IP_CSUM), while the
> existing code uses a switch() statement that only matches when
> send_flags is exactly IB_SEND_IP_CSUM.
>
> As a result, checksum offload is not enabled when additional SEND
> flags are present.
>
> Replace the switch() with a bitmask test:
>
> if (wr->send_flags & IB_SEND_IP_CSUM)
>
> This ensures IP checksum offload is enabled correctly when multiple
> SEND flags are used.
>
> Fixes: 1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
> Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> ---
> drivers/infiniband/hw/bnxt_re/ib_verbs.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index f19b55c13d58..ff91511bd338 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -2919,14 +2919,9 @@ int bnxt_re_post_send(struct ib_qp *ib_qp, const struct ib_send_wr *wr,
> wqe.rawqp1.lflags |=
> SQ_SEND_RAWETH_QP1_LFLAGS_ROCE_CRC;
> }
> - switch (wr->send_flags) {
> - case IB_SEND_IP_CSUM:
> + if (wr->send_flags & IB_SEND_IP_CSUM)
> wqe.rawqp1.lflags |=
> SQ_SEND_RAWETH_QP1_LFLAGS_IP_CHKSUM;
> - break;
> - default:
> - break;
> - }
> fallthrough;
> case IB_WR_SEND_WITH_INV:
> rc = bnxt_re_build_send_wqe(qp, wr, &wqe);
> --
> 2.50.1
>
Thank you Alok for the patch and the changes look good to me.
Just curious to know, how did you identify this issue? Is that through
a code review or did any test cases fail for you?
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
--
Regards,
Kalesh AP
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5509 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] RDMA/bnxt_re: Fix IB_SEND_IP_CSUM handling in post_send
2025-12-19 9:32 [PATCH] RDMA/bnxt_re: Fix IB_SEND_IP_CSUM handling in post_send Alok Tiwari
2025-12-21 9:42 ` Leon Romanovsky
2025-12-21 15:56 ` Kalesh Anakkur Purayil
@ 2025-12-22 8:04 ` Leon Romanovsky
2 siblings, 0 replies; 5+ messages in thread
From: Leon Romanovsky @ 2025-12-22 8:04 UTC (permalink / raw)
To: somnath.kotur, sriharsha.basavapatna, jgg, kalesh-anakkur.purayil,
selvin.xavier, linux-rdma, Alok Tiwari
Cc: alok.a.tiwarilinux
On Fri, 19 Dec 2025 01:32:57 -0800, Alok Tiwari wrote:
> The bnxt_re SEND path checks wr->send_flags to enable features such as
> IP checksum offload. However, send_flags is a bitmask and may contain
> multiple flags (e.g. IB_SEND_SIGNALED | IB_SEND_IP_CSUM), while the
> existing code uses a switch() statement that only matches when
> send_flags is exactly IB_SEND_IP_CSUM.
>
> As a result, checksum offload is not enabled when additional SEND
> flags are present.
>
> [...]
Applied, thanks!
[1/1] RDMA/bnxt_re: Fix IB_SEND_IP_CSUM handling in post_send
https://git.kernel.org/rdma/rdma/c/f01765a2361323
Best regards,
--
Leon Romanovsky <leon@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-12-22 8:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-19 9:32 [PATCH] RDMA/bnxt_re: Fix IB_SEND_IP_CSUM handling in post_send Alok Tiwari
2025-12-21 9:42 ` Leon Romanovsky
2025-12-21 16:03 ` ALOK TIWARI
2025-12-21 15:56 ` Kalesh Anakkur Purayil
2025-12-22 8:04 ` Leon Romanovsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox