public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
* [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-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-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: 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