netdev - Re: [net-next,v20,6/6] eea: introduce callback for ndo_get

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20260119224225.1363113-1-kuba@kernel.org>
Date: Mon, 19 Jan 2026 14:42:25 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: xuanzhuo@...ux.alibaba.com
Cc: Jakub Kicinski <kuba@...nel.org>,
	davem@...emloft.net,
	netdev@...r.kernel.org,
	andrew@...n.ch,
	andrew+netdev@...n.ch,
	dust.li@...ux.alibaba.com,
	pabeni@...hat.com,
	vadim.fedorenko@...ux.dev,
	dong100@...se.com,
	guwen@...ux.alibaba.com,
	lorenzo@...nel.org,
	edumazet@...gle.com,
	lulie@...ux.alibaba.com,
	lukas.bulwahn@...hat.com
Subject: Re: [net-next,v20,6/6] eea: introduce callback for ndo_get_stats64

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
---
eea: introduce callback for ndo_get_stats64

This commit adds ndo_get_stats64 support for the Alibaba Elastic Ethernet
Adapter driver. The eea_stats() function iterates over RX and TX queues
to collect packet and byte statistics using u64_stats_sync primitives.

> diff --git a/drivers/net/ethernet/alibaba/eea/eea_net.c b/drivers/net/ethernet/alibaba/eea/eea_net.c
> index 2eba43a82ebc..440f5f11e055 100644
> --- a/drivers/net/ethernet/alibaba/eea/eea_net.c
> +++ b/drivers/net/ethernet/alibaba/eea/eea_net.c
> @@ -269,6 +269,52 @@ static int eea_netdev_open(struct net_device *netdev)
> 	return err;
>  }
>
> +static void eea_stats(struct net_device *netdev, struct rtnl_link_stats64 *tot)
> +{
> +	struct eea_net *enet = netdev_priv(netdev);
> +	u64 packets, bytes;
> +	u32 start;
> +	int i;
> +
> +	/* This function is protected by RCU. Here uses enet->tx and enet->rx
> +	 * to check whether the TX and RX structures are safe to access. In
> +	 * eea_free_rxtx_q_mem, before freeing the TX and RX resources, enet->rx
> +	 * and enet->tx are set to NULL, and synchronize_net is called.
> +	 */
> +
> +	if (enet->rx) {
> +		for (i = 0; i < enet->cfg.rx_ring_num; i++) {
> +			struct eea_net_rx *rx = enet->rx[i];

The comment states this function is protected by RCU, but enet->rx is read
without READ_ONCE() or rcu_dereference(). Since eea_free_rxtx_q_mem() can
set enet->rx to NULL concurrently (before calling synchronize_net()), should
the pointer be loaded with READ_ONCE() to prevent the compiler from
re-reading it between the NULL check and the array access?

Similarly, enet->tx below has the same pattern. The RCU documentation
recommends using rcu_dereference() for pointers that are accessed under
RCU protection and may be concurrently modified.

> +
> +			do {
> +				start = u64_stats_fetch_begin(&rx->stats.syncp);
> +				packets = u64_stats_read(&rx->stats.packets);
> +				bytes = u64_stats_read(&rx->stats.bytes);
> +			} while (u64_stats_fetch_retry(&rx->stats.syncp,
> +						       start));
> +
> +			tot->rx_packets += packets;
> +			tot->rx_bytes   += bytes;
> +		}
> +	}
> +
> +	if (enet->tx) {
> +		for (i = 0; i < enet->cfg.tx_ring_num; i++) {
> +			struct eea_net_tx *tx = &enet->tx[i];

[ ... ]