[PATCH v1 00/12] net/ice: update to latest version

[PATCH v1 00/12] net/ice: update to latest version

[Anatoly Burakov]

This patch adds an assortment of fixes and updates for net/ice driver,
bringing it up to date with latest shared code.

This patchset includes fixes submitted by Shaiq Wani [1], with a small change of
dropping a direction lookup flag as it's not necessary.

[1] https://patches.dpdk.org/project/dpdk/cover/20250731044132.110174-1-shaiq.wani@intel.com/

Anatoly Burakov (2):
  net/ice/base: add E835 device ID's
  net/ice: update README

Jacob Keller (2):
  net/ice/base: fix memory leak in HW profile handling
  net/ice/base: improve global config lock behavior

Jaroslaw Ilgiewicz (1):
  net/ice/base: add 40G speed

Marcin Szycik (1):
  net/ice/base: add direction metadata

Pandi Kumar Maharajan (2):
  net/ice/base: fix memory leak in recipe handling
  net/ice/base: allow overriding recipe ID

Przemek Kitszel (1):
  net/ice/base: clean up RSS LUT selection

Shaiq Wani (2):
  net/ice/base: fix adding special words
  net/ice: count drop-all filter stats in Rx stats

Stefan Wegrzyn (1):
  net/ice/base: add missing health status defines

 drivers/net/intel/ice/base/README             |   4 +-
 drivers/net/intel/ice/base/ice_adminq_cmd.h   |  21 ++-
 drivers/net/intel/ice/base/ice_common.c       | 133 ++++++++++--------
 drivers/net/intel/ice/base/ice_ddp.c          |  34 +++--
 drivers/net/intel/ice/base/ice_devids.h       |  18 +++
 drivers/net/intel/ice/base/ice_flow.c         |   4 -
 .../net/intel/ice/base/ice_protocol_type.h    |   1 +
 drivers/net/intel/ice/base/ice_switch.c       |  38 ++++-
 drivers/net/intel/ice/base/ice_switch.h       |   4 +
 drivers/net/intel/ice/ice_ethdev.c            |  12 +-
 10 files changed, 184 insertions(+), 85 deletions(-)

-- 
2.47.3

[PATCH v1 01/12] net/ice/base: add direction metadata

[Anatoly Burakov]

From: Marcin Szycik <marcin.szycik@intel.com>

Currently it is possible to create a rule which might break Tx traffic,
because by default all switch rules apply to both Rx and Tx traffic. In
order to avoid situations where Tx traffic accidentally matches Rx rule,
always add direction metadata to all switch rules.

However, because we're adding a new piece of metadata to all rules, some
rules might get too big and be rejected because they were already maxing
out rule capacity. To avoid that, we will only add direction metadata to
rules that are big enough to store it, on the assumption that if a rule
is already big enough to max out the capacity, the rule is therefore so
hyper-specific that it is highly unlikely to match both Rx and Tx
traffic, and so the direction is implied by the fact that the rule is so
specific.

Signed-off-by: Marcin Szycik <marcin.szycik@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/ice_protocol_type.h |  1 +
 drivers/net/intel/ice/base/ice_switch.c        | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/net/intel/ice/base/ice_protocol_type.h b/drivers/net/intel/ice/base/ice_protocol_type.h
