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