Re: [PATCH net-next v3 10/13] net: lan966x: add PCIe FDMA MTU change support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NAPI enablement race condition in reload paths can permanently stall the device.
--

commit 20d04dd9ba5afad59c5a90474d47aff04eccef0c
Author: Daniel Machon <daniel.machon@microchip.com>

net: lan966x: add PCIe FDMA MTU change support

This commit adds MTU change support for the PCIe FDMA path. It allows
continuous ATU-mapped RX and TX buffers to be reallocated with a new size
upon MTU changes, falling back to reusing existing buffers upon allocation
failure.

> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma_pci.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma_pci.c
> index 2c54880460772..491ddc337760f 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma_pci.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma_pci.c
[ ... ]
> @@ -354,9 +359,155 @@ static int lan966x_fdma_pci_init(struct lan966x *lan966x)
[ ... ]
> +static int lan966x_fdma_pci_reload(struct lan966x *lan966x, int new_mtu)
> +{
[ ... ]
> +	/* Keep this order: rx_start, wakeup_netdev, napi_enable. */
> +	lan966x_fdma_rx_start(&lan966x->rx);
> +	lan966x_fdma_pci_wakeup_netdev(lan966x);
> +	napi_enable(&lan966x->napi);

Is this ordering vulnerable to a race condition that can permanently stall
the device?

If an interrupt fires immediately after lan966x_fdma_rx_start() but before
napi_enable(), could the following sequence occur:

lan966x_fdma_rx_start()
  -> enables hardware interrupts via FDMA_INTR_DB_ENA

Hardware Interrupt
lan966x_fdma_irq_handler()
  -> masks interrupt via FDMA_INTR_DB_ENA
  -> napi_schedule()
     -> drops the event because napi_enable() hasn't run yet

napi_enable()
  -> clears NAPI state but schedules no poll

Since the hardware interrupt remains masked by the ISR and NAPI will never run
to unmask it, does this leave the NAPI polling permanently stalled?

Should napi_enable() be called before lan966x_fdma_rx_start() unmasks the
hardware interrupts?

> +
> +	return err;
> +restore:
> +
> +	/* No new buffers are allocated at this point. Use the old buffers,
> +	 * but reset them before starting the FDMA again.
> +	 */
> +
> +	memcpy(&lan966x->tx.fdma, &tx_fdma_old, sizeof(struct fdma));
> +	memcpy(&lan966x->rx.fdma, &rx_fdma_old, sizeof(struct fdma));
> +
> +	lan966x->rx.max_mtu = old_mtu;
> +
> +	lan966x_fdma_pci_reset_mem(lan966x);
> +
> +	/* Keep this order: rx_start, wakeup_netdev, napi_enable. */
> +	lan966x_fdma_rx_start(&lan966x->rx);
> +	lan966x_fdma_pci_wakeup_netdev(lan966x);
> +	napi_enable(&lan966x->napi);

Does this restore path have the same race condition between unmasking
hardware interrupts and enabling NAPI?

> +
> +	return err;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504-lan966x-pci-fdma-v3-0-a56f5740d870@microchip.com?part=10