index de960d7d1b..789f0d7ca5 100644
--- a/drivers/net/intel/ice/base/ice_protocol_type.h
+++ b/drivers/net/intel/ice/base/ice_protocol_type.h
@@ -226,6 +226,7 @@ enum ice_prot_id {
 #define ICE_TUN_FLAG_MDID_OFF(word) \
 	(ICE_MDID_SIZE * (ICE_TUN_FLAG_MDID + (word)))
 #define ICE_TUN_FLAG_MASK 0xFF
+#define ICE_FROM_NETWORK_FLAG_MASK 0x8
 #define ICE_DIR_FLAG_MASK 0x10
 #define ICE_TUN_FLAG_IN_VLAN_MASK 0x80 /* VLAN inside tunneled header */
 #define ICE_TUN_FLAG_VLAN_MASK 0x01
diff --git a/drivers/net/intel/ice/base/ice_switch.c b/drivers/net/intel/ice/base/ice_switch.c
index 777fc88d01..54cc2e1c07 100644
--- a/drivers/net/intel/ice/base/ice_switch.c
+++ b/drivers/net/intel/ice/base/ice_switch.c
@@ -7925,6 +7925,20 @@ ice_add_special_words(struct ice_adv_rule_info *rinfo,
 	u16 mask;
 	u16 off;
 
+	/*
+	 * Failing to add direction metadata is not considered an error, because
+	 * the kinds of rules which would trigger this error are already so
+	 * highly specific that they're unlikely to match both Rx and Tx traffic
+	 * at the same time.
+	 */
+	if (lkup_exts->n_val_words < ICE_MAX_CHAIN_WORDS) {
+		u8 word = lkup_exts->n_val_words++;
+
+		lkup_exts->fv_words[word].prot_id = ICE_META_DATA_ID_HW;
+		lkup_exts->fv_words[word].off = ICE_TUN_FLAG_MDID_OFF(0);
+		lkup_exts->field_mask[word] = ICE_FROM_NETWORK_FLAG_MASK;
+	}
+
 	/* If this is a tunneled packet, then add recipe index to match the
 	 * tunnel bit in the packet metadata flags. If this is a tun_and_non_tun
 	 * packet, then add recipe index to match the direction bit in the flag.
-- 
2.47.3

Re: [PATCH v1 01/12] net/ice/base: add direction metadata

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:26:51PM +0100, Anatoly Burakov wrote:
> From: Marcin Szycik <marcin.szycik@intel.com>
> 
> Currently it is possible to create a rule which might break Tx traffic,
> because by default all switch rules apply to both Rx and Tx traffic. In
> order to avoid situations where Tx traffic accidentally matches Rx rule,
> always add direction metadata to all switch rules.
> 
> However, because we're adding a new piece of metadata to all rules, some
> rules might get too big and be rejected because they were already maxing
> out rule capacity. To avoid that, we will only add direction metadata to
> rules that are big enough to store it, on the assumption that if a rule
> is already big enough to max out the capacity, the rule is therefore so
> hyper-specific that it is highly unlikely to match both Rx and Tx
> traffic, and so the direction is implied by the fact that the rule is so
> specific.
> 
> Signed-off-by: Marcin Szycik <marcin.szycik@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/ice/base/ice_protocol_type.h |  1 +
>  drivers/net/intel/ice/base/ice_switch.c        | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v1 02/12] net/ice/base: fix adding special words

[Anatoly Burakov]

From: Shaiq Wani <shaiq.wani@intel.com>

The function ice_add_special_words() is meant to add special words (such
as traffic direction) to the rule. The function that
interprets/translates these additional words is ice_get_sw_fv_list().

However, the ice_get_sw_fv_list() is called *before*
ice_add_special_words(), so the "special" words weren't added at that
point yet, hence they're not translated. This results in the driver
ignoring whatever special words that were added. The fix is to call
ice_get_sw_fv_list() *after* ice_add_special_words().

Fixes: ed3066a3b1b0 ("net/ice/base: refactor DDP code")
Cc: stable@dpdk.org

Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/ice_switch.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/intel/ice/base/ice_switch.c b/drivers/net/intel/ice/base/ice_switch.c
index 54cc2e1c07..f16bec044c 100644
--- a/drivers/net/intel/ice/base/ice_switch.c
+++ b/drivers/net/intel/ice/base/ice_switch.c
@@ -8287,10 +8287,6 @@ ice_add_adv_recipe(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
 	 */
 	ice_get_compat_fv_bitmap(hw, rinfo, fv_bitmap);
 
-	status = ice_get_sw_fv_list(hw, lkup_exts, fv_bitmap, &rm->fv_list);
-	if (status)
-		goto err_unroll;
-
 	/* Create any special protocol/offset pairs, such as looking at tunnel
 	 * bits by extracting metadata
 	 */
@@ -8298,6 +8294,10 @@ ice_add_adv_recipe(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
 	if (status)
 		goto err_free_lkup_exts;
 
+	status = ice_get_sw_fv_list(hw, lkup_exts, fv_bitmap, &rm->fv_list);
+	if (status)
+		goto err_unroll;
+
 	/* Group match words into recipes using preferred recipe grouping
 	 * criteria.
 	 */
-- 
2.47.3

Re: [PATCH v1 02/12] net/ice/base: fix adding special words

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:26:52PM +0100, Anatoly Burakov wrote:
> From: Shaiq Wani <shaiq.wani@intel.com>
> 
> The function ice_add_special_words() is meant to add special words (such
> as traffic direction) to the rule. The function that
> interprets/translates these additional words is ice_get_sw_fv_list().
> 
> However, the ice_get_sw_fv_list() is called *before*
> ice_add_special_words(), so the "special" words weren't added at that
> point yet, hence they're not translated. This results in the driver
> ignoring whatever special words that were added. The fix is to call
> ice_get_sw_fv_list() *after* ice_add_special_words().
> 
> Fixes: ed3066a3b1b0 ("net/ice/base: refactor DDP code")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/ice/base/ice_switch.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v1 03/12] net/ice/base: fix memory leak in HW profile handling

[Anatoly Burakov]

From: Jacob Keller <jacob.e.keller@intel.com>

The ice_flow_set_hw_prof() function allocates a params structure with
ice_malloc. It uses this structure to hold some data temporarily while
processing the hardware profile to set.

Static analysis indicated that this memory is not released. Fix this
function to free the memory upon exit.

Fixes: 8ebb93942b2c ("net/ice/base: add function to set HW profile for raw flow")
Cc: stable@dpdk.org

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/ice_flow.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/intel/ice/base/ice_flow.c b/drivers/net/intel/ice/base/ice_flow.c
index cdc9ee26c5..7b0ecd54df 100644
--- a/drivers/net/intel/ice/base/ice_flow.c
+++ b/drivers/net/intel/ice/base/ice_flow.c
@@ -2632,10 +2632,6 @@ ice_flow_set_hw_prof(struct ice_hw *hw, u16 dest_vsi_handle,
 
 	status = ice_flow_assoc_hw_prof(hw, blk, dest_vsi_handle,
 					fdir_vsi_handle, id);
-	if (status)
-		goto free_params;
-
-	return 0;
 
 free_params:
 	ice_free(hw, params);
-- 
2.47.3

Re: [PATCH v1 03/12] net/ice/base: fix memory leak in HW profile handling

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:26:53PM +0100, Anatoly Burakov wrote:
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The ice_flow_set_hw_prof() function allocates a params structure with
> ice_malloc. It uses this structure to hold some data temporarily while
> processing the hardware profile to set.
> 
> Static analysis indicated that this memory is not released. Fix this
> function to free the memory upon exit.
> 
> Fixes: 8ebb93942b2c ("net/ice/base: add function to set HW profile for raw flow")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/ice/base/ice_flow.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v1 04/12] net/ice/base: fix memory leak in recipe handling

[Anatoly Burakov]

From: Pandi Kumar Maharajan <pandi.maharajan@intel.com>

Advanced filter operations (apply/remove GENEVE/VXLAN filters) trigger
the call chain: ice_add_adv_rule()/ice_rem_adv_rule() -> ice_find_recp()
-> ice_get_recp_frm_fw(). Each call to ice_get_recp_frm_fw() creates new
linked list entries for SW recipe tracking without cleaning up previous
entries for the same recipe ID. The linked list then continuously grows
with each filter add/remove operation, leading to excessive heap usage
over time.

Fix the memory leak by adding logic to remove the duplicate entries
before adding new ones for the same recipe ID.

Fixes: fed0c5ca5f19 ("net/ice/base: support programming a new switch recipe")
Cc: stable@dpdk.org

Signed-off-by: Pandi Kumar Maharajan <pandi.maharajan@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/ice_switch.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/intel/ice/base/ice_switch.c b/drivers/net/intel/ice/base/ice_switch.c
index f16bec044c..628473f100 100644
--- a/drivers/net/intel/ice/base/ice_switch.c
+++ b/drivers/net/intel/ice/base/ice_switch.c
@@ -2435,6 +2435,7 @@ ice_get_recp_frm_fw(struct ice_hw *hw, struct ice_sw_recipe *recps, u8 rid,
 		    bool *refresh_required)
 {
 	ice_declare_bitmap(result_bm, ICE_MAX_FV_WORDS);
+	struct ice_recp_grp_entry *rg, *tmprg_entry;
 	struct ice_aqc_recipe_data_elem *tmp;
 	u16 num_recps = ICE_MAX_NUM_RECIPES;
 	struct ice_prot_lkup_ext *lkup_exts;
@@ -2481,6 +2482,15 @@ ice_get_recp_frm_fw(struct ice_hw *hw, struct ice_sw_recipe *recps, u8 rid,
 	 */
 	lkup_exts = &recps[rid].lkup_exts;
 
+	/* Remove duplicate entries */
+	LIST_FOR_EACH_ENTRY_SAFE(rg, tmprg_entry, &recps[rid].rg_list,
+	                         ice_recp_grp_entry, l_entry) {
+		if (rg->rid == rid) {
+			LIST_DEL(&rg->l_entry);
+			ice_free(hw, rg);
+		}
+	}
+
 	for (sub_recps = 0; sub_recps < num_recps; sub_recps++) {
 		struct ice_aqc_recipe_data_elem root_bufs = tmp[sub_recps];
 		struct ice_recp_grp_entry *rg_entry;
-- 
2.47.3

Re: [PATCH v1 04/12] net/ice/base: fix memory leak in recipe handling

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:26:54PM +0100, Anatoly Burakov wrote:
> From: Pandi Kumar Maharajan <pandi.maharajan@intel.com>
> 
> Advanced filter operations (apply/remove GENEVE/VXLAN filters) trigger
> the call chain: ice_add_adv_rule()/ice_rem_adv_rule() -> ice_find_recp()
> -> ice_get_recp_frm_fw(). Each call to ice_get_recp_frm_fw() creates new
> linked list entries for SW recipe tracking without cleaning up previous
> entries for the same recipe ID. The linked list then continuously grows
> with each filter add/remove operation, leading to excessive heap usage
> over time.
> 
> Fix the memory leak by adding logic to remove the duplicate entries
> before adding new ones for the same recipe ID.
> 
> Fixes: fed0c5ca5f19 ("net/ice/base: support programming a new switch recipe")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Pandi Kumar Maharajan <pandi.maharajan@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v1 05/12] net/ice/base: clean up RSS LUT selection

[Anatoly Burakov]

From: Przemek Kitszel <przemyslaw.kitszel@intel.com>

Refactor __ice_aq_get_set_rss_lut():
- Use enumeration to specify different available LUT sizes
- Use enumeration to specify different LUT types
- Refactor setting/validating various LUT settings
- Use params->lut_size and params->lut_type to communicate LUT size

Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/ice_adminq_cmd.h |  18 ++-
 drivers/net/intel/ice/base/ice_common.c     | 124 ++++++++++----------
 2 files changed, 81 insertions(+), 61 deletions(-)

diff --git a/drivers/net/intel/ice/base/ice_adminq_cmd.h b/drivers/net/intel/ice/base/ice_adminq_cmd.h
index 1fa8bbad29..a5817daca9 100644
--- a/drivers/net/intel/ice/base/ice_adminq_cmd.h
+++ b/drivers/net/intel/ice/base/ice_adminq_cmd.h
@@ -2385,6 +2385,20 @@ struct ice_aqc_get_set_rss_keys {
 	u8 extended_hash_key[ICE_AQC_GET_SET_RSS_KEY_DATA_HASH_KEY_SIZE];
 };
 
+enum ice_lut_type {
+	ICE_LUT_VSI = 0,
+	ICE_LUT_PF = 1,
+	ICE_LUT_GLOBAL = 2,
+	ICE_LUT_TYPE_MASK = 3
+};
+
+enum ice_lut_size {
+	ICE_LUT_SIZE_64 = 64,
+	ICE_LUT_SIZE_128 = 128,
+	ICE_LUT_SIZE_512 = 512,
+	ICE_LUT_SIZE_2048 = 2048,
+};
+
 /* Get/Set RSS LUT (indirect 0x0B05/0x0B03) */
 struct ice_aqc_get_set_rss_lut {
 #define ICE_AQC_GSET_RSS_LUT_VSI_VALID	BIT(15)
@@ -2393,7 +2407,7 @@ struct ice_aqc_get_set_rss_lut {
 	__le16 vsi_id;
 #define ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_S	0
 #define ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_M	\
-				(0x3 << ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_S)
+		(ICE_LUT_TYPE_MASK << ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_S)
 
 #define ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_VSI	 0
 #define ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_PF	 1
@@ -2401,7 +2415,7 @@ struct ice_aqc_get_set_rss_lut {
 
 #define ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S	 2
 #define ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_M	 \
-				(0x3 << ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S)
+		(ICE_LUT_TYPE_MASK << ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S)
 
 #define ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128	 128
 #define ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128_FLAG 0
diff --git a/drivers/net/intel/ice/base/ice_common.c b/drivers/net/intel/ice/base/ice_common.c
index dfc2fd69e7..8be56f0aab 100644
--- a/drivers/net/intel/ice/base/ice_common.c
+++ b/drivers/net/intel/ice/base/ice_common.c
@@ -4332,6 +4332,50 @@ ice_aq_read_topo_dev_nvm(struct ice_hw *hw,
 	return 0;
 }
 
+static u16 ice_lut_is_valid_size(u16 lut_type, u16 lut_size)
+{
+	switch (lut_type) {
+	case ICE_LUT_VSI:
+		return lut_size == ICE_LUT_SIZE_64;
+	case ICE_LUT_GLOBAL:
+		switch (lut_size) {
+		case ICE_LUT_SIZE_128:
+		case ICE_LUT_SIZE_512:
+			return true;
+		default:
+			return false;
+		}
+	case ICE_LUT_PF:
+		switch (lut_size) {
+		case ICE_LUT_SIZE_128:
+		case ICE_LUT_SIZE_512:
+		case ICE_LUT_SIZE_2048:
+			return true;
+		default:
+			return false;
+		}
+	default:
+		return false;
+	}
+}
+
+static u16 ice_lut_size_to_flag(u16 lut_size)
+{
+	u16 f = 0;
+
+	switch (lut_size) {
+	case ICE_LUT_SIZE_512:
+		f = ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_512_FLAG;
+		break;
+	case ICE_LUT_SIZE_2048:
+		f = ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_2K_FLAG;
+		break;
+	default:
+		break;
+	}
+	return f << ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S;
+}
+
 /**
  * __ice_aq_get_set_rss_lut
  * @hw: pointer to the hardware structure
@@ -4343,7 +4387,7 @@ ice_aq_read_topo_dev_nvm(struct ice_hw *hw,
 static int
 __ice_aq_get_set_rss_lut(struct ice_hw *hw, struct ice_aq_get_set_rss_lut_params *params, bool set)
 {
-	u16 flags = 0, vsi_id, lut_type, lut_size, glob_lut_idx, vsi_handle;
+	u16 flags, vsi_id, lut_type, lut_size, glob_lut_idx = 0, vsi_handle;
 	struct ice_aqc_get_set_rss_lut *cmd_resp;
 	struct ice_aq_desc desc;
 	int status;
@@ -4355,15 +4399,23 @@ __ice_aq_get_set_rss_lut(struct ice_hw *hw, struct ice_aq_get_set_rss_lut_params
 	vsi_handle = params->vsi_handle;
 	lut = params->lut;
 
-	if (!ice_is_vsi_valid(hw, vsi_handle) || !lut)
-		return ICE_ERR_PARAM;
-
+	lut_type = params->lut_type;
 	lut_size = params->lut_size;
 	lut_type = params->lut_type;
-	glob_lut_idx = params->global_lut_id;
-	vsi_id = ice_get_hw_vsi_num(hw, vsi_handle);
-
 	cmd_resp = &desc.params.get_set_rss_lut;
+	if (lut_type == ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_GLOBAL)
+		glob_lut_idx = params->global_lut_id;
+
+	if (!lut || !ice_is_vsi_valid(hw, vsi_handle))
+		return ICE_ERR_PARAM;
+
+	if (lut_type & ~ICE_LUT_TYPE_MASK)
+		return ICE_ERR_PARAM;
+
+	if (!ice_lut_is_valid_size(lut_type, lut_size))
+		return ICE_ERR_INVAL_SIZE;
+
+	vsi_id = ice_get_hw_vsi_num(hw, vsi_handle);
 
 	if (set) {
 		ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_set_rss_lut);
@@ -4377,61 +4429,15 @@ __ice_aq_get_set_rss_lut(struct ice_hw *hw, struct ice_aq_get_set_rss_lut_params
 					ICE_AQC_GSET_RSS_LUT_VSI_ID_M) |
 				       ICE_AQC_GSET_RSS_LUT_VSI_VALID);
 
-	switch (lut_type) {
-	case ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_VSI:
-	case ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_PF:
-	case ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_GLOBAL:
-		flags |= ((lut_type << ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_S) &
-			  ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_M);
-		break;
-	default:
-		status = ICE_ERR_PARAM;
-		goto ice_aq_get_set_rss_lut_exit;
-	}
+	flags = ice_lut_size_to_flag(lut_size) |
+			((lut_type << ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_S) &
+				ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_M) |
+			((glob_lut_idx << ICE_AQC_GSET_RSS_LUT_GLOBAL_IDX_S) &
+				ICE_AQC_GSET_RSS_LUT_GLOBAL_IDX_M);
 
-	if (lut_type == ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_GLOBAL) {
-		flags |= ((glob_lut_idx << ICE_AQC_GSET_RSS_LUT_GLOBAL_IDX_S) &
-			  ICE_AQC_GSET_RSS_LUT_GLOBAL_IDX_M);
-
-		if (!set)
-			goto ice_aq_get_set_rss_lut_send;
-	} else if (lut_type == ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_PF) {
-		if (!set)
-			goto ice_aq_get_set_rss_lut_send;
-	} else {
-		goto ice_aq_get_set_rss_lut_send;
-	}
-
-	/* LUT size is only valid for Global and PF table types */
-	switch (lut_size) {
-	case ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128:
-		flags |= (ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_128_FLAG <<
-			  ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S) &
-			 ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_M;
-		break;
-	case ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_512:
-		flags |= (ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_512_FLAG <<
-			  ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S) &
-			 ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_M;
-		break;
-	case ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_2K:
-		if (lut_type == ICE_AQC_GSET_RSS_LUT_TABLE_TYPE_PF) {
-			flags |= (ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_2K_FLAG <<
-				  ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_S) &
-				 ICE_AQC_GSET_RSS_LUT_TABLE_SIZE_M;
-			break;
-		}
-		/* fall-through */
-	default:
-		status = ICE_ERR_PARAM;
-		goto ice_aq_get_set_rss_lut_exit;
-	}
-
-ice_aq_get_set_rss_lut_send:
 	cmd_resp->flags = CPU_TO_LE16(flags);
 	status = ice_aq_send_cmd(hw, &desc, lut, lut_size, NULL);
-
-ice_aq_get_set_rss_lut_exit:
+	params->lut_size = LE16_TO_CPU(desc.datalen);
 	return status;
 }
 
-- 
2.47.3

Re: [PATCH v1 05/12] net/ice/base: clean up RSS LUT selection

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:26:55PM +0100, Anatoly Burakov wrote:
> From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> 
> Refactor __ice_aq_get_set_rss_lut():
> - Use enumeration to specify different available LUT sizes
> - Use enumeration to specify different LUT types
> - Refactor setting/validating various LUT settings
> - Use params->lut_size and params->lut_type to communicate LUT size
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v1 06/12] net/ice/base: add 40G speed

[Anatoly Burakov]

From: Jaroslaw Ilgiewicz <jaroslaw.ilgiewicz@intel.com>

Added 40G speed for port options

Signed-off-by: Jaroslaw Ilgiewicz <jaroslaw.ilgiewicz@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/ice_adminq_cmd.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/intel/ice/base/ice_adminq_cmd.h b/drivers/net/intel/ice/base/ice_adminq_cmd.h
index a5817daca9..34b008f5fd 100644
--- a/drivers/net/intel/ice/base/ice_adminq_cmd.h
+++ b/drivers/net/intel/ice/base/ice_adminq_cmd.h
@@ -1917,6 +1917,7 @@ struct ice_aqc_get_port_options_elem {
 #define ICE_AQC_PORT_OPT_MAX_LANE_50G	6
 #define ICE_AQC_PORT_OPT_MAX_LANE_100G	7
 #define ICE_AQC_PORT_OPT_MAX_LANE_200G	8
+#define ICE_AQC_PORT_OPT_MAX_LANE_40G	9
 	u8 global_scid[2];
 	u8 phy_scid[2];
 	u8 pf2port_cid[2];
-- 
2.47.3

Re: [PATCH v1 06/12] net/ice/base: add 40G speed

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:26:56PM +0100, Anatoly Burakov wrote:
> From: Jaroslaw Ilgiewicz <jaroslaw.ilgiewicz@intel.com>
> 
> Added 40G speed for port options
> 
> Signed-off-by: Jaroslaw Ilgiewicz <jaroslaw.ilgiewicz@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v1 07/12] net/ice/base: allow overriding recipe ID

[Anatoly Burakov]

From: Pandi Kumar Maharajan <pandi.maharajan@intel.com>

With the addition of double VLAN mode on the device, the MAC VLAN filter
add rule was previously matching only the inner VLAN, rather than the
intended outer VLAN. This patch resolves the issue by introducing a new
software recipe ID as part of ice_fltr_info, allowing the base code to
send the newly created recipe ID. This ensures that the filter correctly
matches the outer VLAN when processing packets. Issue is observed in ESXi
environment but this patch introduces a generic solution for all
operating systems by adding a rid_override flag. If the base driver needs
to use a recipe ID different from the shared code default, it can set the
rid variable and enable the rid_override flag. This instructs the shared
code to use the specified rid instead of the default value.

Signed-off-by: Pandi Kumar Maharajan <pandi.maharajan@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/ice_switch.c | 6 +++++-
 drivers/net/intel/ice/base/ice_switch.h | 4 ++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/intel/ice/base/ice_switch.c b/drivers/net/intel/ice/base/ice_switch.c
index 628473f100..fff61b89d7 100644
--- a/drivers/net/intel/ice/base/ice_switch.c
+++ b/drivers/net/intel/ice/base/ice_switch.c
@@ -4113,7 +4113,11 @@ ice_fill_sw_rule(struct ice_hw *hw, struct ice_fltr_info *f_info,
 		CPU_TO_LE16(ICE_AQC_SW_RULES_T_LKUP_TX);
 
 	/* Recipe set depending on lookup type */
-	s_rule->recipe_id = CPU_TO_LE16(f_info->lkup_type);
+	if (f_info->rid_override) {
+		s_rule->recipe_id = CPU_TO_LE16(f_info->rid);
+	} else {
+		s_rule->recipe_id = CPU_TO_LE16(f_info->lkup_type);
+	}
 	s_rule->src = CPU_TO_LE16(f_info->src);
 	s_rule->act = CPU_TO_LE32(act);
 
diff --git a/drivers/net/intel/ice/base/ice_switch.h b/drivers/net/intel/ice/base/ice_switch.h
index 8eac7739fb..73452cc4c7 100644
--- a/drivers/net/intel/ice/base/ice_switch.h
+++ b/drivers/net/intel/ice/base/ice_switch.h
@@ -195,6 +195,10 @@ struct ice_fltr_info {
 	u8 lb_en;	/* Indicate if packet can be looped back */
 	u8 lan_en;	/* Indicate if packet can be forwarded to the uplink */
 	u8 fltVeb_en;   /* Indicate if VSI is connected to floating VEB */
+
+	/* Override default Recipe ID */
+	u16 rid;
+	bool rid_override;
 };
 
 struct ice_update_recipe_lkup_idx_params {
-- 
2.47.3

Re: [PATCH v1 07/12] net/ice/base: allow overriding recipe ID

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:26:57PM +0100, Anatoly Burakov wrote:
> From: Pandi Kumar Maharajan <pandi.maharajan@intel.com>
> 
> With the addition of double VLAN mode on the device, the MAC VLAN filter
> add rule was previously matching only the inner VLAN, rather than the
> intended outer VLAN. This patch resolves the issue by introducing a new
> software recipe ID as part of ice_fltr_info, allowing the base code to
> send the newly created recipe ID. This ensures that the filter correctly
> matches the outer VLAN when processing packets. Issue is observed in ESXi
> environment but this patch introduces a generic solution for all
> operating systems by adding a rid_override flag. If the base driver needs
> to use a recipe ID different from the shared code default, it can set the
> rid variable and enable the rid_override flag. This instructs the shared
> code to use the specified rid instead of the default value.
> 
> Signed-off-by: Pandi Kumar Maharajan <pandi.maharajan@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/ice/base/ice_switch.c | 6 +++++-
>  drivers/net/intel/ice/base/ice_switch.h | 4 ++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v1 08/12] net/ice/base: add missing health status defines

[Anatoly Burakov]

From: Stefan Wegrzyn <stefan.wegrzyn@intel.com>

Added missing defines for commands related to health status.

Signed-off-by: Stefan Wegrzyn <stefan.wegrzyn@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/ice_adminq_cmd.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/intel/ice/base/ice_adminq_cmd.h b/drivers/net/intel/ice/base/ice_adminq_cmd.h
index 34b008f5fd..36656b0aad 100644
--- a/drivers/net/intel/ice/base/ice_adminq_cmd.h
+++ b/drivers/net/intel/ice/base/ice_adminq_cmd.h
@@ -3184,6 +3184,7 @@ struct ice_aqc_set_health_status_config {
 #define ICE_AQC_HEALTH_STATUS_SET_PF_SPECIFIC_MASK	BIT(0)
 #define ICE_AQC_HEALTH_STATUS_SET_ALL_PF_MASK		BIT(1)
 #define ICE_AQC_HEALTH_STATUS_SET_GLOBAL_MASK		BIT(2)
+#define ICE_AQC_HEALTH_STATUS_DISABLE_HISTORY_MASK	BIT(3)
 	u8 reserved[15];
 };
 
@@ -3224,6 +3225,7 @@ struct ice_aqc_set_health_status_config {
 #define ICE_AQC_HEALTH_STATUS_ERR_BMC_RESET			0x50B
 #define ICE_AQC_HEALTH_STATUS_ERR_LAST_MNG_FAIL			0x50C
 #define ICE_AQC_HEALTH_STATUS_ERR_RESOURCE_ALLOC_FAIL		0x50D
+#define ICE_AQC_HEALTH_STATUS_INFO_DPLL_LOCK			0x601
 #define ICE_AQC_HEALTH_STATUS_ERR_FW_LOOP			0x1000
 #define ICE_AQC_HEALTH_STATUS_ERR_FW_PFR_FAIL			0x1001
 #define ICE_AQC_HEALTH_STATUS_ERR_LAST_FAIL_AQ			0x1002
-- 
2.47.3

Re: [PATCH v1 08/12] net/ice/base: add missing health status defines

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:26:58PM +0100, Anatoly Burakov wrote:
> From: Stefan Wegrzyn <stefan.wegrzyn@intel.com>
> 
> Added missing defines for commands related to health status.
> 
> Signed-off-by: Stefan Wegrzyn <stefan.wegrzyn@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/ice/base/ice_adminq_cmd.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v1 09/12] net/ice/base: improve global config lock behavior

[Anatoly Burakov]

From: Jacob Keller <jacob.e.keller@intel.com>

The ice_cfg_tx_topo function attempts to apply Tx scheduler topology
configuration based on NVM parameters, selecting either a 5 or 9 layer
topology.

As part of this flow, the driver acquires the "Global Configuration
Lock", which is a hardware resource associated with programming the DDP
package to the device. This "lock" is implemented by firmware as a way to
guarantee that only one PF can program the DDP for a device. Unlike a
traditional lock, once a PF has acquired this lock, no other PF will be
able to acquire it again (including that PF) until a core reset of the
device. Future requests to acquire the lock report that global
configuration has already completed.

The following flow is used to program the Tx topology:

 * Read the DDP package for scheduler configuration data
 * Acquire the global configuration lock
 * Program Tx scheduler topology according to DDP package data
 * Trigger a core reset which clears the global configuration lock

This is followed by the flow for programming the DDP package:

 * Acquire the global configuration lock (again)
 * Download the DDP package to the device
 * Release the global configuration lock.

However, if configuration of the Tx topology fails, (i.e.
ice_get_set_tx_topo() returns an error code), the driver exits
ice_cfg_tx_topo() immediately, and fails to trigger core reset.

While the global configuration lock is held, the firmware rejects most
AdminQ commands, as it is waiting for the DDP package download (or Tx
scheduler topology programming) to occur.

The current driver flows assume that the global configuration lock has
been reset after programming the Tx topology. Thus, the same PF attempts
to acquire the global lock again, and fails. This results in the driver
reporting "an unknown error occurred when loading the DDP package". It
then attempts to enter safe mode, but ultimately fails to finish
ice_probe() since nearly all AdminQ command report error codes, and the
driver stops loading the device at some point during its initialization.

We cannot simply release the global lock after a failed call to
ice_get_set_tx_topo(). Releasing the lock indicates to firmware that
global configuration (downloading of the DDP) has completed. Future
attempts by this or other PFs to load the DDP will fail with a report
that the DDP package has already been downloaded. Then, PFs will enter
safe mode as they realize that the package on the device does not meet
the minimum version requirement to load. The reported error messages are
confusing, as they indicate the version of the default "safe mode"
package in the NVM, rather than the version of the DDP package loaded
from the filesystem.

Instead, we need to trigger core reset to clear global configuration.
This is the lowest level of hardware reset which clears the global
configuration lock and related state. It also clears any already
downloaded DDP. Crucially, it does *not* clear the Tx scheduler topology
configuration.

Refactor ice_cfg_tx_topo() to always trigger a core reset after acquiring
the global lock, regardless of success or failure of the topology
configuration.

We need to re-initialize the HW structure when we trigger the core reset.
Previously, this was the responsibility of the core driver to cleanup
after the core reset. Instead, make it the responsibility of this
function. This avoids needless re-initialization for the cases where no
reset occurred.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/ice_ddp.c | 34 ++++++++++++++++++----------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/net/intel/ice/base/ice_ddp.c b/drivers/net/intel/ice/base/ice_ddp.c
index 850c722a3f..68e75be4d2 100644
--- a/drivers/net/intel/ice/base/ice_ddp.c
+++ b/drivers/net/intel/ice/base/ice_ddp.c
@@ -2370,7 +2370,7 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
 	struct ice_buf_hdr *section;
 	struct ice_pkg_hdr *pkg_hdr;
 	enum ice_ddp_state state;
-	u16 i, size = 0, offset;
+	u16 size = 0, offset;
 	u32 reg = 0;
 	int status;
 	u8 flags;
@@ -2457,25 +2457,35 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
 	/* check reset was triggered already or not */
 	reg = rd32(hw, GLGEN_RSTAT);
 	if (reg & GLGEN_RSTAT_DEVSTATE_M) {
-		/* Reset is in progress, re-init the hw again */
 		ice_debug(hw, ICE_DBG_INIT, "Reset is in progress. layer topology might be applied already\n");
 		ice_check_reset(hw);
-		return 0;
+		/* Reset is in progress, re-init the hw again */
+		goto reinit_hw;
 	}
 
 	/* set new topology */
 	status = ice_get_set_tx_topo(hw, new_topo, size, NULL, NULL, true);
 	if (status) {
-		ice_debug(hw, ICE_DBG_INIT, "Set tx topology is failed\n");
-		return status;
+		ice_debug(hw, ICE_DBG_INIT, "Failed setting Tx topology, status %d\n",
+			  status);
+		status = ICE_ERR_CFG;
 	}
 
-	/* new topology is updated, delay 1 second before issuing the CORRER */
-	for (i = 0; i < 10; i++)
-		ice_msec_delay(100, true);
+	/* Even if Tx topology config failed, we need to CORE reset here to
+	 * clear the global configuration lock. Delay 1 second to allow
+	 * hardware to settle then issue a CORER
+	 */
+	ice_msec_delay(1000, true);
 	ice_reset(hw, ICE_RESET_CORER);
-	/* CORER will clear the global lock, so no explicit call
-	 * required for release
-	 */
-	return 0;
+	ice_check_reset(hw);
+
+reinit_hw:
+	/* Since we triggered a CORER, re-initialize hardware */
+	ice_deinit_hw(hw);
+	if (ice_init_hw(hw)) {
+		ice_debug(hw, ICE_DBG_INIT, "Failed to re-init hardware after setting Tx topology\n");
+		return ICE_ERR_RESET_FAILED;
+	}
+
+	return status;
 }
-- 
2.47.3

Re: [PATCH v1 09/12] net/ice/base: improve global config lock behavior

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:26:59PM +0100, Anatoly Burakov wrote:
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The ice_cfg_tx_topo function attempts to apply Tx scheduler topology
> configuration based on NVM parameters, selecting either a 5 or 9 layer
> topology.
> 
> As part of this flow, the driver acquires the "Global Configuration
> Lock", which is a hardware resource associated with programming the DDP
> package to the device. This "lock" is implemented by firmware as a way to
> guarantee that only one PF can program the DDP for a device. Unlike a
> traditional lock, once a PF has acquired this lock, no other PF will be
> able to acquire it again (including that PF) until a core reset of the
> device. Future requests to acquire the lock report that global
> configuration has already completed.
> 
> The following flow is used to program the Tx topology:
> 
>  * Read the DDP package for scheduler configuration data
>  * Acquire the global configuration lock
>  * Program Tx scheduler topology according to DDP package data
>  * Trigger a core reset which clears the global configuration lock
> 
> This is followed by the flow for programming the DDP package:
> 
>  * Acquire the global configuration lock (again)
>  * Download the DDP package to the device
>  * Release the global configuration lock.
> 
> However, if configuration of the Tx topology fails, (i.e.
> ice_get_set_tx_topo() returns an error code), the driver exits
> ice_cfg_tx_topo() immediately, and fails to trigger core reset.
> 
> While the global configuration lock is held, the firmware rejects most
> AdminQ commands, as it is waiting for the DDP package download (or Tx
> scheduler topology programming) to occur.
> 
> The current driver flows assume that the global configuration lock has
> been reset after programming the Tx topology. Thus, the same PF attempts
> to acquire the global lock again, and fails. This results in the driver
> reporting "an unknown error occurred when loading the DDP package". It
> then attempts to enter safe mode, but ultimately fails to finish
> ice_probe() since nearly all AdminQ command report error codes, and the
> driver stops loading the device at some point during its initialization.
> 
> We cannot simply release the global lock after a failed call to
> ice_get_set_tx_topo(). Releasing the lock indicates to firmware that
> global configuration (downloading of the DDP) has completed. Future
> attempts by this or other PFs to load the DDP will fail with a report
> that the DDP package has already been downloaded. Then, PFs will enter
> safe mode as they realize that the package on the device does not meet
> the minimum version requirement to load. The reported error messages are
> confusing, as they indicate the version of the default "safe mode"
> package in the NVM, rather than the version of the DDP package loaded
> from the filesystem.
> 
> Instead, we need to trigger core reset to clear global configuration.
> This is the lowest level of hardware reset which clears the global
> configuration lock and related state. It also clears any already
> downloaded DDP. Crucially, it does *not* clear the Tx scheduler topology
> configuration.
> 
> Refactor ice_cfg_tx_topo() to always trigger a core reset after acquiring
> the global lock, regardless of success or failure of the topology
> configuration.
> 
> We need to re-initialize the HW structure when we trigger the core reset.
> Previously, this was the responsibility of the core driver to cleanup
> after the core reset. Instead, make it the responsibility of this
> function. This avoids needless re-initialization for the cases where no
> reset occurred.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/ice/base/ice_ddp.c | 34 ++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

See one comment inline below.


> diff --git a/drivers/net/intel/ice/base/ice_ddp.c b/drivers/net/intel/ice/base/ice_ddp.c
> index 850c722a3f..68e75be4d2 100644
> --- a/drivers/net/intel/ice/base/ice_ddp.c
> +++ b/drivers/net/intel/ice/base/ice_ddp.c
> @@ -2370,7 +2370,7 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
>  	struct ice_buf_hdr *section;
>  	struct ice_pkg_hdr *pkg_hdr;
>  	enum ice_ddp_state state;
> -	u16 i, size = 0, offset;
> +	u16 size = 0, offset;
>  	u32 reg = 0;
>  	int status;
>  	u8 flags;
> @@ -2457,25 +2457,35 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
>  	/* check reset was triggered already or not */
>  	reg = rd32(hw, GLGEN_RSTAT);
>  	if (reg & GLGEN_RSTAT_DEVSTATE_M) {
> -		/* Reset is in progress, re-init the hw again */
>  		ice_debug(hw, ICE_DBG_INIT, "Reset is in progress. layer topology might be applied already\n");
>  		ice_check_reset(hw);
> -		return 0;
> +		/* Reset is in progress, re-init the hw again */
> +		goto reinit_hw;
>  	}
>  
>  	/* set new topology */
>  	status = ice_get_set_tx_topo(hw, new_topo, size, NULL, NULL, true);
>  	if (status) {
> -		ice_debug(hw, ICE_DBG_INIT, "Set tx topology is failed\n");
> -		return status;
> +		ice_debug(hw, ICE_DBG_INIT, "Failed setting Tx topology, status %d\n",
> +			  status);
> +		status = ICE_ERR_CFG;
>  	}
>  
> -	/* new topology is updated, delay 1 second before issuing the CORRER */
> -	for (i = 0; i < 10; i++)
> -		ice_msec_delay(100, true);
> +	/* Even if Tx topology config failed, we need to CORE reset here to
> +	 * clear the global configuration lock. Delay 1 second to allow
> +	 * hardware to settle then issue a CORER
> +	 */
> +	ice_msec_delay(1000, true);
>  	ice_reset(hw, ICE_RESET_CORER);
> -	/* CORER will clear the global lock, so no explicit call
> -	 * required for release
> -	 */
> -	return 0;
> +	ice_check_reset(hw);
> +
> +reinit_hw:
> +	/* Since we triggered a CORER, re-initialize hardware */
> +	ice_deinit_hw(hw);
> +	if (ice_init_hw(hw)) {
> +		ice_debug(hw, ICE_DBG_INIT, "Failed to re-init hardware after setting Tx topology\n");
> +		return ICE_ERR_RESET_FAILED;
> +	}
> +
> +	return status;
>  }

There is a similer deinit + init combo in ice_init_pkg in the same file,
which is the only use of this function (in DPDK anyway). Therefore, I
believe that that code can be removed now that the reinit is handled by the
cfg_tx_topo() function.

Re: [PATCH v1 09/12] net/ice/base: improve global config lock behavior

[Jacob Keller]

[-- Attachment #1.1: Type: text/plain, Size: 7391 bytes --]



On 9/5/2025 8:32 AM, Bruce Richardson wrote:
> On Tue, Sep 02, 2025 at 06:26:59PM +0100, Anatoly Burakov wrote:
>> From: Jacob Keller <jacob.e.keller@intel.com>
>>
>> The ice_cfg_tx_topo function attempts to apply Tx scheduler topology
>> configuration based on NVM parameters, selecting either a 5 or 9 layer
>> topology.
>>
>> As part of this flow, the driver acquires the "Global Configuration
>> Lock", which is a hardware resource associated with programming the DDP
>> package to the device. This "lock" is implemented by firmware as a way to
>> guarantee that only one PF can program the DDP for a device. Unlike a
>> traditional lock, once a PF has acquired this lock, no other PF will be
>> able to acquire it again (including that PF) until a core reset of the
>> device. Future requests to acquire the lock report that global
>> configuration has already completed.
>>
>> The following flow is used to program the Tx topology:
>>
>>  * Read the DDP package for scheduler configuration data
>>  * Acquire the global configuration lock
>>  * Program Tx scheduler topology according to DDP package data
>>  * Trigger a core reset which clears the global configuration lock
>>
>> This is followed by the flow for programming the DDP package:
>>
>>  * Acquire the global configuration lock (again)
>>  * Download the DDP package to the device
>>  * Release the global configuration lock.
>>
>> However, if configuration of the Tx topology fails, (i.e.
>> ice_get_set_tx_topo() returns an error code), the driver exits
>> ice_cfg_tx_topo() immediately, and fails to trigger core reset.
>>
>> While the global configuration lock is held, the firmware rejects most
>> AdminQ commands, as it is waiting for the DDP package download (or Tx
>> scheduler topology programming) to occur.
>>
>> The current driver flows assume that the global configuration lock has
>> been reset after programming the Tx topology. Thus, the same PF attempts
>> to acquire the global lock again, and fails. This results in the driver
>> reporting "an unknown error occurred when loading the DDP package". It
>> then attempts to enter safe mode, but ultimately fails to finish
>> ice_probe() since nearly all AdminQ command report error codes, and the
>> driver stops loading the device at some point during its initialization.
>>
>> We cannot simply release the global lock after a failed call to
>> ice_get_set_tx_topo(). Releasing the lock indicates to firmware that
>> global configuration (downloading of the DDP) has completed. Future
>> attempts by this or other PFs to load the DDP will fail with a report
>> that the DDP package has already been downloaded. Then, PFs will enter
>> safe mode as they realize that the package on the device does not meet
>> the minimum version requirement to load. The reported error messages are
>> confusing, as they indicate the version of the default "safe mode"
>> package in the NVM, rather than the version of the DDP package loaded
>> from the filesystem.
>>
>> Instead, we need to trigger core reset to clear global configuration.
>> This is the lowest level of hardware reset which clears the global
>> configuration lock and related state. It also clears any already
>> downloaded DDP. Crucially, it does *not* clear the Tx scheduler topology
>> configuration.
>>
>> Refactor ice_cfg_tx_topo() to always trigger a core reset after acquiring
>> the global lock, regardless of success or failure of the topology
>> configuration.
>>
>> We need to re-initialize the HW structure when we trigger the core reset.
>> Previously, this was the responsibility of the core driver to cleanup
>> after the core reset. Instead, make it the responsibility of this
>> function. This avoids needless re-initialization for the cases where no
>> reset occurred.
>>
>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>  drivers/net/intel/ice/base/ice_ddp.c | 34 ++++++++++++++++++----------
>>  1 file changed, 22 insertions(+), 12 deletions(-)
>>
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> See one comment inline below.
> 
> 
>> diff --git a/drivers/net/intel/ice/base/ice_ddp.c b/drivers/net/intel/ice/base/ice_ddp.c
>> index 850c722a3f..68e75be4d2 100644
>> --- a/drivers/net/intel/ice/base/ice_ddp.c
>> +++ b/drivers/net/intel/ice/base/ice_ddp.c
>> @@ -2370,7 +2370,7 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
>>  	struct ice_buf_hdr *section;
>>  	struct ice_pkg_hdr *pkg_hdr;
>>  	enum ice_ddp_state state;
>> -	u16 i, size = 0, offset;
>> +	u16 size = 0, offset;
>>  	u32 reg = 0;
>>  	int status;
>>  	u8 flags;
>> @@ -2457,25 +2457,35 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
>>  	/* check reset was triggered already or not */
>>  	reg = rd32(hw, GLGEN_RSTAT);
>>  	if (reg & GLGEN_RSTAT_DEVSTATE_M) {
>> -		/* Reset is in progress, re-init the hw again */
>>  		ice_debug(hw, ICE_DBG_INIT, "Reset is in progress. layer topology might be applied already\n");
>>  		ice_check_reset(hw);
>> -		return 0;
>> +		/* Reset is in progress, re-init the hw again */
>> +		goto reinit_hw;
>>  	}
>>  
>>  	/* set new topology */
>>  	status = ice_get_set_tx_topo(hw, new_topo, size, NULL, NULL, true);
>>  	if (status) {
>> -		ice_debug(hw, ICE_DBG_INIT, "Set tx topology is failed\n");
>> -		return status;
>> +		ice_debug(hw, ICE_DBG_INIT, "Failed setting Tx topology, status %d\n",
>> +			  status);
>> +		status = ICE_ERR_CFG;
>>  	}
>>  
>> -	/* new topology is updated, delay 1 second before issuing the CORRER */
>> -	for (i = 0; i < 10; i++)
>> -		ice_msec_delay(100, true);
>> +	/* Even if Tx topology config failed, we need to CORE reset here to
>> +	 * clear the global configuration lock. Delay 1 second to allow
>> +	 * hardware to settle then issue a CORER
>> +	 */
>> +	ice_msec_delay(1000, true);
>>  	ice_reset(hw, ICE_RESET_CORER);
>> -	/* CORER will clear the global lock, so no explicit call
>> -	 * required for release
>> -	 */
>> -	return 0;
>> +	ice_check_reset(hw);
>> +
>> +reinit_hw:
>> +	/* Since we triggered a CORER, re-initialize hardware */
>> +	ice_deinit_hw(hw);
>> +	if (ice_init_hw(hw)) {
>> +		ice_debug(hw, ICE_DBG_INIT, "Failed to re-init hardware after setting Tx topology\n");
>> +		return ICE_ERR_RESET_FAILED;
>> +	}
>> +
>> +	return status;
>>  }
> 
> There is a similer deinit + init combo in ice_init_pkg in the same file,
> which is the only use of this function (in DPDK anyway). Therefore, I
> believe that that code can be removed now that the reinit is handled by the
> cfg_tx_topo() function.

I am not sure we can do that. We program Tx topology by taking the
global configuration lock, then programming the topology, and then
performing a CORE reset. We can't simply release the global config lock,
because then DDP won't be able to download at all. We can't avoid this
re-init, because after a CORE reset, the drivers AQ setup will be hosed
and thus we need a re-init.

ice_init_pkg should be called after Tx topology is executed. If another
reset occurs there, I think you may still need to do that re-init too.
There could possibly be a complete refactor that removed the need for a
double reset, but I am not certain.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH v1 09/12] net/ice/base: improve global config lock behavior

[Bruce Richardson]

On Fri, Sep 05, 2025 at 04:25:19PM -0700, Jacob Keller wrote:
> 
> 
> On 9/5/2025 8:32 AM, Bruce Richardson wrote:
> > On Tue, Sep 02, 2025 at 06:26:59PM +0100, Anatoly Burakov wrote:
> >> From: Jacob Keller <jacob.e.keller@intel.com>
> >>
> >> The ice_cfg_tx_topo function attempts to apply Tx scheduler topology
> >> configuration based on NVM parameters, selecting either a 5 or 9 layer
> >> topology.
> >>
> >> As part of this flow, the driver acquires the "Global Configuration
> >> Lock", which is a hardware resource associated with programming the DDP
> >> package to the device. This "lock" is implemented by firmware as a way to
> >> guarantee that only one PF can program the DDP for a device. Unlike a
> >> traditional lock, once a PF has acquired this lock, no other PF will be
> >> able to acquire it again (including that PF) until a core reset of the
> >> device. Future requests to acquire the lock report that global
> >> configuration has already completed.
> >>
> >> The following flow is used to program the Tx topology:
> >>
> >>  * Read the DDP package for scheduler configuration data
> >>  * Acquire the global configuration lock
> >>  * Program Tx scheduler topology according to DDP package data
> >>  * Trigger a core reset which clears the global configuration lock
> >>
> >> This is followed by the flow for programming the DDP package:
> >>
> >>  * Acquire the global configuration lock (again)
> >>  * Download the DDP package to the device
> >>  * Release the global configuration lock.
> >>
> >> However, if configuration of the Tx topology fails, (i.e.
> >> ice_get_set_tx_topo() returns an error code), the driver exits
> >> ice_cfg_tx_topo() immediately, and fails to trigger core reset.
> >>
> >> While the global configuration lock is held, the firmware rejects most
> >> AdminQ commands, as it is waiting for the DDP package download (or Tx
> >> scheduler topology programming) to occur.
> >>
> >> The current driver flows assume that the global configuration lock has
> >> been reset after programming the Tx topology. Thus, the same PF attempts
> >> to acquire the global lock again, and fails. This results in the driver
> >> reporting "an unknown error occurred when loading the DDP package". It
> >> then attempts to enter safe mode, but ultimately fails to finish
> >> ice_probe() since nearly all AdminQ command report error codes, and the
> >> driver stops loading the device at some point during its initialization.
> >>
> >> We cannot simply release the global lock after a failed call to
> >> ice_get_set_tx_topo(). Releasing the lock indicates to firmware that
> >> global configuration (downloading of the DDP) has completed. Future
> >> attempts by this or other PFs to load the DDP will fail with a report
> >> that the DDP package has already been downloaded. Then, PFs will enter
> >> safe mode as they realize that the package on the device does not meet
> >> the minimum version requirement to load. The reported error messages are
> >> confusing, as they indicate the version of the default "safe mode"
> >> package in the NVM, rather than the version of the DDP package loaded
> >> from the filesystem.
> >>
> >> Instead, we need to trigger core reset to clear global configuration.
> >> This is the lowest level of hardware reset which clears the global
> >> configuration lock and related state. It also clears any already
> >> downloaded DDP. Crucially, it does *not* clear the Tx scheduler topology
> >> configuration.
> >>
> >> Refactor ice_cfg_tx_topo() to always trigger a core reset after acquiring
> >> the global lock, regardless of success or failure of the topology
> >> configuration.
> >>
> >> We need to re-initialize the HW structure when we trigger the core reset.
> >> Previously, this was the responsibility of the core driver to cleanup
> >> after the core reset. Instead, make it the responsibility of this
> >> function. This avoids needless re-initialization for the cases where no
> >> reset occurred.
> >>
> >> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >> ---
> >>  drivers/net/intel/ice/base/ice_ddp.c | 34 ++++++++++++++++++----------
> >>  1 file changed, 22 insertions(+), 12 deletions(-)
> >>
> > 
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > 
> > See one comment inline below.
> > 
> > 
> >> diff --git a/drivers/net/intel/ice/base/ice_ddp.c b/drivers/net/intel/ice/base/ice_ddp.c
> >> index 850c722a3f..68e75be4d2 100644
> >> --- a/drivers/net/intel/ice/base/ice_ddp.c
> >> +++ b/drivers/net/intel/ice/base/ice_ddp.c
> >> @@ -2370,7 +2370,7 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
> >>  	struct ice_buf_hdr *section;
> >>  	struct ice_pkg_hdr *pkg_hdr;
> >>  	enum ice_ddp_state state;
> >> -	u16 i, size = 0, offset;
> >> +	u16 size = 0, offset;
> >>  	u32 reg = 0;
> >>  	int status;
> >>  	u8 flags;
> >> @@ -2457,25 +2457,35 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
> >>  	/* check reset was triggered already or not */
> >>  	reg = rd32(hw, GLGEN_RSTAT);
> >>  	if (reg & GLGEN_RSTAT_DEVSTATE_M) {
> >> -		/* Reset is in progress, re-init the hw again */
> >>  		ice_debug(hw, ICE_DBG_INIT, "Reset is in progress. layer topology might be applied already\n");
> >>  		ice_check_reset(hw);
> >> -		return 0;
> >> +		/* Reset is in progress, re-init the hw again */
> >> +		goto reinit_hw;
> >>  	}
> >>  
> >>  	/* set new topology */
> >>  	status = ice_get_set_tx_topo(hw, new_topo, size, NULL, NULL, true);
> >>  	if (status) {
> >> -		ice_debug(hw, ICE_DBG_INIT, "Set tx topology is failed\n");
> >> -		return status;
> >> +		ice_debug(hw, ICE_DBG_INIT, "Failed setting Tx topology, status %d\n",
> >> +			  status);
> >> +		status = ICE_ERR_CFG;
> >>  	}
> >>  
> >> -	/* new topology is updated, delay 1 second before issuing the CORRER */
> >> -	for (i = 0; i < 10; i++)
> >> -		ice_msec_delay(100, true);
> >> +	/* Even if Tx topology config failed, we need to CORE reset here to
> >> +	 * clear the global configuration lock. Delay 1 second to allow
> >> +	 * hardware to settle then issue a CORER
> >> +	 */
> >> +	ice_msec_delay(1000, true);
> >>  	ice_reset(hw, ICE_RESET_CORER);
> >> -	/* CORER will clear the global lock, so no explicit call
> >> -	 * required for release
> >> -	 */
> >> -	return 0;
> >> +	ice_check_reset(hw);
> >> +
> >> +reinit_hw:
> >> +	/* Since we triggered a CORER, re-initialize hardware */
> >> +	ice_deinit_hw(hw);
> >> +	if (ice_init_hw(hw)) {
> >> +		ice_debug(hw, ICE_DBG_INIT, "Failed to re-init hardware after setting Tx topology\n");
> >> +		return ICE_ERR_RESET_FAILED;
> >> +	}
> >> +
> >> +	return status;
> >>  }
> > 
> > There is a similer deinit + init combo in ice_init_pkg in the same file,
> > which is the only use of this function (in DPDK anyway). Therefore, I
> > believe that that code can be removed now that the reinit is handled by the
> > cfg_tx_topo() function.
> 
> I am not sure we can do that. We program Tx topology by taking the
> global configuration lock, then programming the topology, and then
> performing a CORE reset. We can't simply release the global config lock,
> because then DDP won't be able to download at all. We can't avoid this
> re-init, because after a CORE reset, the drivers AQ setup will be hosed
> and thus we need a re-init.
> 
> ice_init_pkg should be called after Tx topology is executed. If another
> reset occurs there, I think you may still need to do that re-init too.
> There could possibly be a complete refactor that removed the need for a
> double reset, but I am not certain.

Ok, thanks for the analysis and explanation. Taking this patch as-is.

/Bruce

RE: [PATCH v1 09/12] net/ice/base: improve global config lock behavior

[Keller, Jacob E]

> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Monday, September 8, 2025 2:00 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH v1 09/12] net/ice/base: improve global config lock behavior
> 
> On Fri, Sep 05, 2025 at 04:25:19PM -0700, Jacob Keller wrote:
> >
> >
> > On 9/5/2025 8:32 AM, Bruce Richardson wrote:
> > > On Tue, Sep 02, 2025 at 06:26:59PM +0100, Anatoly Burakov wrote:
> > >> From: Jacob Keller <jacob.e.keller@intel.com>
> > >>
> > >> The ice_cfg_tx_topo function attempts to apply Tx scheduler topology
> > >> configuration based on NVM parameters, selecting either a 5 or 9 layer
> > >> topology.
> > >>
> > >> As part of this flow, the driver acquires the "Global Configuration
> > >> Lock", which is a hardware resource associated with programming the DDP
> > >> package to the device. This "lock" is implemented by firmware as a way to
> > >> guarantee that only one PF can program the DDP for a device. Unlike a
> > >> traditional lock, once a PF has acquired this lock, no other PF will be
> > >> able to acquire it again (including that PF) until a core reset of the
> > >> device. Future requests to acquire the lock report that global
> > >> configuration has already completed.
> > >>
> > >> The following flow is used to program the Tx topology:
> > >>
> > >>  * Read the DDP package for scheduler configuration data
> > >>  * Acquire the global configuration lock
> > >>  * Program Tx scheduler topology according to DDP package data
> > >>  * Trigger a core reset which clears the global configuration lock
> > >>
> > >> This is followed by the flow for programming the DDP package:
> > >>
> > >>  * Acquire the global configuration lock (again)
> > >>  * Download the DDP package to the device
> > >>  * Release the global configuration lock.
> > >>
> > >> However, if configuration of the Tx topology fails, (i.e.
> > >> ice_get_set_tx_topo() returns an error code), the driver exits
> > >> ice_cfg_tx_topo() immediately, and fails to trigger core reset.
> > >>
> > >> While the global configuration lock is held, the firmware rejects most
> > >> AdminQ commands, as it is waiting for the DDP package download (or Tx
> > >> scheduler topology programming) to occur.
> > >>
> > >> The current driver flows assume that the global configuration lock has
> > >> been reset after programming the Tx topology. Thus, the same PF attempts
> > >> to acquire the global lock again, and fails. This results in the driver
> > >> reporting "an unknown error occurred when loading the DDP package". It
> > >> then attempts to enter safe mode, but ultimately fails to finish
> > >> ice_probe() since nearly all AdminQ command report error codes, and the
> > >> driver stops loading the device at some point during its initialization.
> > >>
> > >> We cannot simply release the global lock after a failed call to
> > >> ice_get_set_tx_topo(). Releasing the lock indicates to firmware that
> > >> global configuration (downloading of the DDP) has completed. Future
> > >> attempts by this or other PFs to load the DDP will fail with a report
> > >> that the DDP package has already been downloaded. Then, PFs will enter
> > >> safe mode as they realize that the package on the device does not meet
> > >> the minimum version requirement to load. The reported error messages are
> > >> confusing, as they indicate the version of the default "safe mode"
> > >> package in the NVM, rather than the version of the DDP package loaded
> > >> from the filesystem.
> > >>
> > >> Instead, we need to trigger core reset to clear global configuration.
> > >> This is the lowest level of hardware reset which clears the global
> > >> configuration lock and related state. It also clears any already
> > >> downloaded DDP. Crucially, it does *not* clear the Tx scheduler topology
> > >> configuration.
> > >>
> > >> Refactor ice_cfg_tx_topo() to always trigger a core reset after acquiring
> > >> the global lock, regardless of success or failure of the topology
> > >> configuration.
> > >>
> > >> We need to re-initialize the HW structure when we trigger the core reset.
> > >> Previously, this was the responsibility of the core driver to cleanup
> > >> after the core reset. Instead, make it the responsibility of this
> > >> function. This avoids needless re-initialization for the cases where no
> > >> reset occurred.
> > >>
> > >> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > >> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > >> ---
> > >>  drivers/net/intel/ice/base/ice_ddp.c | 34 ++++++++++++++++++----------
> > >>  1 file changed, 22 insertions(+), 12 deletions(-)
> > >>
> > >
> > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > >
> > > See one comment inline below.
> > >
> > >
> > >> diff --git a/drivers/net/intel/ice/base/ice_ddp.c
> b/drivers/net/intel/ice/base/ice_ddp.c
> > >> index 850c722a3f..68e75be4d2 100644
> > >> --- a/drivers/net/intel/ice/base/ice_ddp.c
> > >> +++ b/drivers/net/intel/ice/base/ice_ddp.c
> > >> @@ -2370,7 +2370,7 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32
> len)
> > >>  	struct ice_buf_hdr *section;
> > >>  	struct ice_pkg_hdr *pkg_hdr;
> > >>  	enum ice_ddp_state state;
> > >> -	u16 i, size = 0, offset;
> > >> +	u16 size = 0, offset;
> > >>  	u32 reg = 0;
> > >>  	int status;
> > >>  	u8 flags;
> > >> @@ -2457,25 +2457,35 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf,
> u32 len)
> > >>  	/* check reset was triggered already or not */
> > >>  	reg = rd32(hw, GLGEN_RSTAT);
> > >>  	if (reg & GLGEN_RSTAT_DEVSTATE_M) {
> > >> -		/* Reset is in progress, re-init the hw again */
> > >>  		ice_debug(hw, ICE_DBG_INIT, "Reset is in progress. layer topology
> might be applied already\n");
> > >>  		ice_check_reset(hw);
> > >> -		return 0;
> > >> +		/* Reset is in progress, re-init the hw again */
> > >> +		goto reinit_hw;
> > >>  	}
> > >>
> > >>  	/* set new topology */
> > >>  	status = ice_get_set_tx_topo(hw, new_topo, size, NULL, NULL, true);
> > >>  	if (status) {
> > >> -		ice_debug(hw, ICE_DBG_INIT, "Set tx topology is failed\n");
> > >> -		return status;
> > >> +		ice_debug(hw, ICE_DBG_INIT, "Failed setting Tx topology, status
> %d\n",
> > >> +			  status);
> > >> +		status = ICE_ERR_CFG;
> > >>  	}
> > >>
> > >> -	/* new topology is updated, delay 1 second before issuing the CORRER */
> > >> -	for (i = 0; i < 10; i++)
> > >> -		ice_msec_delay(100, true);
> > >> +	/* Even if Tx topology config failed, we need to CORE reset here to
> > >> +	 * clear the global configuration lock. Delay 1 second to allow
> > >> +	 * hardware to settle then issue a CORER
> > >> +	 */
> > >> +	ice_msec_delay(1000, true);
> > >>  	ice_reset(hw, ICE_RESET_CORER);
> > >> -	/* CORER will clear the global lock, so no explicit call
> > >> -	 * required for release
> > >> -	 */
> > >> -	return 0;
> > >> +	ice_check_reset(hw);
> > >> +
> > >> +reinit_hw:
> > >> +	/* Since we triggered a CORER, re-initialize hardware */
> > >> +	ice_deinit_hw(hw);
> > >> +	if (ice_init_hw(hw)) {
> > >> +		ice_debug(hw, ICE_DBG_INIT, "Failed to re-init hardware after
> setting Tx topology\n");
> > >> +		return ICE_ERR_RESET_FAILED;
> > >> +	}
> > >> +
> > >> +	return status;
> > >>  }
> > >
> > > There is a similer deinit + init combo in ice_init_pkg in the same file,
> > > which is the only use of this function (in DPDK anyway). Therefore, I
> > > believe that that code can be removed now that the reinit is handled by the
> > > cfg_tx_topo() function.
> >
> > I am not sure we can do that. We program Tx topology by taking the
> > global configuration lock, then programming the topology, and then
> > performing a CORE reset. We can't simply release the global config lock,
> > because then DDP won't be able to download at all. We can't avoid this
> > re-init, because after a CORE reset, the drivers AQ setup will be hosed
> > and thus we need a re-init.
> >
> > ice_init_pkg should be called after Tx topology is executed. If another
> > reset occurs there, I think you may still need to do that re-init too.
> > There could possibly be a complete refactor that removed the need for a
> > double reset, but I am not certain.
> 
> Ok, thanks for the analysis and explanation. Taking this patch as-is.
> 
> /Bruce

No problem. The flow here is a mess because of the complexity of the global config "lock" which does more than just serialize access.

Thanks,
Jake

[PATCH v1 10/12] net/ice: count drop-all filter stats in Rx stats

[Anatoly Burakov]

From: Shaiq Wani <shaiq.wani@intel.com>

Packets dropped in the receive direction are counted as
"rx_unknown_protocol" (GLSWID_RUPP),these packets need to be added to the
"ipackets" and the GLV_GORCH/GLV_GORCL counters need to be added to
rx_bytes.

Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/ice_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index 513777e372..0ebe58f858 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -6383,12 +6383,13 @@ ice_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 
 	stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
 			  pf->main_vsi->eth_stats.rx_multicast +
+			  ns->eth.rx_unknown_protocol +
 			  pf->main_vsi->eth_stats.rx_broadcast -
 			  pf->main_vsi->eth_stats.rx_discards;
 	stats->opackets = ns->eth.tx_unicast +
 			  ns->eth.tx_multicast +
 			  ns->eth.tx_broadcast;
-	stats->ibytes   = pf->main_vsi->eth_stats.rx_bytes;
+	stats->ibytes   = ns->eth.rx_bytes;
 	stats->obytes   = ns->eth.tx_bytes;
 	stats->oerrors  = ns->eth.tx_errors +
 			  pf->main_vsi->eth_stats.tx_errors;
-- 
2.47.3

Re: [PATCH v1 10/12] net/ice: count drop-all filter stats in Rx stats

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:27:00PM +0100, Anatoly Burakov wrote:
> From: Shaiq Wani <shaiq.wani@intel.com>
> 
> Packets dropped in the receive direction are counted as
> "rx_unknown_protocol" (GLSWID_RUPP),these packets need to be added to the
> "ipackets" and the GLV_GORCH/GLV_GORCL counters need to be added to
> rx_bytes.
> 

I assume from the code below that for the latter change, we don't actually
need to add them, but instead can just get them already included by reading
a different stats source?

> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/ice/ice_ethdev.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
> index 513777e372..0ebe58f858 100644
> --- a/drivers/net/intel/ice/ice_ethdev.c
> +++ b/drivers/net/intel/ice/ice_ethdev.c
> @@ -6383,12 +6383,13 @@ ice_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>  
>  	stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
>  			  pf->main_vsi->eth_stats.rx_multicast +
> +			  ns->eth.rx_unknown_protocol +
>  			  pf->main_vsi->eth_stats.rx_broadcast -
>  			  pf->main_vsi->eth_stats.rx_discards;
>  	stats->opackets = ns->eth.tx_unicast +
>  			  ns->eth.tx_multicast +
>  			  ns->eth.tx_broadcast;
> -	stats->ibytes   = pf->main_vsi->eth_stats.rx_bytes;
> +	stats->ibytes   = ns->eth.rx_bytes;
>  	stats->obytes   = ns->eth.tx_bytes;
>  	stats->oerrors  = ns->eth.tx_errors +
>  			  pf->main_vsi->eth_stats.tx_errors;
> -- 
> 2.47.3
> 

Re: [PATCH v1 10/12] net/ice: count drop-all filter stats in Rx stats

[Shaw, Jeffrey B]

On 9/5/2025 8:34 AM, Bruce Richardson wrote:
> On Tue, Sep 02, 2025 at 06:27:00PM +0100, Anatoly Burakov wrote:
>> From: Shaiq Wani <shaiq.wani@intel.com>
>>
>> Packets dropped in the receive direction are counted as
>> "rx_unknown_protocol" (GLSWID_RUPP),these packets need to be added to the
>> "ipackets" and the GLV_GORCH/GLV_GORCL counters need to be added to
>> rx_bytes.
>>
> 
> I assume from the code below that for the latter change, we don't actually
> need to add them, but instead can just get them already included by reading
> a different stats source?

Correct, those statistics are not counted in the vsi stats but they are 
counted in the pf stats.

> 
>> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
>> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>   drivers/net/intel/ice/ice_ethdev.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
>> index 513777e372..0ebe58f858 100644
>> --- a/drivers/net/intel/ice/ice_ethdev.c
>> +++ b/drivers/net/intel/ice/ice_ethdev.c
>> @@ -6383,12 +6383,13 @@ ice_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>>   
>>   	stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
>>   			  pf->main_vsi->eth_stats.rx_multicast +
>> +			  ns->eth.rx_unknown_protocol +
>>   			  pf->main_vsi->eth_stats.rx_broadcast -
>>   			  pf->main_vsi->eth_stats.rx_discards;
>>   	stats->opackets = ns->eth.tx_unicast +
>>   			  ns->eth.tx_multicast +
>>   			  ns->eth.tx_broadcast;
>> -	stats->ibytes   = pf->main_vsi->eth_stats.rx_bytes;
>> +	stats->ibytes   = ns->eth.rx_bytes;
>>   	stats->obytes   = ns->eth.tx_bytes;
>>   	stats->oerrors  = ns->eth.tx_errors +
>>   			  pf->main_vsi->eth_stats.tx_errors;
>> -- 
>> 2.47.3
>>

Re: [PATCH v1 10/12] net/ice: count drop-all filter stats in Rx stats

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:27:00PM +0100, Anatoly Burakov wrote:
> From: Shaiq Wani <shaiq.wani@intel.com>
> 
> Packets dropped in the receive direction are counted as
> "rx_unknown_protocol" (GLSWID_RUPP),these packets need to be added to the
> "ipackets" and the GLV_GORCH/GLV_GORCL counters need to be added to
> rx_bytes.
> 
> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/ice/ice_ethdev.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v1 11/12] net/ice/base: add E835 device ID's

