Re: [net-next,v22,4/7] net: mtip: Add net_device_ops functions to the L2 switch driver
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-02-03 01:44:06
Also in:
imx, linux-arm-kernel, linux-devicetree, 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. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- net: mtip: Add net_device_ops functions to the L2 switch driver This patch adds net_device_ops callbacks for the MTIP L2 switch driver, including transmit, multicast configuration, MAC address handling, and timeout recovery functions.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c --- a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c +++ b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c
[ ... ]
+static netdev_tx_t mtip_start_xmit_port(struct sk_buff *skb,
+ struct net_device *dev, int port)
+{
+ struct mtip_ndev_priv *priv = netdev_priv(dev);
+ struct switch_enet_private *fep = priv->fep;
+ unsigned short status;
+ unsigned int index;
+ struct cbd_t *bdp;
+ void *bufaddr;
+
+ spin_lock_bh(&fep->hw_lock);[ ... ]
+ /* Save skb pointer. */ + fep->tx_skbuff[index] = skb;
mtip_start_xmit_port() stores sk_buffs in fep->tx_skbuff[index] and creates
DMA mappings, but mtip_switch_tx() which should complete TX operations is
empty:
static void mtip_switch_tx(struct switch_enet_private *fep)
{
}
The NAPI poll handler mtip_rx_napi() calls mtip_switch_tx() to reclaim
transmitted buffers. Since it does nothing, won't every transmitted packet
leak the skb and DMA mapping? Also, dirty_tx is never advanced, so the TX
ring will fill (cur_tx == dirty_tx) and stop accepting packets.
jakub: maybe add a TODO comment transiently here, since later patch implements
the cleanup?
+static void mtip_timeout(struct net_device *dev, unsigned int txqueue)
+{
+ struct mtip_ndev_priv *priv = netdev_priv(dev);
+
+ dev->stats.tx_errors++;Jakub Kicinski noted in v16 review that tx_errors are already counted by the network stack, and the statistic is exposed per-queue in sysfs. Does incrementing tx_errors here double-count timeout events? https://lore.kernel.org/all/20250725151618.0bc84bdb@kernel.org/ (local)
+ DO_ONCE(mtip_print_hw_state, dev); + + schedule_work(&priv->tx_timeout_work); +}