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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.