From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 326B123EA8B for ; Mon, 2 Feb 2026 12:17:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770034634; cv=none; b=qeJjmXzwWG5iZBJOvuqxOgBwVzyikfzYZesJO86daC5Cy/q3lVbFe2hX6D0CI8YB2YtVScOEQx7u3vTeTjSfcVKRfs6sRaO5koeWJc9DfUBEPh81TAqT6T6WMyJ6ZgKEUZ0EJ2cUVW5glqDwDYvigFrKYLnv03OshxN1cwybYgo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770034634; c=relaxed/simple; bh=/Nwl5ZllXu/6hoWJdgfjmTR+MjI/9d47hFN7D5G7Pjs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ox/2eM7MsxlyK7h70cC3NxjiuzZYt2UaCswsvq9aQHx0bxwfGEU/GJ7/SgOF1wX+IKV3sDwpTOLP2fLKwGjoGpZTlkJL0L3dteGW40jKjuNG85sLA64g5Z7wy++HHBHCToZBLqbtJB600bkimG/jXX7S6ZLcQF1C+EP2DtMZ8+Q= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=isimbHWP; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="isimbHWP" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 01253C116C6; Mon, 2 Feb 2026 12:17:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1770034633; bh=/Nwl5ZllXu/6hoWJdgfjmTR+MjI/9d47hFN7D5G7Pjs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=isimbHWPCPOrzUDMDcMvE5whhCxvVgMh/BvzolJxOTDVB+MbdzH2FMA8J8rMrJz2Q tSvbbdb+KEI1B8V36xJwVGmg/G015BPvoz/fkQUZfYNxoYLTmX/3J/1s+UGMLhENE5 xjkIwn7/UCNs/tCsNAmVEHFbS8DctvAlSx8qLfpl2Xf1fz8KVEiapz1HgK3ol8Uqzq Dr3qU4bIWImLHtCGQ4/kuw8kKEnfRD6pJj6o/sD2XhLcBylgZFfJpQwT8pJt9HsHpp uPEHFKX++0n2w3tYsCKQXy2++7UH6+mv1bG3xGJc2L2CoSVJXs5Q1KnznmAUOJBWDa s+J71xITCTH5g== Date: Mon, 2 Feb 2026 14:17:07 +0200 From: Leon Romanovsky To: Kalesh AP Cc: jgg@ziepe.ca, linux-rdma@vger.kernel.org, andrew.gospodarek@broadcom.com, selvin.xavier@broadcom.com, Damodharam Ammepalli , Hongguang Gao Subject: Re: [PATCH rdma-rext V3 1/5] RDMA/bnxt_re: Add support for QP rate limiting Message-ID: <20260202121707.GI34749@unreal> References: <20260202045120.3139712-1-kalesh-anakkur.purayil@broadcom.com> <20260202045120.3139712-2-kalesh-anakkur.purayil@broadcom.com> Precedence: bulk X-Mailing-List: linux-rdma@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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 > Reviewed-by: Hongguang Gao > Signed-off-by: Kalesh AP > --- > 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 >