Re: [PATCH net v4 15/15] net: stmmac: retain DMA resources across MTU changes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: stmmac: double disable of NAPI causing deadlock in stmmac_reconfigure_mtu()
--

commit ed7896fe947ec45eb3dddd37b9ca9ed5f20ea4e9
Author: James Hilliard <james.hilliard1@gmail.com>

net: stmmac: retain DMA resources across MTU changes

This commit modifies the MTU change procedure to prepare the replacement
configuration before quiescing the old datapath. It retains the rings and IRQ
registrations until setup succeeds so rollback needs no new allocations, and
leaves the interface detached in a distinct HALTED state if rollback fails.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

[ ... ]

> @@ -6601,6 +6651,109 @@ static void stmmac_set_rx_mode(struct net_device *dev)
>  	stmmac_set_filter(priv, priv->hw, dev);
>  }
>  
> +static int stmmac_reconfigure_mtu(struct net_device *dev, int mtu)
> +{
> +	struct stmmac_priv *priv = netdev_priv(dev);
> +	struct stmmac_dma_conf *old_conf = priv->dma_conf;
> +	struct stmmac_dma_conf *new_conf;
> +	int old_mtu = dev->mtu;
> +	int ret, restore_ret;
> +	u32 irq_mask[STMMAC_CH_MAX];
> +	u32 chan;
> +
> +	new_conf = stmmac_setup_dma_desc(priv, mtu);
> +	if (IS_ERR(new_conf))
> +		return PTR_ERR(new_conf);
> +
> +	mutex_lock(&priv->ptp_mutex);
> +	stmmac_block_ptp(priv, true);
> +	netif_device_detach(dev);
> +	phylink_stop(priv->phylink);
> +	stmmac_quiesce(priv);

[Severity: High]
Does this unconditional call to stmmac_quiesce() lead to a double-disable of
NAPI if the interface is already in the HALTED state?

If stmmac_reconfigure_mtu() fails to rollback after a hardware setup error,
the interface state is set to STMMAC_DATAPATH_HALTED with NAPI left disabled,
but the device remains administratively up (netif_running(dev) remains true).

If an MTU change is attempted again, stmmac_change_mtu() will call
stmmac_reconfigure_mtu() a second time. This unconditional stmmac_quiesce()
call will invoke stmmac_disable_all_queues(), resulting in a second
napi_disable() on the already disabled NAPI instances. This can cause the
kernel to deadlock in napi_disable_locked() while waiting for
NAPIF_STATE_SCHED to clear.

> +	if (stmmac_fpe_supported(priv))
> +		ethtool_mmsv_stop(&priv->fpe_cfg.mmsv);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926-submit-stmmac-reset-fixes-v1-v4-0-ec1c0250b3c9@gmail.com?part=15