* [PATCH net-next] eth: bnxt: fix backward compatibility with older devices
@ 2023-10-12 22:41 Jakub Kicinski
2023-10-13 3:53 ` Michael Chan
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Kicinski @ 2023-10-12 22:41 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, pabeni, Jakub Kicinski, michael.chan
Recent FW interface update bumped the size of struct hwrm_func_cfg_input
above 128B which is the max some devices support.
Probe on Stratus (BCM957452) with FW 20.8.3.11 fails with:
bnxt_en ...: Unable to reserve tx rings
bnxt_en ...: 2nd rings reservation failed.
bnxt_en ...: Not enough rings available.
Once probe is fixed other error pop up:
bnxt_en ...: Failed to set async event completion ring.
This is because __hwrm_send() rejects requests larger than
bp->hwrm_max_ext_req_len with -E2BIG. Since the driver doesn't
actually access any of the new fields, yet, trim the length.
It should be safe.
Similar workaround exists for backing_store_cfg_input,
although that one mins() to a constant of 256?!
To make debugging easier in the future add a warning
for oversized requests.
Fixes: 754fbf604ff6 ("bnxt_en: Update firmware interface to 1.10.2.171")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: michael.chan@broadcom.com
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 21 +++++++++++++++----
.../net/ethernet/broadcom/bnxt/bnxt_hwrm.c | 2 ++
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 5c2afcd5ce80..10dc680f022e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -5855,6 +5855,19 @@ static int hwrm_ring_alloc_send_msg(struct bnxt *bp,
return rc;
}
+/* Older devices can only support req length of 128.
+ * Requests which don't need fields starting at num_quic_tx_key_ctxs
+ * can use this helper to avoid getting -E2BIG.
+ */
+static int bnxt_hwrm_func_cfg_short_req_init(struct bnxt *bp,
+ struct hwrm_func_cfg_input **req)
+{
+ u32 req_len;
+
+ req_len = min_t(u32, sizeof(**req), bp->hwrm_max_ext_req_len);
+ return __hwrm_req_init(bp, (void **)req, HWRM_FUNC_CFG, req_len);
+}
+
static int bnxt_hwrm_set_async_event_cr(struct bnxt *bp, int idx)
{
int rc;
@@ -5862,7 +5875,7 @@ static int bnxt_hwrm_set_async_event_cr(struct bnxt *bp, int idx)
if (BNXT_PF(bp)) {
struct hwrm_func_cfg_input *req;
- rc = hwrm_req_init(bp, req, HWRM_FUNC_CFG);
+ rc = bnxt_hwrm_func_cfg_short_req_init(bp, &req);
if (rc)
return rc;
@@ -6273,7 +6286,7 @@ __bnxt_hwrm_reserve_pf_rings(struct bnxt *bp, int tx_rings, int rx_rings,
struct hwrm_func_cfg_input *req;
u32 enables = 0;
- if (hwrm_req_init(bp, req, HWRM_FUNC_CFG))
+ if (bnxt_hwrm_func_cfg_short_req_init(bp, &req))
return NULL;
req->fid = cpu_to_le16(0xffff);
@@ -8618,7 +8631,7 @@ static int bnxt_hwrm_set_br_mode(struct bnxt *bp, u16 br_mode)
else
return -EINVAL;
- rc = hwrm_req_init(bp, req, HWRM_FUNC_CFG);
+ rc = bnxt_hwrm_func_cfg_short_req_init(bp, &req);
if (rc)
return rc;
@@ -8636,7 +8649,7 @@ static int bnxt_hwrm_set_cache_line_size(struct bnxt *bp, int size)
if (BNXT_VF(bp) || bp->hwrm_spec_code < 0x10803)
return 0;
- rc = hwrm_req_init(bp, req, HWRM_FUNC_CFG);
+ rc = bnxt_hwrm_func_cfg_short_req_init(bp, &req);
if (rc)
return rc;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
index 132442f16fe6..1df3d56cc4b5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
@@ -485,6 +485,8 @@ static int __hwrm_send(struct bnxt *bp, struct bnxt_hwrm_ctx *ctx)
if (msg_len > BNXT_HWRM_MAX_REQ_LEN &&
msg_len > bp->hwrm_max_ext_req_len) {
+ netdev_warn(bp->dev, "oversized hwrm request, req_type 0x%x",
+ req_type);
rc = -E2BIG;
goto exit;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net-next] eth: bnxt: fix backward compatibility with older devices
2023-10-12 22:41 [PATCH net-next] eth: bnxt: fix backward compatibility with older devices Jakub Kicinski
@ 2023-10-13 3:53 ` Michael Chan
0 siblings, 0 replies; 2+ messages in thread
From: Michael Chan @ 2023-10-13 3:53 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, edumazet, pabeni
[-- Attachment #1: Type: text/plain, Size: 4758 bytes --]
On Thu, Oct 12, 2023 at 3:41 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Recent FW interface update bumped the size of struct hwrm_func_cfg_input
> above 128B which is the max some devices support.
>
> Probe on Stratus (BCM957452) with FW 20.8.3.11 fails with:
>
> bnxt_en ...: Unable to reserve tx rings
> bnxt_en ...: 2nd rings reservation failed.
> bnxt_en ...: Not enough rings available.
>
> Once probe is fixed other error pop up:
>
> bnxt_en ...: Failed to set async event completion ring.
Thanks for catching this. We need to do more compatibility testing
with older firmware.
>
> This is because __hwrm_send() rejects requests larger than
> bp->hwrm_max_ext_req_len with -E2BIG. Since the driver doesn't
> actually access any of the new fields, yet, trim the length.
> It should be safe.
>
> Similar workaround exists for backing_store_cfg_input,
> although that one mins() to a constant of 256?!
Because the backing store cfg command is supported by relatively newer
firmware that will accept 256 bytes at least.
The HWRM_FUNC_CFG is used quite a bit in bnxt_sriov.c and also in
bnxt_devlink.c. I think we should apply the same treatment to all of
them.
Thanks.
>
> To make debugging easier in the future add a warning
> for oversized requests.
>
> Fixes: 754fbf604ff6 ("bnxt_en: Update firmware interface to 1.10.2.171")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: michael.chan@broadcom.com
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 21 +++++++++++++++----
> .../net/ethernet/broadcom/bnxt/bnxt_hwrm.c | 2 ++
> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 5c2afcd5ce80..10dc680f022e 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -5855,6 +5855,19 @@ static int hwrm_ring_alloc_send_msg(struct bnxt *bp,
> return rc;
> }
>
> +/* Older devices can only support req length of 128.
> + * Requests which don't need fields starting at num_quic_tx_key_ctxs
> + * can use this helper to avoid getting -E2BIG.
> + */
> +static int bnxt_hwrm_func_cfg_short_req_init(struct bnxt *bp,
> + struct hwrm_func_cfg_input **req)
> +{
> + u32 req_len;
> +
> + req_len = min_t(u32, sizeof(**req), bp->hwrm_max_ext_req_len);
> + return __hwrm_req_init(bp, (void **)req, HWRM_FUNC_CFG, req_len);
> +}
> +
> static int bnxt_hwrm_set_async_event_cr(struct bnxt *bp, int idx)
> {
> int rc;
> @@ -5862,7 +5875,7 @@ static int bnxt_hwrm_set_async_event_cr(struct bnxt *bp, int idx)
> if (BNXT_PF(bp)) {
> struct hwrm_func_cfg_input *req;
>
> - rc = hwrm_req_init(bp, req, HWRM_FUNC_CFG);
> + rc = bnxt_hwrm_func_cfg_short_req_init(bp, &req);
> if (rc)
> return rc;
>
> @@ -6273,7 +6286,7 @@ __bnxt_hwrm_reserve_pf_rings(struct bnxt *bp, int tx_rings, int rx_rings,
> struct hwrm_func_cfg_input *req;
> u32 enables = 0;
>
> - if (hwrm_req_init(bp, req, HWRM_FUNC_CFG))
> + if (bnxt_hwrm_func_cfg_short_req_init(bp, &req))
> return NULL;
>
> req->fid = cpu_to_le16(0xffff);
> @@ -8618,7 +8631,7 @@ static int bnxt_hwrm_set_br_mode(struct bnxt *bp, u16 br_mode)
> else
> return -EINVAL;
>
> - rc = hwrm_req_init(bp, req, HWRM_FUNC_CFG);
> + rc = bnxt_hwrm_func_cfg_short_req_init(bp, &req);
> if (rc)
> return rc;
>
> @@ -8636,7 +8649,7 @@ static int bnxt_hwrm_set_cache_line_size(struct bnxt *bp, int size)
> if (BNXT_VF(bp) || bp->hwrm_spec_code < 0x10803)
> return 0;
>
> - rc = hwrm_req_init(bp, req, HWRM_FUNC_CFG);
> + rc = bnxt_hwrm_func_cfg_short_req_init(bp, &req);
> if (rc)
> return rc;
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
> index 132442f16fe6..1df3d56cc4b5 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hwrm.c
> @@ -485,6 +485,8 @@ static int __hwrm_send(struct bnxt *bp, struct bnxt_hwrm_ctx *ctx)
>
> if (msg_len > BNXT_HWRM_MAX_REQ_LEN &&
> msg_len > bp->hwrm_max_ext_req_len) {
> + netdev_warn(bp->dev, "oversized hwrm request, req_type 0x%x",
> + req_type);
> rc = -E2BIG;
> goto exit;
> }
> --
> 2.41.0
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-10-13 3:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-12 22:41 [PATCH net-next] eth: bnxt: fix backward compatibility with older devices Jakub Kicinski
2023-10-13 3:53 ` Michael Chan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).