net/ice: update to latest version

From: Marcin Szycik <[email protected]>

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 <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/base/ice_protocol_type.h |  1 +
 drivers/net/intel/ice/base/ice_switch.c        | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

From: Shaiq Wani <[email protected]>

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 [email protected]

Signed-off-by Jeff Shaw <[email protected]>

Signed-off-by Shaiq Wani <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/base/ice_switch.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

From: Jacob Keller <[email protected]>

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 [email protected]

Signed-off-by Jacob Keller <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/base/ice_flow.c | 4 ----
 1 file changed, 4 deletions(-)

From: Pandi Kumar Maharajan <[email protected]>

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 [email protected]

Signed-off-by Pandi Kumar Maharajan <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---

From: Przemek Kitszel <[email protected]>

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 <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---

From: Jaroslaw Ilgiewicz <[email protected]>

Added 40G speed for port options

Signed-off-by Jaroslaw Ilgiewicz <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---

From: Pandi Kumar Maharajan <[email protected]>

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 <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 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(-)

From: Stefan Wegrzyn <[email protected]>

Added missing defines for commands related to health status.

Signed-off-by Stefan Wegrzyn <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/base/ice_adminq_cmd.h | 2 ++
 1 file changed, 2 insertions(+)

From: Jacob Keller <[email protected]>

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 <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/base/ice_ddp.c | 34 ++++++++++++++++++----------
 1 file changed, 22 insertions(+), 12 deletions(-)

drivers/net/intel/ice/base/ice_ddp.c +13 -12

index 850c722a3f..68e75be4d2 100644
@@ -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);
+

Quoted 71 lines · Anatoly Burakov

From: Jacob Keller <[email protected]>

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.

Quoted 6 lines

Signed-off-by Jacob Keller <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/base/ice_ddp.c | 34 ++++++++++++++++++----------
 1 file changed, 22 insertions(+), 12 deletions(-)

Acked-by Bruce Richardson <[email protected]>

See one comment inline below.

Quoted 57 lines

drivers/net/intel/ice/base/ice_ddp.c +20 -12

index 850c722a3f..68e75be4d2 100644
@@ -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;
+	}

drivers/net/intel/ice/base/ice_ddp.c int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len) · line 2491

Quoted 3 lines

drivers/net/intel/ice/base/ice_ddp.c +2 -0

+
+	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.

Quoted 141 lines · Bruce Richardson

Quoted 71 lines · Anatoly Burakov

From: Jacob Keller <[email protected]>

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.

Quoted 6 lines

Signed-off-by Jacob Keller <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/base/ice_ddp.c | 34 ++++++++++++++++++----------
 1 file changed, 22 insertions(+), 12 deletions(-)

Acked-by Bruce Richardson <[email protected]>

See one comment inline below.

Quoted 57 lines

drivers/net/intel/ice/base/ice_ddp.c +20 -12

index 850c722a3f..68e75be4d2 100644
@@ -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;
+	}

Quoted 8 lines

drivers/net/intel/ice/base/ice_ddp.c int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len) · line 2491

Quoted 3 lines

drivers/net/intel/ice/base/ice_ddp.c +2 -0

+
+	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.

-----Original Message-----
From: Richardson, Bruce <[email protected]>
Sent: Monday, September 8, 2025 2:00 AM
To: Keller, Jacob E <[email protected]>
Cc: Burakov, Anatoly <[email protected]>; [email protected]
Subject: Re: [PATCH v1 09/12] net/ice/base: improve global config lock behavior

Quoted 82 lines · Jacob Keller

Quoted 79 lines · Bruce Richardson

Quoted 77 lines · Anatoly Burakov

From: Jacob Keller <[email protected]>

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 <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/base/ice_ddp.c | 34 ++++++++++++++++++----------
 1 file changed, 22 insertions(+), 12 deletions(-)

Quoted 6 lines

Quoted 6 lines

Acked-by Bruce Richardson <[email protected]>

See one comment inline below.

Review

Quoted 1 line

(quoted diff) +0 -0

b/drivers/net/intel/ice/base/ice_ddp.c

Quoted 4 lines

Quoted 4 lines

drivers/net/intel/ice/base/ice_ddp.c int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32

Quoted 4 lines

index 850c722a3f..68e75be4d2 100644

drivers/net/intel/ice/base/ice_ddp.c +0 -0

@@ -2370,7 +2370,7 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32

len)

Quoted 9 lines

Quoted 9 lines

drivers/net/intel/ice/base/ice_ddp.c int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf,

Quoted 9 lines

drivers/net/intel/ice/base/ice_ddp.c +1 -1

 	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)

Quoted 5 lines

Quoted 5 lines

drivers/net/intel/ice/base/ice_ddp.c int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)

Quoted 5 lines

drivers/net/intel/ice/base/ice_ddp.c +0 -1

 	/* 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");

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

drivers/net/intel/ice/base/ice_ddp.c +2 -1

 		ice_check_reset(hw);
-		return 0;
+		/* Reset is in progress, re-init the hw again */
+		goto reinit_hw;
 	}
 

Quoted 6 lines

Quoted 6 lines

drivers/net/intel/ice/base/ice_ddp.c int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)

Quoted 6 lines

drivers/net/intel/ice/base/ice_ddp.c +1 -2

 	/* 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",

Quoted 16 lines

Quoted 16 lines

Quoted 16 lines

drivers/net/intel/ice/base/ice_ddp.c +7 -6

+			  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
-	 */

Quoted 8 lines

Quoted 8 lines

drivers/net/intel/ice/base/ice_ddp.c int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)

Quoted 8 lines

drivers/net/intel/ice/base/ice_ddp.c +7 -1

-	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");

Quoted 18 lines

Quoted 10 lines

drivers/net/intel/ice/base/ice_ddp.c int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len) · line 2491

Quoted 5 lines

drivers/net/intel/ice/base/ice_ddp.c +4 -0

+		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.

Quoted 4 lines

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

From: Shaiq Wani <[email protected]>

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 <[email protected]>

Signed-off-by Shaiq Wani <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/ice_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/net/intel/ice/ice_ethdev.c +2 -1

index 513777e372..0ebe58f858 100644
@@ -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;

Signature · 1 line

2.47.3

Quoted 7 lines · Anatoly Burakov

From: Shaiq Wani <[email protected]>

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?

Quoted 30 lines

Signed-off-by Jeff Shaw <[email protected]>

Signed-off-by Shaiq Wani <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
  drivers/net/intel/ice/ice_ethdev.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

drivers/net/intel/ice/ice_ethdev.c +2 -1

index 513777e372..0ebe58f858 100644
@@ -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;

Signature · 1 line

2.47.3

From: Shaiq Wani <[email protected]>

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 <[email protected]>

Signed-off-by Shaiq Wani <[email protected]>

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/ice_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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 <[email protected]>

---

Update the README with date of latest driver snapshot.

Signed-off-by Anatoly Burakov <[email protected]>

---
 drivers/net/intel/ice/base/README | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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/[email protected]/