[Anatoly Burakov]

E835 is an enhanced version of the E830, and uses the same interfaces as
E830.

Following device IDs are added:
- 0x1248: Intel(R) Ethernet Controller E835-CC for backplane
- 0x1249: Intel(R) Ethernet Controller E835-CC for QSFP
- 0x124A: Intel(R) Ethernet Controller E835-CC for SFP
- 0x1261: Intel(R) Ethernet Controller E835-C for backplane
- 0x1262: Intel(R) Ethernet Controller E835-C for QSFP
- 0x1263: Intel(R) Ethernet Controller E835-C for SFP
- 0x1265: Intel(R) Ethernet Controller E835-L for backplane
- 0x1266: Intel(R) Ethernet Controller E835-L for QSFP
- 0x1267: Intel(R) Ethernet Controller E835-L for SFP

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/ice_common.c |  9 +++++++++
 drivers/net/intel/ice/base/ice_devids.h | 18 ++++++++++++++++++
 drivers/net/intel/ice/ice_ethdev.c      |  9 +++++++++
 3 files changed, 36 insertions(+)

diff --git a/drivers/net/intel/ice/base/ice_common.c b/drivers/net/intel/ice/base/ice_common.c
index 8be56f0aab..d9520097bd 100644
--- a/drivers/net/intel/ice/base/ice_common.c
+++ b/drivers/net/intel/ice/base/ice_common.c
@@ -189,6 +189,15 @@ static int ice_set_mac_type(struct ice_hw *hw)
 	case ICE_DEV_ID_E830_L_QSFP:
 	case ICE_DEV_ID_E830C_SFP:
 	case ICE_DEV_ID_E830_L_SFP:
