Re: [PATCH v2] net: lan966x: restore RX state on reload failure

[David CARLIER]

ACK on the code part at least, maintainers might disagree with this:
changelog could also note the subject rename ("restore XDP RXQ page
pool" -> "restore RX state"), so the scope change from v1 is obvious
but that s a very minor nit IMO.

Reviewed-by: David Carlier <devnexen@xxxxxxxxx>

On Sun, 7 Jun 2026 at 15:58, Guangshuo Li <lgs201920130244@xxxxxxxxx> wrote:
>
> lan966x_fdma_reload() backs up rx->page_pool and rx->fdma before
> reallocating the RX resources for the new MTU. If the allocation fails,
> the restore path puts these fields back before restarting RX.
>
> However, the reload path also updates rx->page_order and rx->max_mtu
> before calling lan966x_fdma_rx_alloc(). These fields are not restored on
> failure, so RX can be restarted with the old pages, old FDMA state and
> old page pool, but with the page geometry from the failed new MTU.
>
> This can make the XDP path advertise a frame size derived from the new
> page_order while the actual RX pages still come from the old allocation.
> For example, after a failed reload to a jumbo MTU, xdp_init_buff() may be
> called with a frame size larger than the restored RX pages.
>
> lan966x_fdma_rx_alloc_page_pool() also registers the newly allocated page
> pool with each port's XDP RXQ before fdma_alloc_coherent() is called. If
> fdma_alloc_coherent() fails, the new page pool is destroyed, but the
> rollback path does not restore the per-port XDP RXQ mem model
> registration either.
>
> Save and restore rx->page_order and rx->max_mtu, and restore the old page
> pool registration for each port's XDP RXQ before RX is started again.
> This keeps the restored RX state consistent after a failed reload.
>
> Fixes: 59c3d55a946c ("net: lan966x: fix use-after-free and leak in lan966x_fdma_reload()")
> Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> ---
> v2:
>   - Save and restore rx->page_order and rx->max_mtu in the reload rollback
>     path.
>   - Keep the XDP RXQ page-pool registration restore from v1.
>
>  .../ethernet/microchip/lan966x/lan966x_fdma.c | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> index f8ce735a7fc0..8272ad085150 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> @@ -815,6 +815,7 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
>         struct page *(*old_pages)[FDMA_RX_DCB_MAX_DBS];
>         struct page_pool *page_pool;
>         struct fdma fdma_rx_old;
> +       int page_order, max_mtu;
>         int err, i, j;
>
>         old_pages = kmemdup(lan966x->rx.page, sizeof(lan966x->rx.page),
> @@ -825,6 +826,8 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
>         /* Store these for later to free them */
>         memcpy(&fdma_rx_old, &lan966x->rx.fdma, sizeof(struct fdma));
>         page_pool = lan966x->rx.page_pool;
> +       page_order = lan966x->rx.page_order;
> +       max_mtu = lan966x->rx.max_mtu;
>
>         napi_synchronize(&lan966x->napi);
>         napi_disable(&lan966x->napi);
> @@ -854,7 +857,24 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
>         return 0;
>  restore:
>         lan966x->rx.page_pool = page_pool;
> +       lan966x->rx.page_order = page_order;
> +       lan966x->rx.max_mtu = max_mtu;
>         memcpy(&lan966x->rx.fdma, &fdma_rx_old, sizeof(struct fdma));
> +       /*
> +        * lan966x_fdma_rx_alloc_page_pool() registered the new pool with
> +        * each port's XDP RXQ before the allocation failed. The new pool is
> +        * destroyed by lan966x_fdma_rx_alloc(), so restore the old pool's
> +        * registration before restarting RX.
> +        */
> +       for (i = 0; i < lan966x->num_phys_ports; i++) {
> +               if (!lan966x->ports[i])
> +                       continue;
> +
> +               xdp_rxq_info_unreg_mem_model(&lan966x->ports[i]->xdp_rxq);
> +               xdp_rxq_info_reg_mem_model(&lan966x->ports[i]->xdp_rxq,
> +                                          MEM_TYPE_PAGE_POOL, page_pool);
> +       }
> +
>         lan966x_fdma_rx_start(&lan966x->rx);
>
>         lan966x_fdma_wakeup_netdev(lan966x);
> --
> 2.43.0
>