From: Mohammad Shuab Siddique <[email protected]>
In bnxt_hwrm_vnic_rss_cfg_hash_mode_p5(), the VF path unconditionally
accessed vnic->fw_grp_ids[0], but fw_grp_ids is NULL when no RSS
contexts have been allocated (num_lb_ctxts == 0). The
bnxt_hwrm_vnic_rss_cfg_p5() call just before it skips its loop when
num_lb_ctxts == 0, so the NULL pointer passed through silently to
this function and caused a segfault on the next RSS hash-level
change after port stop.
A separate HWRM_VNIC_RSS_CFG call to set hash_mode on its own, with
ring_grp_tbl_addr=0 and hash_type=0, is also unsafe on P5/P7 VFs:
firmware treats those zeroes literally and wipes the ring table, so
all traffic lands on queue 0 regardless of the configured RSS level.
Set hash_mode_flags directly in bnxt_hwrm_vnic_rss_cfg_p5() for VFs
on P5/P7, translating DEFAULT to INNERMOST. Firmware now receives one
complete call per context with the ring table, hash type, and hash
mode all set together.
Embedding hash_mode_flags there alone would permanently short-circuit
the separate hash-mode call in bnxt_hwrm_vnic_rss_cfg_hash_mode_p5()
for every VF, silently dropping a trusted VF's explicit RSS
hash-level request (e.g. "rss level-outer") no matter what value was
requested. Firmware's own gate for that separate call (vnic_id ==
0xFFFF && rss_ctx_idx == 0xFFFF) explicitly permits trusted VFs, not
just PFs, to use it. So instead of skipping VFs unconditionally,
bnxt_hwrm_vnic_rss_cfg_hash_mode_p5() now only short-circuits for a
VF that is untrusted or has no initialized RSS context (fw_grp_ids/
num_lb_ctxts), and lets a trusted VF with an initialized context
reach the same call PFs already use. The per-context ring-table-wipe
fix above is unchanged either way.
Fixes: 3fd58de5a903 ("net/bnxt: fix RSS hash mode configuration for VF")
Cc: [email protected]
Signed-off-by: Farah Smith <[email protected]>
Signed-off-by: Mohammad Shuab Siddique <[email protected]>
---
drivers/net/bnxt/bnxt_hwrm.c | 79 +++++++++++++-----------------------
1 file changed, 28 insertions(+), 51 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 1615b36aae..84e2aee863 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -2893,15 +2893,16 @@ bnxt_hwrm_vnic_rss_cfg_p5(struct bnxt *bp, struct
bnxt_vnic_info *vnic)
req.vnic_id = rte_cpu_to_le_16(vnic->fw_vnic_id);
req.hash_type = rte_cpu_to_le_32(bnxt_sanitize_rss_type(bp,
vnic->hash_type));
- /* Update req with vnic ring_select_mode for P7 */
if (BNXT_CHIP_P7(bp))
req.ring_select_mode = vnic->ring_select_mode;
- /* When the vnic_id in the request field is a valid
- * one, the hash_mode_flags in the request field must
- * be set to DEFAULT. And any request to change the
- * default behavior must be done in a separate call
- * to HWRM_VNIC_RSS_CFG by exclusively setting hash
- * mode and vnic_id, rss_ctx_idx to INVALID.
+
+ /* Do not embed hash_mode_flags here for VFs. Doing so, plus
+ * syncing prev_hash_mode below, permanently short-circuits
+ * bnxt_hwrm_vnic_rss_cfg_hash_mode_p5()'s guard for every VF,
+ * so a trusted VF's explicit RSS hash-level request (e.g.
+ * rss level-outer) is silently dropped. Firmware's own gate
+ * (vnic_rss_cfg_cmd()) permits trusted VFs on that call, so
+ * treat VFs the same as PFs here.
*/
req.hash_mode_flags = BNXT_HASH_MODE_DEFAULT;
@@ -2930,70 +2931,46 @@ static int
bnxt_hwrm_vnic_rss_cfg_hash_mode_p5(struct bnxt *bp, struct bnxt_vnic_info
*vnic)
{
struct hwrm_vnic_rss_cfg_output *resp = bp->hwrm_cmd_resp_addr;
- struct hwrm_vnic_rss_cfg_input req = {.req_type = 0 };
+ struct hwrm_vnic_rss_cfg_input req = {.req_type = 0};
int rc = 0;
- /* The reason we are returning success here is that this
- * call is in the context of user/stack RSS configuration.
- * Even though OUTER RSS is not supported, the normal RSS
- * configuration should continue to work.
- */
+ /* Return success when outer RSS is unsupported so normal RSS works. */
if ((BNXT_CHIP_P5(bp) && BNXT_VNIC_OUTER_RSS_UNSUPPORTED(bp)) ||
(!BNXT_CHIP_P5(bp) && !(bp->vnic_cap_flags &
BNXT_VNIC_CAP_OUTER_RSS)))
return 0;
- /* TODO Revisit for Thor 2 */
- /* if (BNXT_CHIP_P5_P7(bp))
- * bnxt_hwrm_vnic_rss_cfg_p5(bp, vnic);
- */
- /* Don't call RSS hash level configuration if the current
- * hash level is the same as the hash level that is requested.
- */
if (vnic->prev_hash_mode == vnic->hash_mode)
return 0;
+ /* Trusted VFs now reach this call the same as PFs; firmware's own
+ * gate (IS_PF_FID() || pcie_func_is_trusted_vf()) restricts which
+ * callers it accepts the request from. Firmware would reject a
+ * request from an untrusted VF, so keep it a silent no-op here
+ * instead of surfacing a new HWRM error.
+ * The fw_grp_ids/num_lb_ctxts check guards against a NULL RSS
+ * context that is not yet initialized or has been torn down by
+ * a port stop.
+ */
+ if (BNXT_VF(bp) &&
+ (!BNXT_VF_IS_TRUSTED(bp) || !vnic->fw_grp_ids ||
!vnic->num_lb_ctxts))
+ return 0;
+
HWRM_PREP(&req, HWRM_VNIC_RSS_CFG, BNXT_USE_CHIMP_MB);
- /* For FW, hash_mode == DEFAULT means that
- * the FW is capable of doing INNER & OUTER RSS as well.
- * DEFAULT doesn't mean that the FW is
- * going to change the hash_mode to INNER. However, for
- * the USER, DEFAULT means, change the hash mode to the
- * NIC's DEFAULT hash mode which is INNER.
- *
- * Hence, driver should make the translation of hash_mode
- * to INNERMOST when hash_mode from the dpdk stack is
- * DEFAULT.
+ /* FW DEFAULT keeps existing hash level; translate to INNERMOST so
+ * the NIC uses its hardware default (inner headers).
*/
if (vnic->hash_mode == BNXT_HASH_MODE_DEFAULT)
req.hash_mode_flags = BNXT_HASH_MODE_INNERMOST;
else
req.hash_mode_flags = vnic->hash_mode;
- /* VFs must use actual vnic_id for per-VNIC configuration.
- * PFs can use INVALID vnic_id for global configuration.
- * This is because VFs don't have permission to configure
- * global hash mode, even if they're trusted.
- */
- if (BNXT_VF(bp)) {
- req.vnic_id = rte_cpu_to_le_16(vnic->fw_vnic_id);
- req.rss_ctx_idx = rte_cpu_to_le_16(vnic->fw_grp_ids[0]);
- PMD_DRV_LOG_LINE(DEBUG, "VF using per-VNIC RSS config
(vnic_id=%u)",
- vnic->fw_vnic_id);
- } else {
- req.vnic_id = rte_cpu_to_le_16(BNXT_DFLT_VNIC_ID_INVALID);
- req.rss_ctx_idx = rte_cpu_to_le_16(BNXT_RSS_CTX_IDX_INVALID);
- PMD_DRV_LOG_LINE(DEBUG, "PF using global RSS config");
- }
-
+ req.vnic_id = rte_cpu_to_le_16(BNXT_DFLT_VNIC_ID_INVALID);
+ req.rss_ctx_idx = rte_cpu_to_le_16(BNXT_RSS_CTX_IDX_INVALID);
PMD_DRV_LOG_LINE(DEBUG, "RSS CFG: Hash level %d", req.hash_mode_flags);
- rc = bnxt_hwrm_send_message(bp, &req, sizeof(req),
- BNXT_USE_CHIMP_MB);
+ rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
HWRM_CHECK_RESULT();
- /* Store the programmed hash_mode in prev_hash_mode so that
- * it can checked against the next user requested hash mode.
- */
if (!rc)
vnic->prev_hash_mode = vnic->hash_mode;
HWRM_UNLOCK();
--
2.47.3