+	case ICE_DEV_ID_E835CC_BACKPLANE:
+	case ICE_DEV_ID_E835CC_QSFP56:
+	case ICE_DEV_ID_E835CC_SFP:
+	case ICE_DEV_ID_E835C_BACKPLANE:
+	case ICE_DEV_ID_E835C_QSFP:
+	case ICE_DEV_ID_E835C_SFP:
+	case ICE_DEV_ID_E835_L_BACKPLANE:
+	case ICE_DEV_ID_E835_L_QSFP:
+	case ICE_DEV_ID_E835_L_SFP:
 		hw->mac_type = ICE_MAC_E830;
 		break;
 	default:
diff --git a/drivers/net/intel/ice/base/ice_devids.h b/drivers/net/intel/ice/base/ice_devids.h
index 20c6dbd4a5..9a38ebf15b 100644
--- a/drivers/net/intel/ice/base/ice_devids.h
+++ b/drivers/net/intel/ice/base/ice_devids.h
@@ -35,6 +35,24 @@
 #define ICE_DEV_ID_E830C_SFP            0x12DA
 /* Intel(R) Ethernet Controller E830-L for SFP */
 #define ICE_DEV_ID_E830_L_SFP         0x12DE
+/* Intel(R) Ethernet Controller E835-CC for backplane */
+#define ICE_DEV_ID_E835CC_BACKPLANE	0x1248
+/* Intel(R) Ethernet Controller E835-CC for QSFP */
+#define ICE_DEV_ID_E835CC_QSFP56	0x1249
+/* Intel(R) Ethernet Controller E835-CC for SFP */
+#define ICE_DEV_ID_E835CC_SFP		0x124A
+/* Intel(R) Ethernet Controller E835-C for backplane */
+#define ICE_DEV_ID_E835C_BACKPLANE	0x1261
+/* Intel(R) Ethernet Controller E835-C for QSFP */
+#define ICE_DEV_ID_E835C_QSFP		0x1262
+/* Intel(R) Ethernet Controller E835-C for SFP */
+#define ICE_DEV_ID_E835C_SFP		0x1263
+/* Intel(R) Ethernet Controller E835-L for backplane */
+#define ICE_DEV_ID_E835_L_BACKPLANE	0x1265
+/* Intel(R) Ethernet Controller E835-L for QSFP */
+#define ICE_DEV_ID_E835_L_QSFP		0x1266
+/* Intel(R) Ethernet Controller E835-L for SFP */
+#define ICE_DEV_ID_E835_L_SFP		0x1267
 /* Intel(R) Ethernet Controller E810-C for backplane */
 #define ICE_DEV_ID_E810C_BACKPLANE	0x1591
 /* Intel(R) Ethernet Controller E810-C for QSFP */
diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index 0ebe58f858..ee1adb6b67 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -237,6 +237,15 @@ static const struct rte_pci_id pci_id_ice_map[] = {
 	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E830_L_QSFP) },
 	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E830C_SFP) },
 	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E830_L_SFP) },
+	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E835CC_BACKPLANE), },
+	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E835CC_QSFP56), },
+	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E835CC_SFP), },
+	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E835C_BACKPLANE), },
+	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E835C_QSFP), },
+	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E835C_SFP), },
+	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E835_L_BACKPLANE), },
+	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E835_L_QSFP), },
+	{ RTE_PCI_DEVICE(ICE_INTEL_VENDOR_ID, ICE_DEV_ID_E835_L_SFP), },
 	{ .vendor_id = 0, /* sentinel */ },
 };
 
-- 
2.47.3

Re: [PATCH v1 11/12] net/ice/base: add E835 device ID's

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:27:01PM +0100, Anatoly Burakov wrote:
> E835 is an enhanced version of the E830, and uses the same interfaces as
> E830.
> 
> Following device IDs are added:
> - 0x1248: Intel(R) Ethernet Controller E835-CC for backplane
> - 0x1249: Intel(R) Ethernet Controller E835-CC for QSFP
> - 0x124A: Intel(R) Ethernet Controller E835-CC for SFP
> - 0x1261: Intel(R) Ethernet Controller E835-C for backplane
> - 0x1262: Intel(R) Ethernet Controller E835-C for QSFP
> - 0x1263: Intel(R) Ethernet Controller E835-C for SFP
> - 0x1265: Intel(R) Ethernet Controller E835-L for backplane
> - 0x1266: Intel(R) Ethernet Controller E835-L for QSFP
> - 0x1267: Intel(R) Ethernet Controller E835-L for SFP
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v1 12/12] net/ice: update README

