Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* [PATCH rdma-next] RDMA/bnxt_re: Limit SRQ depth to the firmware command field
@ 2026-09-15 13:09 Leon Romanovsky
  2026-09-15 13:24 ` sashiko-bot
  0 siblings, 1 reply; 5+ messages in thread
From: Leon Romanovsky @ 2026-09-15 13:09 UTC (permalink / raw)
  To: Selvin Xavier, Kalesh AP, Jason Gunthorpe, Leon Romanovsky,
	Doug Ledford, Devesh Sharma
  Cc: linux-rdma, linux-kernel

From: Leon Romanovsky <leonro@nvidia.com>

Firmware can report an SRQ depth above 16 bits, but CREATE_SRQ carries the
allocated depth in a 16-bit field. Requests near the advertised limit can
therefore truncate the command value and make firmware use a different
depth than the allocated queue.

Cap usable work requests so the reserved empty-ring entry still fits in the
command field. Accept the capped maximum and correct the associated
diagnostic.

Fixes: 37cb11acf1f7 ("RDMA/bnxt_re: Add SRQ support for Broadcom adapters")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 4 ++--
 drivers/infiniband/hw/bnxt_re/qplib_sp.c | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index ef08d42f377e..43ff3e7a459f 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -2229,8 +2229,8 @@ int bnxt_re_create_srq(struct ib_srq *ib_srq,
 	dev_attr = rdev->dev_attr;
 	srq = container_of(ib_srq, struct bnxt_re_srq, ib_srq);
 
-	if (srq_init_attr->attr.max_wr >= dev_attr->max_srq_wqes) {
-		ibdev_err(&rdev->ibdev, "Create CQ failed - max exceeded");
+	if (srq_init_attr->attr.max_wr > dev_attr->max_srq_wqes) {
+		ibdev_err(&rdev->ibdev, "Create SRQ failed - max exceeded");
 		rc = -EINVAL;
 		goto exit;
 	}
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.c b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
index ec9eb52a8ebf..63dcbb9d80f4 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_sp.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
@@ -159,7 +159,8 @@ int bnxt_qplib_get_dev_attr(struct bnxt_qplib_rcfw *rcfw)
 	attr->max_ah = le32_to_cpu(sb->max_ah);
 
 	attr->max_srq = le16_to_cpu(sb->max_srq);
-	attr->max_srq_wqes = le32_to_cpu(sb->max_srq_wr) - 1;
+	attr->max_srq_wqes = min_t(u32, le32_to_cpu(sb->max_srq_wr),
+				   U16_MAX) - 1;
 	attr->max_srq_sges = sb->max_srq_sge;
 	attr->max_pkey = 1;
 	attr->max_inline_data = attr->max_qp_sges * sizeof(struct sq_sge);

---
base-commit: f7d28791433d373ecdab7b00f60ddb504e858e0a
change-id: 20260915-silent-16-bit-truncation-of-srq-size-a1495f0bf133

Best regards,
--  
Leon Romanovsky <leonro@nvidia.com>


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH rdma-next] RDMA/bnxt_re: Limit SRQ depth to the firmware command field
  2026-09-15 13:09 [PATCH rdma-next] RDMA/bnxt_re: Limit SRQ depth to the firmware command field Leon Romanovsky
@ 2026-09-15 13:24 ` sashiko-bot
  2026-09-15 18:46   ` Leon Romanovsky
  0 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-09-15 13:24 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: linux-rdma

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] RDMA/bnxt_re: silent SRQ desync for legacy userspace due to non-power-of-two cap
--

commit a0fff360dfd527e4f635aa0d8696bce687733ce9
Author: Leon Romanovsky <leon@kernel.org>

RDMA/bnxt_re: Limit SRQ depth to the firmware command field

Firmware can report an SRQ depth above 16 bits, but CREATE_SRQ carries the
allocated depth in a 16-bit field. Requests near the advertised limit can
therefore truncate the command value and make firmware use a different
depth than the allocated queue.

> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index ef08d42f377e6..43ff3e7a459fa 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -2229,8 +2229,8 @@ int bnxt_re_create_srq(struct ib_srq *ib_srq,
>  	dev_attr = rdev->dev_attr;
>  	srq = container_of(ib_srq, struct bnxt_re_srq, ib_srq);
>  
> -	if (srq_init_attr->attr.max_wr >= dev_attr->max_srq_wqes) {
> -		ibdev_err(&rdev->ibdev, "Create CQ failed - max exceeded");
> +	if (srq_init_attr->attr.max_wr > dev_attr->max_srq_wqes) {
> +		ibdev_err(&rdev->ibdev, "Create SRQ failed - max exceeded");
>  		rc = -EINVAL;
>  		goto exit;
>  	}
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.c b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> index ec9eb52a8ebf7..63dcbb9d80f4b 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> @@ -159,7 +159,8 @@ int bnxt_qplib_get_dev_attr(struct bnxt_qplib_rcfw *rcfw)
>  	attr->max_ah = le32_to_cpu(sb->max_ah);
>  
>  	attr->max_srq = le16_to_cpu(sb->max_srq);
> -	attr->max_srq_wqes = le32_to_cpu(sb->max_srq_wr) - 1;
> +	attr->max_srq_wqes = min_t(u32, le32_to_cpu(sb->max_srq_wr),
> +				   U16_MAX) - 1;

[Severity: High]
Does capping max_srq_wqes to 65534 break legacy userspace applications that
rely on power-of-two queue sizes?

If a legacy application requests an SRQ size that rounds up to 65536
(e.g. 32768), bnxt_re_create_srq() will pass dev_attr->max_srq_wqes + 1
(65535) as the maximum to bnxt_re_init_depth():

drivers/infiniband/hw/bnxt_re/ib_verbs.c:bnxt_re_create_srq() {
    ...
	srq->qplib_srq.max_wqe =
		bnxt_re_init_depth(srq_init_attr->attr.max_wr + 1,
				   dev_attr->max_srq_wqes + 1, uctx);
    ...
}

In bnxt_re_init_depth(), this results in min(65536, 65535) and returns 65535:

drivers/infiniband/hw/bnxt_re/ib_verbs.h:bnxt_re_init_depth() {
    ...
	if (uctx && !(uctx->cmask & BNXT_RE_UCNTX_CAP_POW2_DISABLED))
		return min(roundup_pow_of_two(ent), max);
    ...
}

Because 65535 is not a power of two, legacy userspace masking (e.g., idx &
0xFFFE) becomes mathematically incorrect, immediately corrupting ring state
and causing producer/consumer desynchronization without throwing an error.

>  	attr->max_srq_sges = sb->max_srq_sge;
>  	attr->max_pkey = 1;
>  	attr->max_inline_data = attr->max_qp_sges * sizeof(struct sq_sge);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-silent-16-bit-truncation-of-srq-size-v1-1-3199f8fe94bc@nvidia.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH rdma-next] RDMA/bnxt_re: Limit SRQ depth to the firmware command field
  2026-09-15 13:24 ` sashiko-bot
