netdev - Re: [net-next,v22,4/7] net: mtip: Add net_device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260203014247.824561-2-kuba@kernel.org>
Date: Mon,  2 Feb 2026 17:42:48 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: lukasz.majewski@...lbox.org
Cc: Jakub Kicinski <kuba@...nel.org>,
	andrew@...n.ch,
	shawnguo@...nel.org,
	krzk+dt@...nel.org,
	linux-kernel@...r.kernel.org,
	edumazet@...gle.com,
	netdev@...r.kernel.org,
	pabeni@...hat.com,
	andrew+netdev@...n.ch,
	davem@...emloft.net,
	conor+dt@...nel.org,
	horms@...nel.org,
	richardcochran@...il.com,
	robh@...nel.org,
	imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org,
	devicetree@...r.kernel.org,
	wahrenst@....net,
	s.hauer@...gutronix.de,
	kernel@...gutronix.de,
	festevam@...il.com
Subject: Re: [net-next,v22,4/7] net: mtip: Add net_device_ops functions to the L2 switch driver

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.

> 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/

> +	DO_ONCE(mtip_print_hw_state, dev);
> +
> +	schedule_work(&priv->tx_timeout_work);
> +}