Re: [net-next v24 3/7] net: mtip: Add buffers management functions to the L2 switch driver
flat view
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-06-05 02:18:55
Also in:
imx, linux-arm-kernel, lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. --- net: mtip: Add buffers management functions to the L2 switch driver This patch fills in mtip_alloc_buffers() and mtip_free_buffers() for the MTIP L2 switch driver, replacing the previously empty stubs with real page_pool setup/teardown and tx_bounce buffer allocation/free.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c index 1d1005a42306..5b5229ee411d 100644 --- a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c +++ b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c@@ -892,11 +892,101 @@ static void mtip_get_drvinfo(struct net_device *dev, static void mtip_free_buffers(struct net_device *dev) { + struct mtip_ndev_priv *priv = netdev_priv(dev); + struct switch_enet_private *fep = priv->fep; + int i; + + for (i = 0; i < RX_RING_SIZE; i++) { + if (!fep->page[i]) + continue; + + page_pool_put_full_page(fep->page_pool, + fep->page[i], false); + fep->page[i] = NULL; + } + + page_pool_destroy(fep->page_pool); + fep->page_pool = NULL; + + for (i = 0; i < TX_RING_SIZE; i++) { + kfree(fep->tx_bounce[i]); + fep->tx_bounce[i] = NULL; + } +}
[High]
Now that mtip_free_buffers() is no longer an empty stub, can the
mtip_open() error path tear down buffers that another active port is
still using?
Looking at mtip_open():
if (fep->usage_count == 0) {
...
ret = mtip_alloc_buffers(dev);
if (ret)
goto mtip_alloc_buffers_err;
}
...
ret = mtip_mii_probe(dev);
if (ret)
goto mtip_mii_probe_err;
...
mtip_mii_probe_err:
mtip_free_buffers(dev);
mtip_alloc_buffers_err:
if (fep->usage_count == 0) {
netif_napi_del(&fep->napi);
clk_disable(fep->clk_ipg);
}
return ret;
mtip_alloc_buffers() only runs when fep->usage_count == 0, so the
page_pool, fep->page[] and fep->tx_bounce[] are shared across switch
ports and only allocated on first open. mtip_close() reflects this and
only calls mtip_free_buffers() when fep->usage_count == 1.
When a second port is opened (fep->usage_count > 0) and mtip_mii_probe()
fails, the mtip_mii_probe_err label still calls mtip_free_buffers()
unconditionally, which destroys the page_pool and frees the tx_bounce
buffers that the first, still-active port is using.
Should the mtip_mii_probe_err teardown also be guarded by
fep->usage_count == 0 (or the labels reordered) so buffers are only
freed for the last user, similar to mtip_close()?
After this change the previously harmless call becomes a real teardown,
so the surviving port's NAPI path (mtip_switch_rx() and the later
mtip_update_cbd() refill) would dereference a freed fep->page_pool and
NULL fep->page[] / fep->tx_bounce[] entries whenever the second port's
PHY probe fails while the first port is up.
+static int mtip_create_page_pool(struct switch_enet_private *fep, int size)
+{[ ... ]
quoted hunk ↗ jump to hunk
+} static int mtip_alloc_buffers(struct net_device *dev) {
[ ... ]
quoted hunk ↗ jump to hunk
return 0; + + err: + mtip_free_buffers(dev); + return -ENOMEM; }