From: Leon Romanovsky <leon@kernel.org>
To: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Cc: jgg@ziepe.ca, linux-rdma@vger.kernel.org,
andrew.gospodarek@broadcom.com, selvin.xavier@broadcom.com,
Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>,
Hongguang Gao <hongguang.gao@broadcom.com>
Subject: Re: [PATCH rdma-rext V3 1/5] RDMA/bnxt_re: Add support for QP rate limiting
Date: Mon, 2 Feb 2026 14:17:07 +0200 [thread overview]
Message-ID: <20260202121707.GI34749@unreal> (raw)
In-Reply-To: <20260202045120.3139712-2-kalesh-anakkur.purayil@broadcom.com>
On Mon, Feb 02, 2026 at 10:21:16AM +0530, Kalesh AP wrote:
> Broadcom P7 chips supports applying rate limit to RC QPs.
> It allows adjust shaper rate values during the INIT -> RTR,
> RTR -> RTS, RTS -> RTS state changes or after QP transitions
> to RTR or RTS.
>
> Signed-off-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
> Reviewed-by: Hongguang Gao <hongguang.gao@broadcom.com>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> ---
> drivers/infiniband/hw/bnxt_re/ib_verbs.c | 11 ++++++++++-
> drivers/infiniband/hw/bnxt_re/qplib_fp.c | 12 +++++++++++-
> drivers/infiniband/hw/bnxt_re/qplib_fp.h | 3 +++
> drivers/infiniband/hw/bnxt_re/qplib_res.h | 6 ++++++
> drivers/infiniband/hw/bnxt_re/qplib_sp.c | 5 +++++
> drivers/infiniband/hw/bnxt_re/qplib_sp.h | 2 ++
> drivers/infiniband/hw/bnxt_re/roce_hsi.h | 13 +++++++++----
> 7 files changed, 46 insertions(+), 6 deletions(-)
AI generates the following:
Scenario:
1. User creates an RC QP (ext_modify_flags = 0, zero-initialized)
2. User calls `ib_modify_qp()` with `IB_QP_RATE_LIMIT` → `ext_modify_flags` becomes `0x8`
3. User calls `ib_modify_qp()` **without** `IB_QP_RATE_LIMIT` (e.g., just state change)
4. `modify_flags` is reset to 0, but `ext_modify_flags` still has `0x8`
5. In `bnxt_qplib_modify_qp()`:
- `req.ext_modify_mask = cpu_to_le32(qp->ext_modify_flags);` → sends 0x8 to firmware
- `if (bmask_ext & CMDQ_MODIFY_QP_EXT_MODIFY_MASK_RATE_LIMIT_VALID)` → TRUE
- `req.rate_limit = cpu_to_le32(qp->rate_limit);` → sends stale rate_limit value
6. Firmware receives unintended rate limit modification
**Severity**: This is a functional bug that can cause:
- Unintended rate limiting on subsequent QP modifications
- The stale `rate_limit` value being sent to firmware on every modify_qp call
- Unexpected QP behavior
--------------------------------------------------------------------------------
Is it expected behavior?
Thanks
>
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index f19b55c13d58..2930461be20d 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -2089,7 +2089,7 @@ int bnxt_re_modify_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr,
> unsigned int flags;
> u8 nw_type;
>
> - if (qp_attr_mask & ~IB_QP_ATTR_STANDARD_BITS)
> + if (qp_attr_mask & ~(IB_QP_ATTR_STANDARD_BITS | IB_QP_RATE_LIMIT))
> return -EOPNOTSUPP;
>
> qp->qplib_qp.modify_flags = 0;
> @@ -2129,6 +2129,15 @@ int bnxt_re_modify_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr,
> bnxt_re_unlock_cqs(qp, flags);
> }
> }
> +
> + if (qp_attr_mask & IB_QP_RATE_LIMIT) {
> + if (qp->qplib_qp.type != IB_QPT_RC ||
> + !_is_modify_qp_rate_limit_supported(dev_attr->dev_cap_flags2))
> + return -EOPNOTSUPP;
> + qp->qplib_qp.ext_modify_flags |=
> + CMDQ_MODIFY_QP_EXT_MODIFY_MASK_RATE_LIMIT_VALID;
> + qp->qplib_qp.rate_limit = qp_attr->rate_limit;
> + }
> if (qp_attr_mask & IB_QP_EN_SQD_ASYNC_NOTIFY) {
> qp->qplib_qp.modify_flags |=
> CMDQ_MODIFY_QP_MODIFY_MASK_EN_SQD_ASYNC_NOTIFY;
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_fp.c b/drivers/infiniband/hw/bnxt_re/qplib_fp.c
> index c88f049136fc..3e44311bf939 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_fp.c
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_fp.c
> @@ -1313,8 +1313,8 @@ int bnxt_qplib_modify_qp(struct bnxt_qplib_res *res, struct bnxt_qplib_qp *qp)
> struct bnxt_qplib_cmdqmsg msg = {};
> struct cmdq_modify_qp req = {};
> u16 vlan_pcp_vlan_dei_vlan_id;
> + u32 bmask, bmask_ext;
> u32 temp32[4];
> - u32 bmask;
> int rc;
>
> bnxt_qplib_rcfw_cmd_prep((struct cmdq_base *)&req,
> @@ -1329,9 +1329,16 @@ int bnxt_qplib_modify_qp(struct bnxt_qplib_res *res, struct bnxt_qplib_qp *qp)
> is_optimized_state_transition(qp))
> bnxt_set_mandatory_attributes(res, qp, &req);
> }
> +
> bmask = qp->modify_flags;
> req.modify_mask = cpu_to_le32(qp->modify_flags);
> + bmask_ext = qp->ext_modify_flags;
> + req.ext_modify_mask = cpu_to_le32(qp->ext_modify_flags);
> req.qp_cid = cpu_to_le32(qp->id);
> +
> + if (bmask_ext & CMDQ_MODIFY_QP_EXT_MODIFY_MASK_RATE_LIMIT_VALID)
> + req.rate_limit = cpu_to_le32(qp->rate_limit);
> +
> if (bmask & CMDQ_MODIFY_QP_MODIFY_MASK_STATE) {
> req.network_type_en_sqd_async_notify_new_state =
> (qp->state & CMDQ_MODIFY_QP_NEW_STATE_MASK) |
> @@ -1429,6 +1436,9 @@ int bnxt_qplib_modify_qp(struct bnxt_qplib_res *res, struct bnxt_qplib_qp *qp)
> rc = bnxt_qplib_rcfw_send_message(rcfw, &msg);
> if (rc)
> return rc;
> +
> + if (bmask_ext & CMDQ_MODIFY_QP_EXT_MODIFY_MASK_RATE_LIMIT_VALID)
> + qp->shaper_allocation_status = resp.shaper_allocation_status;
> qp->cur_qp_state = qp->state;
> return 0;
> }
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_fp.h b/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> index 1b414a73b46d..30c3f99be07b 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_fp.h
> @@ -280,6 +280,7 @@ struct bnxt_qplib_qp {
> u8 state;
> u8 cur_qp_state;
> u64 modify_flags;
> + u32 ext_modify_flags;
> u32 max_inline_data;
> u32 mtu;
> u8 path_mtu;
> @@ -346,6 +347,8 @@ struct bnxt_qplib_qp {
> bool is_host_msn_tbl;
> u8 tos_dscp;
> u32 ugid_index;
> + u32 rate_limit;
> + u8 shaper_allocation_status;
> };
>
> #define BNXT_RE_MAX_MSG_SIZE 0x80000000
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_res.h b/drivers/infiniband/hw/bnxt_re/qplib_res.h
> index 2ea3b7f232a3..9a5dcf97b6f4 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_res.h
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_res.h
> @@ -623,4 +623,10 @@ static inline bool _is_max_srq_ext_supported(u16 dev_cap_ext_flags_2)
> return !!(dev_cap_ext_flags_2 & CREQ_QUERY_FUNC_RESP_SB_MAX_SRQ_EXTENDED);
> }
>
> +static inline bool _is_modify_qp_rate_limit_supported(u16 dev_cap_ext_flags2)
> +{
> + return dev_cap_ext_flags2 &
> + CREQ_QUERY_FUNC_RESP_SB_MODIFY_QP_RATE_LIMIT_SUPPORTED;
> +}
> +
> #endif /* __BNXT_QPLIB_RES_H__ */
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.c b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> index 408a34df2667..ec9eb52a8ebf 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.c
> @@ -193,6 +193,11 @@ int bnxt_qplib_get_dev_attr(struct bnxt_qplib_rcfw *rcfw)
> attr->max_dpi = le32_to_cpu(sb->max_dpi);
>
> attr->is_atomic = bnxt_qplib_is_atomic_cap(rcfw);
> +
> + if (_is_modify_qp_rate_limit_supported(attr->dev_cap_flags2)) {
> + attr->rate_limit_min = le16_to_cpu(sb->rate_limit_min);
> + attr->rate_limit_max = le32_to_cpu(sb->rate_limit_max);
> + }
> bail:
> dma_free_coherent(&rcfw->pdev->dev, sbuf.size,
> sbuf.sb, sbuf.dma_addr);
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_sp.h b/drivers/infiniband/hw/bnxt_re/qplib_sp.h
> index 5a45c55c6464..9fadd637cb5b 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_sp.h
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_sp.h
> @@ -76,6 +76,8 @@ struct bnxt_qplib_dev_attr {
> u16 dev_cap_flags;
> u16 dev_cap_flags2;
> u32 max_dpi;
> + u16 rate_limit_min;
> + u32 rate_limit_max;
> };
>
> struct bnxt_qplib_pd {
> diff --git a/drivers/infiniband/hw/bnxt_re/roce_hsi.h b/drivers/infiniband/hw/bnxt_re/roce_hsi.h
> index 99ecd72e72e2..aac338f2afd8 100644
> --- a/drivers/infiniband/hw/bnxt_re/roce_hsi.h
> +++ b/drivers/infiniband/hw/bnxt_re/roce_hsi.h
> @@ -690,10 +690,11 @@ struct cmdq_modify_qp {
> __le32 ext_modify_mask;
> #define CMDQ_MODIFY_QP_EXT_MODIFY_MASK_EXT_STATS_CTX 0x1UL
> #define CMDQ_MODIFY_QP_EXT_MODIFY_MASK_SCHQ_ID_VALID 0x2UL
> + #define CMDQ_MODIFY_QP_EXT_MODIFY_MASK_RATE_LIMIT_VALID 0x8UL
> __le32 ext_stats_ctx_id;
> __le16 schq_id;
> __le16 unused_0;
> - __le32 reserved32;
> + __le32 rate_limit;
> };
>
> /* creq_modify_qp_resp (size:128b/16B) */
> @@ -716,7 +717,8 @@ struct creq_modify_qp_resp {
> #define CREQ_MODIFY_QP_RESP_PINGPONG_PUSH_INDEX_MASK 0xeUL
> #define CREQ_MODIFY_QP_RESP_PINGPONG_PUSH_INDEX_SFT 1
> #define CREQ_MODIFY_QP_RESP_PINGPONG_PUSH_STATE 0x10UL
> - u8 reserved8;
> + u8 shaper_allocation_status;
> + #define CREQ_MODIFY_QP_RESP_SHAPER_ALLOCATED 0x1UL
> __le32 lag_src_mac;
> };
>
> @@ -2179,7 +2181,7 @@ struct creq_query_func_resp {
> u8 reserved48[6];
> };
>
> -/* creq_query_func_resp_sb (size:1088b/136B) */
> +/* creq_query_func_resp_sb (size:1280b/160B) */
> struct creq_query_func_resp_sb {
> u8 opcode;
> #define CREQ_QUERY_FUNC_RESP_SB_OPCODE_QUERY_FUNC 0x83UL
> @@ -2256,12 +2258,15 @@ struct creq_query_func_resp_sb {
> #define CREQ_QUERY_FUNC_RESP_SB_REQ_RETRANSMISSION_SUPPORT_LAST \
> CREQ_QUERY_FUNC_RESP_SB_REQ_RETRANSMISSION_SUPPORT_IQM_MSN_TABLE
> #define CREQ_QUERY_FUNC_RESP_SB_MAX_SRQ_EXTENDED 0x40UL
> + #define CREQ_QUERY_FUNC_RESP_SB_MODIFY_QP_RATE_LIMIT_SUPPORTED 0x400UL
> #define CREQ_QUERY_FUNC_RESP_SB_MIN_RNR_RTR_RTS_OPT_SUPPORTED 0x1000UL
> __le16 max_xp_qp_size;
> __le16 create_qp_batch_size;
> __le16 destroy_qp_batch_size;
> __le16 max_srq_ext;
> - __le64 reserved64;
> + __le16 reserved16;
> + __le16 rate_limit_min;
> + __le32 rate_limit_max;
> };
>
> /* cmdq_set_func_resources (size:448b/56B) */
> --
> 2.43.5
>
next prev parent reply other threads:[~2026-02-02 12:17 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-02 4:51 [PATCH rdma-rext V3 0/5] RDMA/bnxt_re: Add QP rate limit support Kalesh AP
2026-02-02 4:51 ` [PATCH rdma-rext V3 1/5] RDMA/bnxt_re: Add support for QP rate limiting Kalesh AP
2026-02-02 12:17 ` Leon Romanovsky [this message]
2026-02-02 12:51 ` Kalesh Anakkur Purayil
2026-02-02 4:51 ` [PATCH rdma-rext V3 2/5] RDMA/bnxt_re: Report packet pacing capabilities when querying device Kalesh AP
2026-02-02 4:51 ` [PATCH rdma-rext V3 3/5] RDMA/bnxt_re: Report QP rate limit in debugfs Kalesh AP
2026-02-02 4:51 ` [PATCH rdma-rext V3 4/5] RDMA/mlx5: Support rate limit only for Raw Packet QP Kalesh AP
2026-02-02 4:51 ` [PATCH rdma-rext V3 5/5] IB/core: Extend rate limit support for RC QPs Kalesh AP
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260202121707.GI34749@unreal \
--to=leon@kernel.org \
--cc=andrew.gospodarek@broadcom.com \
--cc=damodharam.ammepalli@broadcom.com \
--cc=hongguang.gao@broadcom.com \
--cc=jgg@ziepe.ca \
--cc=kalesh-anakkur.purayil@broadcom.com \
--cc=linux-rdma@vger.kernel.org \
--cc=selvin.xavier@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox