[PATCH net v3 0/2] net: stmmac: restore previous state if tc_setup_dwmac510_mqprio() fails

[PATCH net v3 0/2] net: stmmac: restore previous state if tc_setup_dwmac510_mqprio() fails

[Lorenzo Bianconi]

Restore previous mqprio qdisc configuration if
tc_setup_dwmac510_mqprio() fails running the following configuration:

  $tc qdisc add dev eth0 root handle 1: mqprio queues 2@0 2@2
  $tc qdisc replace dev eth0 root handle 2: mqprio queues 2@0 2@2 fp E P

Propagate FPE preemption-class mapping errors in
tc_setup_dwmac510_mqprio() and tc_taprio_configure().

---
Changes in v3:
- Fix commit message.
- Rebase on top of net main branch.
- Add log in case of netif_set_real_num_tx_queues() failures in
  tc_setup_dwmac510_mqprio() error path.
- Link to v2: https://lore.kernel.org/r/20260909-stmmac-tc_setup_dwmac510_mqprio-error-path-v2-0-8158b8333b3d@oss.qualcomm.com

Changes in v2:
- Move stmmac_fpe_map_preemption_class macro definition in a dedicated
  patch.
- Improve commit message providing a configuration example.
- Link to v1: https://lore.kernel.org/r/20260903-stmmac-tc_setup_dwmac510_mqprio-error-path-v1-1-eb171bdd340a@oss.qualcomm.com

---
Lorenzo Bianconi (2):
      net: stmmac: propagate FPE preemption-class mapping errors
      net: stmmac: preserve real_num_tx_queues on mqprio setup failure

 drivers/net/ethernet/stmicro/stmmac/hwif.h      |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 99 +++++++++++++++++--------
 2 files changed, 71 insertions(+), 30 deletions(-)
---
base-commit: 78445023439506ebd83b86d40b1e428a3b309d4a
change-id: 20260903-stmmac-tc_setup_dwmac510_mqprio-error-path-8d472a43d3f2

Best regards,
-- 
Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

[PATCH net v3 1/2] net: stmmac: propagate FPE preemption-class mapping errors

[Lorenzo Bianconi]

stmmac_fpe_map_preemption_class() dispatches through the
stmmac_do_void_callback() helper, which forces the callback's return
value to 0 whenever the op pointer is populated. As a result the
-EINVAL returned by dwmac5_fpe_map_preemption_class() (e.g. when a
preemptible TC owns more than one TXQ under SP scheduling) is silently
swallowed by every caller.

Switch the dispatch macro to stmmac_do_callback() so the callback's real
result is propagated, and honour it in the taprio and mqprio qdisc
offload.

Note that the taprio "if (ret)" check in tc_taprio_configure() used to
be dead code and now becomes live: a preemptible TC spanning more than
one TXQ under SP scheduling cannot be programmed in hardware, so a
taprio or mqprio configuration that previously returned success while
leaving the preemption-class register unprogrammed now fails with
-EINVAL. For taprio, the failure also runs the disable path, tearing
down the schedule that was just installed; this is the intended
behaviour.

Fixes: 195e4f409a40 ("net: stmmac: support fp parameter of tc-mqprio")
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/hwif.h      |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 19 +++++++++----------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 04dafec021b4..9314bcb85c22 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -494,7 +494,7 @@ struct stmmac_ops {
 #define stmmac_set_arp_offload(__priv, __args...) \
 	stmmac_do_void_callback(__priv, mac, set_arp_offload, __args)
 #define stmmac_fpe_map_preemption_class(__priv, __args...) \
-	stmmac_do_void_callback(__priv, mac, fpe_map_preemption_class, __args)
+	stmmac_do_callback(__priv, mac, fpe_map_preemption_class, __args)
 
 /* PTP and HW Timer helpers */
 struct stmmac_hwtimestamp {
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 14cabe76e53e..5398616fcdfe 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -970,7 +970,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
 	struct netlink_ext_ack *extack = qopt->mqprio.extack;
 	struct timespec64 time, current_time, qopt_time;
 	ktime_t current_time_ns;
-	int i, ret = 0;
+	int err, i, ret = 0;
 	u64 ctr;
 
 	if (qopt->base_time < 0)
@@ -1120,9 +1120,9 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
 		mutex_unlock(&priv->est_lock);
 	}
 
-	stmmac_fpe_map_preemption_class(priv, priv->dev, extack, 0);
+	err = stmmac_fpe_map_preemption_class(priv, priv->dev, extack, 0);
 
-	return ret;
+	return qopt->cmd == TAPRIO_CMD_DESTROY ? err : ret;
 }
 
 static void tc_taprio_stats(struct stmmac_priv *priv,
@@ -1237,14 +1237,15 @@ static int tc_query_caps(struct stmmac_priv *priv,
 	}
 }
 
-static void stmmac_reset_tc_mqprio(struct net_device *ndev,
-				   struct netlink_ext_ack *extack)
+static int stmmac_reset_tc_mqprio(struct net_device *ndev,
+				  struct netlink_ext_ack *extack)
 {
 	struct stmmac_priv *priv = netdev_priv(ndev);
 
 	netdev_reset_tc(ndev);
 	netif_set_real_num_tx_queues(ndev, priv->plat->tx_queues_to_use);
-	stmmac_fpe_map_preemption_class(priv, ndev, extack, 0);
+
+	return stmmac_fpe_map_preemption_class(priv, ndev, extack, 0);
 }
 
 static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
@@ -1257,10 +1258,8 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
 	u32 num_tc = qopt->num_tc;
 	int err;
 
-	if (!num_tc) {
-		stmmac_reset_tc_mqprio(ndev, extack);
-		return 0;
-	}
+	if (!num_tc)
+		return stmmac_reset_tc_mqprio(ndev, extack);
 
 	err = netdev_set_num_tc(ndev, num_tc);
 	if (err)

-- 
2.55.0

Re: [PATCH net v3 1/2] net: stmmac: propagate FPE preemption-class mapping errors

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 6571 bytes --]

commenting on sashiko's report:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260911-stmmac-tc_setup_dwmac510_mqprio-error-path-v3-0-a76b1e2547c1%40oss.qualcomm.com

> stmmac_fpe_map_preemption_class() dispatches through the
> stmmac_do_void_callback() helper, which forces the callback's return
> value to 0 whenever the op pointer is populated. As a result the
> -EINVAL returned by dwmac5_fpe_map_preemption_class() (e.g. when a
> preemptible TC owns more than one TXQ under SP scheduling) is silently
> swallowed by every caller.
> 
> Switch the dispatch macro to stmmac_do_callback() so the callback's real
> result is propagated, and honour it in the taprio and mqprio qdisc
> offload.
> 
> Note that the taprio "if (ret)" check in tc_taprio_configure() used to
> be dead code and now becomes live: a preemptible TC spanning more than
> one TXQ under SP scheduling cannot be programmed in hardware, so a
> taprio or mqprio configuration that previously returned success while
> leaving the preemption-class register unprogrammed now fails with
> -EINVAL. For taprio, the failure also runs the disable path, tearing
> down the schedule that was just installed; this is the intended
> behaviour.

- Is the described condition narrower than what the code actually rejects?
  In dwmac5_fpe_map_preemption_class() the SP rejection is not gated on the
  TC being preemptible:
  - I guess we can do a little improvement of the commit message if I need to
    repost.
> 
> Fixes: 195e4f409a40 ("net: stmmac: support fp parameter of tc-mqprio")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