[Anatoly Burakov]

Update the README with date of latest driver snapshot.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/intel/ice/base/README | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/intel/ice/base/README b/drivers/net/intel/ice/base/README
index 86f69d1d04..98182a8da6 100644
--- a/drivers/net/intel/ice/base/README
+++ b/drivers/net/intel/ice/base/README
@@ -1,12 +1,12 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2020-2024 Intel Corporation
+ * Copyright(c) 2020-2025 Intel Corporation
  */
 
 Intel® ICE driver
 ==================
 
 This directory contains source code of ice base driver generated on
-2025-05-23 released by the team which develops
+2025-09-01 released by the team which develops
 basic drivers for any ice NIC. The directory of base/ contains the
 original source package.
 This driver is valid for the product(s) listed below
-- 
2.47.3

Re: [PATCH v1 12/12] net/ice: update README

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:27:02PM +0100, Anatoly Burakov wrote:
> Update the README with date of latest driver snapshot.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/ice/base/README | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH v1 00/12] net/ice: update to latest version

[Bruce Richardson]

On Tue, Sep 02, 2025 at 06:26:50PM +0100, Anatoly Burakov wrote:
> This patch adds an assortment of fixes and updates for net/ice driver,
> bringing it up to date with latest shared code.
> 
> This patchset includes fixes submitted by Shaiq Wani [1], with a small change of
> dropping a direction lookup flag as it's not necessary.
> 
> [1] https://patches.dpdk.org/project/dpdk/cover/20250731044132.110174-1-shaiq.wani@intel.com/
> 
Series applied to dpdk-next-net-intel.

Thanks,
/Bruce