[PATCH net v2 2/2] net: stmmac: keep datapath state coherent after reinitialization failure
James Hilliard
james.hilliard1 at gmail.com
Tue Sep 22 16:27:41 PDT 2026
An MTU change releases the running datapath before reopening it. If the
reopen fails, its replacement DMA resources are freed, but the netdev is
still administratively up. A later close repeats NAPI shutdown, IRQ
release and DMA cleanup. Hardware resume failure has a different partial
state: suspend disabled NAPI but retained the IRQs and DMA resources, so
ordinary close can hang in a second napi_disable().
Track the datapath independently of the administrative state, with three
states describing running queues, suspended queues with resources still
owned, and a released datapath. Separate quiescing the queues from
releasing their resources, so close can perform only the remaining work.
Serialize these transitions with RTNL, including suspend and resume.
On a failed MTU reopen, leave the PHY attachment and runtime-PM reference
owned until ndo_stop(), but detach the netdev so the released datapath
cannot be used. On failed hardware resume, stop any partially initialized
DMA and leave the retained datapath suspended and detached. Do not call
netif_close() or change the administrative state. A later successful
resume can retry the retained datapath; alternatively an ordinary down
releases it, reattaches the now-down netdev, and permits a fresh open.
Stop phylink directly from its suspended state during close. Do not
restart it on failed hardware just to balance its shutdown. Preserve
IRQ-before-final-DMA-stop ordering, and stop DMA on failed open before
the caller frees the replacement rings, including IRQ-request failure
after hardware setup has started DMA.
Account for callers which are not excluded by netif_device_detach():
guard descriptor readback with RTNL and resource ownership, reject TC
queue reconfiguration while detached, and prevent deferred reset work
from reopening the failed interface. Check availability under the TX
queue lock before XDP transmission. XDP configuration must use the actual
datapath state rather than IFF_UP. In particular, AF_XDP pool removal
cannot be rejected: release any suspended rings before a socket's pool
is unmapped and freed, leaving recovery to a subsequent down/up cycle.
Fixes: 3470079687448 ("net: ethernet: stmicro: stmmac: permit MTU change with interface up")
Fixes: 6896c2449a18 ("net: stmmac: Check stmmac_hw_setup() in stmmac_resume()")
Signed-off-by: James Hilliard <james.hilliard1 at gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 11 ++
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 116 ++++++++++++++++------
drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c | 8 +-
3 files changed, 103 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 7582fca63741..5bb92339cbde 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -258,6 +258,15 @@ struct stmmac_msi {
char int_name_tx_irq[MTL_MAX_TX_QUEUES][IFNAMSIZ + 18];
};
+enum stmmac_datapath_state {
+ /* No IRQs or DMA allocations owned by a successful open. */
+ STMMAC_DATAPATH_DOWN,
+ /* Resources allocated, NAPI enabled. */
+ STMMAC_DATAPATH_RUNNING,
+ /* Resources retained, NAPI and DMA stopped; also after failed resume. */
+ STMMAC_DATAPATH_SUSPENDED,
+};
+
struct stmmac_priv {
/* Frequently used values are kept adjacent for cache effect */
u32 tx_coal_frames[MTL_MAX_TX_QUEUES];
@@ -281,6 +290,8 @@ struct stmmac_priv {
struct mutex lock;
struct stmmac_dma_conf dma_conf;
+ /* IRQ/DMA ownership and NAPI state, serialized by RTNL. */
+ enum stmmac_datapath_state datapath;
/* Generic channel for NAPI */
struct stmmac_channel channel[STMMAC_CH_MAX];
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1fb5f804ea23..7b423c87314c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4179,6 +4179,7 @@ static int __stmmac_open(struct net_device *dev,
stmmac_enable_all_queues(priv);
netif_tx_start_all_queues(priv->dev);
stmmac_enable_all_dma_irq(priv);
+ priv->datapath = STMMAC_DATAPATH_RUNNING;
return 0;
@@ -4190,6 +4191,8 @@ static int __stmmac_open(struct net_device *dev,
stmmac_release_ptp(priv);
init_error:
+ stmmac_stop_all_dma(priv);
+ stmmac_mac_set(priv, priv->ioaddr, false);
return ret;
}
@@ -4244,25 +4247,38 @@ static int stmmac_open(struct net_device *dev)
return ret;
}
-static void __stmmac_release(struct net_device *dev)
+/* Quiesce NAPI and transmit queues without releasing their resources. */
+static void stmmac_quiesce(struct stmmac_priv *priv)
{
- struct stmmac_priv *priv = netdev_priv(dev);
u8 chan;
- /* Stop and disconnect the PHY */
- phylink_stop(priv->phylink);
-
stmmac_disable_all_queues(priv);
for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
- netif_tx_disable(dev);
+ netif_tx_disable(priv->dev);
+}
+
+static void __stmmac_release(struct net_device *dev)
+{
+ struct stmmac_priv *priv = netdev_priv(dev);
+
+ /* A failed MTU reopen has already released the data path. */
+ if (priv->datapath == STMMAC_DATAPATH_DOWN)
+ return;
+
+ phylink_stop(priv->phylink);
+
+ /* Suspend retains the resources, but has already stopped activity. */
+ if (priv->datapath == STMMAC_DATAPATH_RUNNING)
+ stmmac_quiesce(priv);
+ priv->datapath = STMMAC_DATAPATH_DOWN;
/* Free the IRQ lines */
stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
- /* Stop TX/RX DMA and clear the descriptors */
+ /* Stop TX/RX DMA after draining IRQ handlers which can restart it. */
stmmac_stop_all_dma(priv);
/* Release and free the Rx/Tx resources */
@@ -4296,6 +4312,8 @@ static int stmmac_release(struct net_device *dev)
stmmac_legacy_serdes_power_down(priv);
phylink_disconnect_phy(priv->phylink);
pm_runtime_put(priv->device);
+ /* Allow a fresh open after a failed MTU reopen or resume. */
+ netif_device_attach(dev);
return 0;
}
@@ -6174,6 +6192,11 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
if (ret) {
free_dma_desc_resources(priv, dma_conf);
kfree(dma_conf);
+ /*
+ * Keep the administrative state and PHY/PM ownership until
+ * ndo_stop(), but prevent use of the released data path.
+ */
+ netif_device_detach(dev);
netdev_err(priv->dev, "failed reopening the interface after MTU change\n");
return ret;
}
@@ -6422,6 +6445,8 @@ static int stmmac_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
if (!tc_cls_can_offload_and_chain0(priv->dev, type_data))
return ret;
+ if (!netif_device_present(priv->dev))
+ return -ENETDOWN;
__stmmac_disable_all_queues(priv);
@@ -6543,8 +6568,9 @@ static int stmmac_rings_status_show(struct seq_file *seq, void *v)
u8 tx_count = priv->plat->tx_queues_to_use;
u8 queue;
- if ((dev->flags & IFF_UP) == 0)
- return 0;
+ rtnl_lock();
+ if (priv->datapath == STMMAC_DATAPATH_DOWN)
+ goto out_unlock;
for (queue = 0; queue < rx_count; queue++) {
struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
@@ -6578,6 +6604,8 @@ static int stmmac_rings_status_show(struct seq_file *seq, void *v)
}
}
+out_unlock:
+ rtnl_unlock();
return 0;
}
DEFINE_SHOW_ATTRIBUTE(stmmac_rings_status);
@@ -6959,6 +6987,18 @@ static int stmmac_bpf(struct net_device *dev, struct netdev_bpf *bpf)
{
struct stmmac_priv *priv = netdev_priv(dev);
+ if (bpf->command != XDP_SETUP_PROG &&
+ bpf->command != XDP_SETUP_XSK_POOL)
+ return -EOPNOTSUPP;
+
+ /*
+ * Pool removal must succeed even after a failed resume. Release the
+ * suspended rings before their pool or XDP buffer layout can change.
+ * Leave the interface detached until it is closed and reopened.
+ */
+ if (priv->datapath == STMMAC_DATAPATH_SUSPENDED)
+ __stmmac_release(dev);
+
switch (bpf->command) {
case XDP_SETUP_PROG:
return stmmac_xdp_set_prog(priv, bpf->prog, bpf->extack);
@@ -6989,6 +7029,10 @@ static int stmmac_xdp_xmit(struct net_device *dev, int num_frames,
nq = netdev_get_tx_queue(priv->dev, queue);
__netif_tx_lock(nq, cpu);
+ if (unlikely(!netif_device_present(dev) || netif_tx_queue_stopped(nq))) {
+ __netif_tx_unlock(nq);
+ return -ENETDOWN;
+ }
/* Avoids TX time-out as we are sharing with slow path */
txq_trans_cond_update(nq);
@@ -7361,6 +7405,9 @@ static void stmmac_reset_subtask(struct stmmac_priv *priv)
netdev_err(priv->dev, "Reset adapter.\n");
rtnl_lock();
+ if (!netif_device_present(priv->dev))
+ goto out_unlock;
+
netif_trans_update(priv->dev);
while (test_and_set_bit(STMMAC_RESETING, &priv->state))
usleep_range(1000, 2000);
@@ -7370,6 +7417,7 @@ static void stmmac_reset_subtask(struct stmmac_priv *priv)
dev_open(priv->dev, NULL);
clear_bit(STMMAC_DOWN, &priv->state);
clear_bit(STMMAC_RESETING, &priv->state);
+out_unlock:
rtnl_unlock();
}
@@ -8198,26 +8246,24 @@ int stmmac_suspend(struct device *dev)
{
struct net_device *ndev = dev_get_drvdata(dev);
struct stmmac_priv *priv = netdev_priv(ndev);
- u8 chan;
- if (!ndev || !netif_running(ndev))
+ rtnl_lock();
+ if (priv->datapath != STMMAC_DATAPATH_RUNNING) {
+ rtnl_unlock();
goto suspend_bsp;
+ }
mutex_lock(&priv->lock);
netif_device_detach(ndev);
- stmmac_disable_all_queues(priv);
-
- for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
- hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
+ stmmac_quiesce(priv);
if (priv->eee_sw_timer_en) {
priv->tx_path_in_lpi_mode = false;
timer_delete_sync(&priv->eee_ctrl_timer);
}
- /* Stop TX/RX DMA */
stmmac_stop_all_dma(priv);
stmmac_legacy_serdes_power_down(priv);
@@ -8233,12 +8279,12 @@ int stmmac_suspend(struct device *dev)
mutex_unlock(&priv->lock);
- rtnl_lock();
phylink_suspend(priv->phylink, !!priv->wolopts);
- rtnl_unlock();
+ priv->datapath = STMMAC_DATAPATH_SUSPENDED;
if (stmmac_fpe_supported(priv))
ethtool_mmsv_stop(&priv->fpe_cfg.mmsv);
+ rtnl_unlock();
suspend_bsp:
if (priv->plat->suspend)
@@ -8302,8 +8348,11 @@ int stmmac_resume(struct device *dev)
return ret;
}
- if (!netif_running(ndev))
- return 0;
+ rtnl_lock();
+ if (priv->datapath != STMMAC_DATAPATH_SUSPENDED) {
+ ret = 0;
+ goto out_unlock;
+ }
/* Power Down bit, into the PM register, is cleared
* automatically as soon as a magic packet or a Wake-up frame
@@ -8326,11 +8375,9 @@ int stmmac_resume(struct device *dev)
if (!(priv->plat->flags & STMMAC_FLAG_SERDES_UP_AFTER_PHY_LINKUP)) {
ret = stmmac_legacy_serdes_power_up(priv);
if (ret < 0)
- return ret;
+ goto out_unlock;
}
- rtnl_lock();
-
/* Prepare the PHY to resume, ensuring that its clocks which are
* necessary for the MAC DMA reset to complete are running
*/
@@ -8346,10 +8393,7 @@ int stmmac_resume(struct device *dev)
ret = stmmac_hw_setup(ndev);
if (ret < 0) {
netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
- stmmac_legacy_serdes_power_down(priv);
- mutex_unlock(&priv->lock);
- rtnl_unlock();
- return ret;
+ goto error_stop_dma;
}
stmmac_init_timestamping(priv);
@@ -8371,11 +8415,25 @@ int stmmac_resume(struct device *dev)
* workqueue thread, which will race with initialisation.
*/
phylink_resume(priv->phylink);
- rtnl_unlock();
-
+ priv->datapath = STMMAC_DATAPATH_RUNNING;
netif_device_attach(ndev);
+ rtnl_unlock();
return 0;
+
+error_stop_dma:
+ stmmac_stop_all_dma(priv);
+ stmmac_mac_set(priv, priv->ioaddr, false);
+ stmmac_legacy_serdes_power_down(priv);
+ mutex_unlock(&priv->lock);
+ /*
+ * Keep the suspended data path detached. A later resume may retry, or
+ * ndo_stop() can release its resources without disabling NAPI again.
+ */
+out_unlock:
+ rtnl_unlock();
+
+ return ret;
}
EXPORT_SYMBOL_GPL(stmmac_resume);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
index d7e4db7224b0..909219775507 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
@@ -31,7 +31,8 @@ static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
return err;
}
- need_update = netif_running(priv->dev) && stmmac_xdp_is_enabled(priv);
+ need_update = priv->datapath == STMMAC_DATAPATH_RUNNING &&
+ stmmac_xdp_is_enabled(priv);
if (need_update) {
napi_disable(&ch->rx_napi);
@@ -69,7 +70,8 @@ static int stmmac_xdp_disable_pool(struct stmmac_priv *priv, u16 queue)
if (!pool)
return -EINVAL;
- need_update = netif_running(priv->dev) && stmmac_xdp_is_enabled(priv);
+ need_update = priv->datapath == STMMAC_DATAPATH_RUNNING &&
+ stmmac_xdp_is_enabled(priv);
if (need_update) {
napi_disable(&ch->rxtx_napi);
@@ -107,7 +109,7 @@ int stmmac_xdp_set_prog(struct stmmac_priv *priv, struct bpf_prog *prog,
bool need_update;
bool if_running;
- if_running = netif_running(dev);
+ if_running = priv->datapath == STMMAC_DATAPATH_RUNNING;
if (prog && dev->mtu > ETH_DATA_LEN) {
/* For now, the driver doesn't support XDP functionality with
--
2.53.0
More information about the linux-arm-kernel
mailing list