Re: [PATCH iwl-next v4 13/15] ice: represent RSS LUTs as devlink resources

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] ice: incorrect vsi->rss_size in ice_vsi_cfg_def() on rebuild and ADQ
- [Medium] ice: dual-NAC secondary PFs collide on RSS LUTs and fail probe in Safe Mode
--

commit 299cfc2cd70e5919800a3878f9f0873adb6b516d
Author: Przemek Kitszel <przemyslaw.kitszel@intel.com>

ice: represent RSS LUTs as devlink resources

This commit exposes the PF and global RSS lookup tables of E800 devices as
devlink resources on both the per-PF and whole-device devlink instances,
allowing a PF to switch between its 2048-entry PF LUT and a 512-entry global
LUT.

> diff --git a/drivers/net/ethernet/intel/ice/devlink/resource.c b/drivers/net/ethernet/intel/ice/devlink/resource.c
> new file mode 100644
> index 0000000000000..b0b2b1adae0c1
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ice/devlink/resource.c
> @@ -0,0 +1,662 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026, Intel Corporation. */
> +
> +#include "resource.h"
> +#include "ice_adapter.h"
> +#include "ice.h"
> +#include "ice_lib.h"
> +
> +#define ICE_NUM_GLOBAL_LUTS	16
> +#define ICE_ANY_SLOT		-1
> +
> +/* each NAC numbers its PFs from 0, secondary NAC ones go after primary ones */
> +static int ice_devl_pf_id(struct ice_pf *pf)
> +{
> +	struct ice_hw *hw = &pf->hw;
> +
> +	if (ice_is_primary(hw))
> +		return hw->logical_pf_id;
> +
> +	return hw->logical_pf_id + ICE_PORTS_PER_QUAD;
> +}

[Severity: Medium]
Can secondary-NAC PFs on dual-NAC E825C devices collide with primary-NAC PFs
and fail probe when entering Safe Mode?

In ice_probe(), ice_init_dev_hw() runs before ice_adapter_get() and
ice_init(). When DDP package loading fails or leaves the PF in Safe Mode,
ice_init_dev_hw() calls ice_set_safe_mode_caps():

drivers/net/ethernet/intel/ice/ice_common.c:ice_set_safe_mode_caps() {
	...
	/* unset dev capabilities */
	memset(dev_caps, 0, sizeof(*dev_caps));
	...
}

Because dev_caps->nac_topo.mode is zeroed and not restored by
ice_set_safe_mode_caps(), ice_is_dual(hw) returns false and
ice_is_primary(hw) returns true even on secondary-NAC PFs.

All E825C PFs share a single ice_adapter via ICE_ADAPTER_INDEX_E825C. If the
first PF probes in Safe Mode, ice_adapter_get() -> ice_adapter_init() ->
ice_devl_whole_dev_resources_register() sees ice_is_dual(hw) as false and
sizes the whole-device LUT resources for a single NAC.

Furthermore, during ice_init() -> ice_init_pf_sw() -> ice_pf_vsi_setup() ->
ice_vsi_cfg_def(), ice_take_rss_lut_pf() runs even in Safe Mode. For a
secondary-NAC PF in Safe Mode, ice_devl_pf_id() returns hw->logical_pf_id
instead of hw->logical_pf_id + ICE_PORTS_PER_QUAD, colliding with the
primary-NAC PF's slot in adapter->resources[ICE_RSS_LUT_PF].owner[] and
failing with -ENOSPC.

During the resulting error unwind in ice_init() -> ice_deinit_pf() ->
ice_free_rss_lut_flr() -> ice_free_rss_lut_flr_locked(), the secondary-NAC
PF then clears resources[ICE_RSS_LUT_PF].owner[pf_id] and any global LUTs
matching pf_id, wiping out the primary-NAC PF's LUT resource ownership.

> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index f81b310a98c3e..215244a1c043b 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -986,10 +988,10 @@ static void ice_rss_clean(struct ice_vsi *vsi)
>  }
>  
>  /**
> - * ice_vsi_set_rss_params - Setup RSS capabilities per VSI type
> + * ice_vsi_set_dflt_rss_params - Setup default RSS capabilities per VSI type
>   * @vsi: the VSI being configured
>   */
> -static void ice_vsi_set_rss_params(struct ice_vsi *vsi)
> +static void ice_vsi_set_dflt_rss_params(struct ice_vsi *vsi)
>  {
>  	struct ice_hw_common_caps *cap;
>  	struct ice_pf *pf = vsi->back;
> @@ -1004,6 +1006,9 @@ static void ice_vsi_set_rss_params(struct ice_vsi *vsi)
>  	max_rss_size = BIT(cap->rss_table_entry_width);
>  	switch (vsi->type) {
>  	case ICE_VSI_CHNL:
> +		if (!vsi->num_rxq)
> +			vsi->num_rxq = 1;
> +		fallthrough;
>  	case ICE_VSI_PF:

[ ... ]

> @@ -2427,6 +2464,18 @@ static int ice_vsi_cfg_def(struct ice_vsi *vsi)
>  
>  	vsi->vsw = pf->first_sw;
>  
> +	if (vsi->flags & ICE_VSI_FLAG_INIT) {
> +		scoped_guard(mutex, &pf->rss_lut_lock) {
> +			u16 lut_size = vsi->rss_table_size;
> +
> +			ice_vsi_set_dflt_rss_params(vsi);
> +			if (vsi->rss_table_size != lut_size) {
> +				devm_kfree(dev, vsi->rss_lut_user);
> +				vsi->rss_lut_user = NULL;
> +			}
> +		}
> +	}
> +
>  	ret = ice_vsi_alloc_def(vsi, vsi->ch);
>  	if (ret)
>  		return ret;
> @@ -2446,15 +2495,23 @@ static int ice_vsi_cfg_def(struct ice_vsi *vsi)
>  	}
>  
>  	/* set RSS capabilities */
> -	ice_vsi_set_rss_params(vsi);

[Severity: Medium]
Does moving ice_vsi_set_dflt_rss_params() before ice_vsi_alloc_def() and
gating it on ICE_VSI_FLAG_INIT cause vsi->rss_size to remain clamped at a
smaller queue count on ICE_VSI_CHNL creation and ICE_VSI_FLAG_NO_INIT
rebuilds?

Previously, ice_vsi_cfg_def() called ice_vsi_alloc_def(vsi, vsi->ch) first
and then unconditionally called ice_vsi_set_rss_params(vsi) on both
ICE_VSI_FLAG_INIT and ICE_VSI_FLAG_NO_INIT paths, resetting vsi->rss_size to
its default ceiling before ice_vsi_cfg_rss_lut_key() clamped it via:

drivers/net/ethernet/intel/ice/ice_lib.c:ice_vsi_cfg_rss_lut_key() {
	...
		vsi->rss_size = min_t(u16, vsi->rss_size, vsi->num_rxq);
	...
}

With this change:

1. When a new ICE_VSI_CHNL VSI is created via ice_chnl_vsi_setup() ->
ice_vsi_setup() -> ice_vsi_cfg_def(), ice_vsi_set_dflt_rss_params() now runs
before ice_vsi_alloc_def() assigns vsi->num_rxq = ch->num_rxq. Because
vsi->num_rxq is still 0 at that point, the new fallback sets vsi->num_rxq = 1
and vsi->rss_size = 1. After ice_vsi_alloc_def() sets the actual channel
vsi->num_rxq, ice_vsi_cfg_rss_lut_key() computes min_t(u16, 1, vsi->num_rxq),
leaving the channel VSI's rss_size stuck at 1.

2. When ADQ is configured in ice_setup_tc_mqprio_qdisc(), main_vsi->orig_rss_size
saves the pre-ADQ rss_size and main_vsi->rss_size is clamped to
main_vsi->ch_rss_size. If a PF reset (ICE_RESET_PFR) occurs while ADQ is
active, ice_prepare_for_reset() clears vsi->orig_rss_size = 0 (relying on the
normal rebuild flow to recompute rss_size) while ADQ stays active and
ice_rebuild_channels() clamps main_vsi->rss_size back to ch_rss_size. When
the ADQ qdisc is later deleted, ice_setup_tc_mqprio_qdisc() restores
vsi->req_rxq and calls ice_vsi_rebuild(vsi, ICE_VSI_FLAG_NO_INIT). Because
ICE_VSI_FLAG_INIT is not set, ice_vsi_set_dflt_rss_params() is skipped,
vsi->orig_rss_size is 0, and ice_vsi_cfg_rss_lut_key() clamps vsi->rss_size
with min_t(u16, vsi->rss_size, vsi->num_rxq), leaving the PF VSI's rss_size
permanently clamped to the smaller ch_rss_size.

3. Similarly, in this commit ice_sriov_set_msix_vec_count() rebuilds
ICE_VSI_VF with ICE_VSI_FLAG_NO_INIT, leaving VF vsi->rss_size clamped if
VF queues are lowered and then raised (though a later patch in the series,
"ice: support up to 256 VF queues", updates ice_vsi_cfg_rss_lut_key() for
ICE_VSI_VF).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103141646.303620-1-przemyslaw.kitszel@intel.com?part=13