> ---
>  drivers/net/ethernet/stmicro/stmmac/hwif.h      |  2 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 19 +++++++++----------
>  2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 04dafec021b4..9314bcb85c22 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -494,7 +494,7 @@ struct stmmac_ops {
>  #define stmmac_set_arp_offload(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, mac, set_arp_offload, __args)
>  #define stmmac_fpe_map_preemption_class(__priv, __args...) \
> -	stmmac_do_void_callback(__priv, mac, fpe_map_preemption_class, __args)
> +	stmmac_do_callback(__priv, mac, fpe_map_preemption_class, __args)
>  
>  /* PTP and HW Timer helpers */
>  struct stmmac_hwtimestamp {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 14cabe76e53e..5398616fcdfe 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -970,7 +970,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
>  	struct netlink_ext_ack *extack = qopt->mqprio.extack;
>  	struct timespec64 time, current_time, qopt_time;
>  	ktime_t current_time_ns;
> -	int i, ret = 0;
> +	int err, i, ret = 0;
>  	u64 ctr;
>  
>  	if (qopt->base_time < 0)
> @@ -1120,9 +1120,9 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
>  		mutex_unlock(&priv->est_lock);
>  	}
>  
> -	stmmac_fpe_map_preemption_class(priv, priv->dev, extack, 0);
> +	err = stmmac_fpe_map_preemption_class(priv, priv->dev, extack, 0);

- Can a failed TAPRIO_CMD_REPLACE now leave sch_taprio and the hardware out
  of sync?  In tc_taprio_configure() the FPE mapping is validated only after
  the gate control list has already been programmed and enabled:
  	ret = stmmac_est_configure(priv, priv, priv->est,
  				   priv->plat->clk_ptp_rate);
  	mutex_unlock(&priv->est_lock);
  	if (ret) {
  	...
  	ret = stmmac_fpe_map_preemption_class(priv, priv->dev, extack,
  					      qopt->mqprio.preemptible_tcs);
  	if (ret)
  		goto disable;
  That "goto disable" was unreachable for every MAC providing the op while
  the macro forced 0, and becomes live now.  The disable block sets
  priv->est->enable = false, reprograms EST, zeroes
  priv->xstats.max_sdu_txq_drop/mtl_est_txq_hlbf/mtl_est_txq_hlbs and clears
  the preemption class register, and the function returns -EINVAL.
  - I think this issue is not introduced by this patch, but it is already there
    and it should be fixed with a dedicated patch.

- Is this ternary reachable in any form other than "return ret;"?  Following
  the call with pclass == 0:
  - dwmac5_fpe_map_preemption_class() takes "if (!pclass) goto
    update_mapping;" and falls through to "return 0;"
  - dwxgmac3_fpe_map_preemption_class() has no error return at all

  - I think the current implementation is more robust for any future change
    even if in the current code if pclass is 0, it can't return an error code.

>  
> -	return ret;
> +	return qopt->cmd == TAPRIO_CMD_DESTROY ? err : ret;
>  }
>  
>  static void tc_taprio_stats(struct stmmac_priv *priv,
> @@ -1237,14 +1237,15 @@ static int tc_query_caps(struct stmmac_priv *priv,
>  	}
>  }
>  
> -static void stmmac_reset_tc_mqprio(struct net_device *ndev,
> -				   struct netlink_ext_ack *extack)
> +static int stmmac_reset_tc_mqprio(struct net_device *ndev,
> +				  struct netlink_ext_ack *extack)
>  {
>  	struct stmmac_priv *priv = netdev_priv(ndev);
>  
>  	netdev_reset_tc(ndev);
>  	netif_set_real_num_tx_queues(ndev, priv->plat->tx_queues_to_use);
> -	stmmac_fpe_map_preemption_class(priv, ndev, extack, 0);
> +
> +	return stmmac_fpe_map_preemption_class(priv, ndev, extack, 0);
>  }
>  
>  static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
> @@ -1257,10 +1258,8 @@ static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
>  	u32 num_tc = qopt->num_tc;
>  	int err;
>  
> -	if (!num_tc) {
> -		stmmac_reset_tc_mqprio(ndev, extack);
> -		return 0;

- Same question for the mqprio delete path: with pclass == 0 the value
  returned by stmmac_reset_tc_mqprio() can only encode "callback op pointer
  missing", after netdev_reset_tc() and netif_set_real_num_tx_queues() have
  already been applied.  Is propagating it useful here, or would keeping
  "return 0;" after the reset match what the hardware state actually is?
  - Same here, I think the current implementation is more robust for any
    future change.

Regards,
Lorenzo

> -	}
> +	if (!num_tc)
> +		return stmmac_reset_tc_mqprio(ndev, extack);
>  
>  	err = netdev_set_num_tc(ndev, num_tc);
>  	if (err)
> 
> -- 
> 2.55.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH net v3 2/2] net: stmmac: preserve real_num_tx_queues on mqprio setup failure

[Lorenzo Bianconi]

With the FPE preemption-class mapping error now propagated from
stmmac_fpe_map_preemption_class(), tc_setup_dwmac510_mqprio() can fail
on the mapping step. The error path used to call stmmac_reset_tc_mqprio(),
which resets the number of real TX queues to priv->plat->tx_queues_to_use
(the platform maximum), overwriting the value that was active before the
offload was attempted (for example a lower count left over from a previous
mqprio configuration).

The issue can be triggered using the following configuration:

  # First mqprio config lowers the hw queue count below the platform
  # default (e.g. 8 TX queues).
  $tc qdisc add dev eth0 root handle 1: mqprio queues 2@0 2@2

  # Replace mqprio configuration with a second one that fails FPE
  # preemption-class mapping. stmmac driver resets the real_num_tx_queues
  # to the platform maximum, losing the previous configuration.
  $tc qdisc replace dev eth0 root handle 2: mqprio queues 2@0 2@2 fp E P

Save ndev->real_num_tx_queues before lowering it and restore it,
together with the TC-to-queue and priority-to-TC mappings, when the FPE
preemption-class mapping fails, instead of resetting the queue count to
the platform maximum.

Note that a failed setup makes the qdisc layer run mqprio_destroy() on
the new qdisc. Because priv->hw_offload is only assigned after
ndo_setup_tc() succeeds, mqprio_destroy() calls netdev_set_num_tc(dev, 0),
so dev->num_tc ends up 0 regardless of the driver-side restore and the
previous qdisc is not reactivated. The restore is still needed to keep
real_num_tx_queues and to avoid leaving the failed configuration's
TC-to-queue and priority-to-TC mappings in place.

Fixes: 195e4f409a40 ("net: stmmac: support fp parameter of tc-mqprio")
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 82 +++++++++++++++++++------
 1 file changed, 62 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 5398616fcdfe..42a00446e9b4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -1237,6 +1237,30 @@ static int tc_query_caps(struct stmmac_priv *priv,
 	}
 }
 
+static int stmmac_set_ndev_tcs(struct net_device *ndev, u8 ntc,
+			       struct netdev_tc_txq *tc_to_txq)
+{
+	int i, err;
+
+	netdev_reset_tc(ndev);
+	if (!ntc)
+		return 0;
+
+	err = netdev_set_num_tc(ndev, ntc);
+	if (err)
+		return err;
+
+	for (i = 0; i < ntc; i++) {
+		u16 count, offset;
+
+		count = tc_to_txq[i].count;
+		offset = tc_to_txq[i].offset;
+		netdev_set_tc_queue(ndev, i, count, offset);
+	}
+
+	return 0;
+}
+
 static int stmmac_reset_tc_mqprio(struct net_device *ndev,
 				  struct netlink_ext_ack *extack)
 {
@@ -1251,43 +1275,61 @@ static int stmmac_reset_tc_mqprio(struct net_device *ndev,
 static int tc_setup_dwmac510_mqprio(struct stmmac_priv *priv,
 				    struct tc_mqprio_qopt_offload *mqprio)
 {
+	unsigned int ndev_num_tx_queues, num_tx_queues = 0;
+	struct netdev_tc_txq ndev_tc_to_txq[TC_MAX_QUEUE];
+	struct netdev_tc_txq tc_to_txq[TC_MAX_QUEUE] = {};
 	struct netlink_ext_ack *extack = mqprio->extack;
 	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
-	u32 offset, count, num_stack_tx_queues = 0;
 	struct net_device *ndev = priv->dev;
-	u32 num_tc = qopt->num_tc;
-	int err;
+	u8 ndev_prio_tc_map[TC_BITMASK + 1];
+	int i, err, ndev_ntc;
 
-	if (!num_tc)
+	if (!qopt->num_tc)
 		return stmmac_reset_tc_mqprio(ndev, extack);
 
-	err = netdev_set_num_tc(ndev, num_tc);
-	if (err)
-		return err;
-
-	for (u32 tc = 0; tc < num_tc; tc++) {
-		offset = qopt->offset[tc];
-		count = qopt->count[tc];
-		num_stack_tx_queues += count;
+	if (qopt->num_tc > ARRAY_SIZE(tc_to_txq))
+		return -EINVAL;
 
-		err = netdev_set_tc_queue(ndev, tc, count, offset);
-		if (err)
-			goto err_reset_tc;
+	/* save current tc values for reset */
+	ndev_ntc = netdev_get_num_tc(ndev);
+	for (i = 0; i < ARRAY_SIZE(ndev->tc_to_txq); i++)
+		ndev_tc_to_txq[i].combined =
+			READ_ONCE(ndev->tc_to_txq[i].combined);
+	for (i = 0; i < ARRAY_SIZE(ndev_prio_tc_map); i++)
+		ndev_prio_tc_map[i] = READ_ONCE(ndev->prio_tc_map[i]);
+
+	for (i = 0; i < qopt->num_tc; i++) {
+		tc_to_txq[i] = (struct netdev_tc_txq) {
+			.count = qopt->count[i],
+			.offset = qopt->offset[i],
+		};
+		num_tx_queues += qopt->count[i];
 	}
 
-	err = netif_set_real_num_tx_queues(ndev, num_stack_tx_queues);
+	err = stmmac_set_ndev_tcs(ndev, qopt->num_tc, tc_to_txq);
+	if (err)
+		goto error_reset_tc;
+
+	ndev_num_tx_queues = ndev->real_num_tx_queues;
+	err = netif_set_real_num_tx_queues(ndev, num_tx_queues);
 	if (err)
-		goto err_reset_tc;
+		goto error_reset_tc;
 
 	err = stmmac_fpe_map_preemption_class(priv, ndev, extack,
 					      mqprio->preemptible_tcs);
 	if (err)
-		goto err_reset_tc;
+		goto error_reset_num_tx_queues;
 
 	return 0;
 
-err_reset_tc:
-	stmmac_reset_tc_mqprio(ndev, extack);
+error_reset_num_tx_queues:
+	if (netif_set_real_num_tx_queues(ndev, ndev_num_tx_queues))
+		netdev_warn(ndev, "Failed to restore %u TX queues\n",
+			    ndev_num_tx_queues);
+error_reset_tc:
+	stmmac_set_ndev_tcs(ndev, ndev_ntc, ndev_tc_to_txq);
+	for (i = 0; i < ARRAY_SIZE(ndev_prio_tc_map); i++)
+		netdev_set_prio_tc_map(ndev, i, ndev_prio_tc_map[i]);
 
 	return err;
 }

-- 
2.55.0

Re: [PATCH net v3 2/2] net: stmmac: preserve real_num_tx_queues on mqprio setup failure

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 4272 bytes --]

> With the FPE preemption-class mapping error now propagated from
> stmmac_fpe_map_preemption_class(), tc_setup_dwmac510_mqprio() can fail
> on the mapping step. The error path used to call stmmac_reset_tc_mqprio(),
> which resets the number of real TX queues to priv->plat->tx_queues_to_use
> (the platform maximum), overwriting the value that was active before the
> offload was attempted (for example a lower count left over from a previous
> mqprio configuration).
> 
> The issue can be triggered using the following configuration:
> 
>   # First mqprio config lowers the hw queue count below the platform
>   # default (e.g. 8 TX queues).
>   $tc qdisc add dev eth0 root handle 1: mqprio queues 2@0 2@2
> 
>   # Replace mqprio configuration with a second one that fails FPE
>   # preemption-class mapping. stmmac driver resets the real_num_tx_queues
>   # to the platform maximum, losing the previous configuration.
>   $tc qdisc replace dev eth0 root handle 2: mqprio queues 2@0 2@2 fp E P
> 
> Save ndev->real_num_tx_queues before lowering it and restore it,
> together with the TC-to-queue and priority-to-TC mappings, when the FPE
> preemption-class mapping fails, instead of resetting the queue count to
> the platform maximum.
> 
> Note that a failed setup makes the qdisc layer run mqprio_destroy() on
> the new qdisc. Because priv->hw_offload is only assigned after
> ndo_setup_tc() succeeds, mqprio_destroy() calls netdev_set_num_tc(dev, 0),
> so dev->num_tc ends up 0 regardless of the driver-side restore and the
> previous qdisc is not reactivated. The restore is still needed to keep
> real_num_tx_queues and to avoid leaving the failed configuration's
> TC-to-queue and priority-to-TC mappings in place.
> 
> Fixes: 195e4f409a40 ("net: stmmac: support fp parameter of tc-mqprio")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 82 +++++++++++++++++++------
>  1 file changed, 62 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c

commenting on sashiko's report:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260911-stmmac-tc_setup_dwmac510_mqprio-error-path-v3-0-a76b1e2547c1%40oss.qualcomm.com

[...]

> -	err = netif_set_real_num_tx_queues(ndev, num_stack_tx_queues);
> +	err = stmmac_set_ndev_tcs(ndev, qopt->num_tc, tc_to_txq);
> +	if (err)
> +		goto error_reset_tc;
> +
> +	ndev_num_tx_queues = ndev->real_num_tx_queues;
> +	err = netif_set_real_num_tx_queues(ndev, num_tx_queues);
>  	if (err)
> -		goto err_reset_tc;
> +		goto error_reset_tc;
>  
>  	err = stmmac_fpe_map_preemption_class(priv, ndev, extack,
>  					      mqprio->preemptible_tcs);
>  	if (err)
> -		goto err_reset_tc;
> +		goto error_reset_num_tx_queues;
>  
>  	return 0;
>  
> -err_reset_tc:
> -	stmmac_reset_tc_mqprio(ndev, extack);
> +error_reset_num_tx_queues:
> +	if (netif_set_real_num_tx_queues(ndev, ndev_num_tx_queues))
> +		netdev_warn(ndev, "Failed to restore %u TX queues\n",
> +			    ndev_num_tx_queues);
> +error_reset_tc:
> +	stmmac_set_ndev_tcs(ndev, ndev_ntc, ndev_tc_to_txq);
> +	for (i = 0; i < ARRAY_SIZE(ndev_prio_tc_map); i++)
> +		netdev_set_prio_tc_map(ndev, i, ndev_prio_tc_map[i]);

- Is the loss of the FPE reprogramming step on these two labels intentional?
  The old err_reset_tc path went through stmmac_reset_tc_mqprio(), which ends
  with:
  	return stmmac_fpe_map_preemption_class(priv, ndev, extack, 0);
  The new error_reset_num_tx_queues / error_reset_tc labels only touch netdev
  software state (stmmac_set_ndev_tcs() -> netdev_reset_tc() /
  netdev_set_num_tc() / netdev_set_tc_queue(), plus
  netdev_set_prio_tc_map()), so the hardware mapping is never rewritten.
  - I guess we have already discussed about it. If qdisc replace fails, I think
    the driver should restore the previous offloaded hw configuration. It is then
    up to sch_mqprio qdisc to properly restore the logic. Here sch_mqprio does
    not run mqprio_disable_offload() and this one seems a sch_mqprio bug to me.

Regards,
Lorenzo


>  
>  	return err;
>  }
> 
> -- 
> 2.55.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net v3 0/2] net: stmmac: restore previous state if tc_setup_dwmac510_mqprio() fails

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 11 Sep 2026 10:58:28 +0200 you wrote:
> Restore previous mqprio qdisc configuration if
> tc_setup_dwmac510_mqprio() fails running the following configuration:
> 
>   $tc qdisc add dev eth0 root handle 1: mqprio queues 2@0 2@2
>   $tc qdisc replace dev eth0 root handle 2: mqprio queues 2@0 2@2 fp E P
> 
> Propagate FPE preemption-class mapping errors in
> tc_setup_dwmac510_mqprio() and tc_taprio_configure().
> 
> [...]

Here is the summary with links:
  - [net,v3,1/2] net: stmmac: propagate FPE preemption-class mapping errors
    https://git.kernel.org/netdev/net/c/90e4b849dfa6
  - [net,v3,2/2] net: stmmac: preserve real_num_tx_queues on mqprio setup failure
    https://git.kernel.org/netdev/net/c/02fffd1939f6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html