Re: [PATCH net v3 2/2] net: stmmac: preserve real_num_tx_queues on mqprio setup failure
flat view
From: Lorenzo Bianconi <hidden>
Date: 2026-09-16 09:08:36
Also in:
linux-arm-kernel
quoted hunk ↗ jump to hunk
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 <redacted> --- 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 [...]
quoted hunk ↗ jump to hunk
- 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
Attachments
- signature.asc [application/pgp-signature] 228 bytes