Re: [PATCH rdma-next] RDMA/bnxt_re: Limit SRQ depth to the firmware command field

[Leon Romanovsky <leon@kernel.org>]

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