@ 2026-09-15 18:46   ` Leon Romanovsky
  2026-09-16  8:06     ` Kalesh Anakkur Purayil
  0 siblings, 1 reply; 5+ messages in thread
From: Leon Romanovsky @ 2026-09-15 18:46 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-rdma

On Tue, Sep 15, 2026 at 01:24:27PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] RDMA/bnxt_re: silent SRQ desync for legacy userspace due to non-power-of-two cap
> --
> 
> commit a0fff360dfd527e4f635aa0d8696bce687733ce9
> Author: Leon Romanovsky <leon@kernel.org>
> 
> RDMA/bnxt_re: Limit SRQ depth to the firmware command field
> 
> Firmware can report an SRQ depth above 16 bits, but CREATE_SRQ carries the
> allocated depth in a 16-bit field. Requests near the advertised limit can
> therefore truncate the command value and make firmware use a different
> depth than the allocated queue.
> 
> > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > index ef08d42f377e6..43ff3e7a459fa 100644
> > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > @@ -2229,8 +2229,8 @@ int bnxt_re_create_srq(struct ib_srq *ib_srq,
> >  	dev_attr = rdev->dev_attr;
> >  	srq = container_of(ib_srq, struct bnxt_re_srq, ib_srq);
> >  
> > -	if (srq_init_attr->attr.max_wr >= dev_attr->max_srq_wqes) {
> > -		ibdev_err(&rdev->ibdev, "Create CQ failed - max exceeded");
> > +	if (srq_init_attr->attr.max_wr > dev_attr->max_srq_wqes) {
> > +		ibdev_err(&rdev->ibdev, "Create SRQ failed - max exceeded");
> >  		rc = -EINVAL;
> >  		goto exit;
> >  	}
> > diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.c b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> > index ec9eb52a8ebf7..63dcbb9d80f4b 100644
> > --- a/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> > +++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> > @@ -159,7 +159,8 @@ int bnxt_qplib_get_dev_attr(struct bnxt_qplib_rcfw *rcfw)
> >  	attr->max_ah = le32_to_cpu(sb->max_ah);
> >  
> >  	attr->max_srq = le16_to_cpu(sb->max_srq);
> > -	attr->max_srq_wqes = le32_to_cpu(sb->max_srq_wr) - 1;
> > +	attr->max_srq_wqes = min_t(u32, le32_to_cpu(sb->max_srq_wr),
> > +				   U16_MAX) - 1;
> 
> [Severity: High]
> Does capping max_srq_wqes to 65534 break legacy userspace applications that
> rely on power-of-two queue sizes?

Sashiko has a point, the power-of-two is negotiated with
BNXT_RE_UCNTX_CMASK_POW2_DISABLED bit.

Thanks

> 
> If a legacy application requests an SRQ size that rounds up to 65536
> (e.g. 32768), bnxt_re_create_srq() will pass dev_attr->max_srq_wqes + 1
> (65535) as the maximum to bnxt_re_init_depth():
> 
> drivers/infiniband/hw/bnxt_re/ib_verbs.c:bnxt_re_create_srq() {
>     ...
> 	srq->qplib_srq.max_wqe =
> 		bnxt_re_init_depth(srq_init_attr->attr.max_wr + 1,
> 				   dev_attr->max_srq_wqes + 1, uctx);
>     ...
> }
> 
> In bnxt_re_init_depth(), this results in min(65536, 65535) and returns 65535:
> 
> drivers/infiniband/hw/bnxt_re/ib_verbs.h:bnxt_re_init_depth() {
>     ...
> 	if (uctx && !(uctx->cmask & BNXT_RE_UCNTX_CAP_POW2_DISABLED))
> 		return min(roundup_pow_of_two(ent), max);
>     ...
> }
> 
> Because 65535 is not a power of two, legacy userspace masking (e.g., idx &
> 0xFFFE) becomes mathematically incorrect, immediately corrupting ring state
> and causing producer/consumer desynchronization without throwing an error.
> 
> >  	attr->max_srq_sges = sb->max_srq_sge;
> >  	attr->max_pkey = 1;
> >  	attr->max_inline_data = attr->max_qp_sges * sizeof(struct sq_sge);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260915-silent-16-bit-truncation-of-srq-size-v1-1-3199f8fe94bc@nvidia.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH rdma-next] RDMA/bnxt_re: Limit SRQ depth to the firmware command field
  2026-09-15 18:46   ` Leon Romanovsky
@ 2026-09-16  8:06     ` Kalesh Anakkur Purayil
  2026-09-16  8:16       ` Leon Romanovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Kalesh Anakkur Purayil @ 2026-09-16  8:06 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: sashiko-reviews, linux-rdma


[-- Attachment #1.1: Type: text/plain, Size: 4200 bytes --]

Hi Leon,

Thank you for the patch. We recently made a similar fix in the OOB driver
(which is queued to push upstream).

We will check the Sashiko report on your patch and propose a fix.

On Wed, Sep 16, 2026 at 1:03 AM Leon Romanovsky <leon@kernel.org> wrote:

> On Tue, Sep 15, 2026 at 01:24:27PM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> > - [High] RDMA/bnxt_re: silent SRQ desync for legacy userspace due to
> non-power-of-two cap
> > --
> >
> > commit a0fff360dfd527e4f635aa0d8696bce687733ce9
> > Author: Leon Romanovsky <leon@kernel.org>
> >
> > RDMA/bnxt_re: Limit SRQ depth to the firmware command field
> >
> > Firmware can report an SRQ depth above 16 bits, but CREATE_SRQ carries
> the
> > allocated depth in a 16-bit field. Requests near the advertised limit can
> > therefore truncate the command value and make firmware use a different
> > depth than the allocated queue.
> >
> > > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > index ef08d42f377e6..43ff3e7a459fa 100644
> > > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > > @@ -2229,8 +2229,8 @@ int bnxt_re_create_srq(struct ib_srq *ib_srq,
> > >     dev_attr = rdev->dev_attr;
> > >     srq = container_of(ib_srq, struct bnxt_re_srq, ib_srq);
> > >
> > > -   if (srq_init_attr->attr.max_wr >= dev_attr->max_srq_wqes) {
> > > -           ibdev_err(&rdev->ibdev, "Create CQ failed - max exceeded");
> > > +   if (srq_init_attr->attr.max_wr > dev_attr->max_srq_wqes) {
> > > +           ibdev_err(&rdev->ibdev, "Create SRQ failed - max
> exceeded");
> > >             rc = -EINVAL;
> > >             goto exit;
> > >     }
> > > diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> > > index ec9eb52a8ebf7..63dcbb9d80f4b 100644
> > > --- a/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> > > +++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> > > @@ -159,7 +159,8 @@ int bnxt_qplib_get_dev_attr(struct bnxt_qplib_rcfw
> *rcfw)
> > >     attr->max_ah = le32_to_cpu(sb->max_ah);
> > >
> > >     attr->max_srq = le16_to_cpu(sb->max_srq);
> > > -   attr->max_srq_wqes = le32_to_cpu(sb->max_srq_wr) - 1;
> > > +   attr->max_srq_wqes = min_t(u32, le32_to_cpu(sb->max_srq_wr),
> > > +                              U16_MAX) - 1;
> >
> > [Severity: High]
> > Does capping max_srq_wqes to 65534 break legacy userspace applications
> that
> > rely on power-of-two queue sizes?
>
> Sashiko has a point, the power-of-two is negotiated with
> BNXT_RE_UCNTX_CMASK_POW2_DISABLED bit.
>
> Thanks
>
> >
> > If a legacy application requests an SRQ size that rounds up to 65536
> > (e.g. 32768), bnxt_re_create_srq() will pass dev_attr->max_srq_wqes + 1
> > (65535) as the maximum to bnxt_re_init_depth():
> >
> > drivers/infiniband/hw/bnxt_re/ib_verbs.c:bnxt_re_create_srq() {
> >     ...
> >       srq->qplib_srq.max_wqe =
> >               bnxt_re_init_depth(srq_init_attr->attr.max_wr + 1,
> >                                  dev_attr->max_srq_wqes + 1, uctx);
> >     ...
> > }
> >
> > In bnxt_re_init_depth(), this results in min(65536, 65535) and returns
> 65535:
> >
> > drivers/infiniband/hw/bnxt_re/ib_verbs.h:bnxt_re_init_depth() {
> >     ...
> >       if (uctx && !(uctx->cmask & BNXT_RE_UCNTX_CAP_POW2_DISABLED))
> >               return min(roundup_pow_of_two(ent), max);
> >     ...
> > }
> >
> > Because 65535 is not a power of two, legacy userspace masking (e.g., idx
> &
> > 0xFFFE) becomes mathematically incorrect, immediately corrupting ring
> state
> > and causing producer/consumer desynchronization without throwing an
> error.
> >
> > >     attr->max_srq_sges = sb->max_srq_sge;
> > >     attr->max_pkey = 1;
> > >     attr->max_inline_data = attr->max_qp_sges * sizeof(struct sq_sge);
> >
> > --
> > Sashiko AI review ·
> https://sashiko.dev/#/patchset/20260915-silent-16-bit-truncation-of-srq-size-v1-1-3199f8fe94bc@nvidia.com?part=1
>
>

-- 
Regards,
Kalesh AP

[-- Attachment #1.2: Type: text/html, Size: 5653 bytes --]

[-- 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-next] RDMA/bnxt_re: Limit SRQ depth to the firmware command field
  2026-09-16  8:06     ` Kalesh Anakkur Purayil
@ 2026-09-16  8:16       ` Leon Romanovsky
  0 siblings, 0 replies; 5+ messages in thread
From: Leon Romanovsky @ 2026-09-16  8:16 UTC (permalink / raw)
  To: Kalesh Anakkur Purayil; +Cc: sashiko-reviews, linux-rdma

On Wed, Sep 16, 2026 at 01:36:28PM +0530, Kalesh Anakkur Purayil wrote:
> Hi Leon,
> 
> Thank you for the patch. We recently made a similar fix in the OOB driver
> (which is queued to push upstream).
> 
> We will check the Sashiko report on your patch and propose a fix.

Thanks

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-16  8:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-15 13:09 [PATCH rdma-next] RDMA/bnxt_re: Limit SRQ depth to the firmware command field Leon Romanovsky
2026-09-15 13:24 ` sashiko-bot
2026-09-15 18:46   ` Leon Romanovsky
2026-09-16  8:06     ` Kalesh Anakkur Purayil
2026-09-16  8:16       ` Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox