Re: [PATCH net-next v2] r8169: expose software counters through netdev qstats

[Heiner Kallweit <hkallweit1@gmail.com>]

On 27.04.2026 22:59, Gustavo Arantes wrote:
> r8169 maintains synchronized per-CPU software counters for packet and
> byte accounting and exposes them through ndo_get_stats64(), but
> userspace using the structured netdev qstats API cannot retrieve them from
> this driver.
> 
> Expose the same counters through netdev_stat_ops. r8169 has a single Rx
> and Tx queue, so report the accumulated counters on queue 0. Use zero
> base stats so device-scope qstats are derived from the queue counters
> and match the packet and byte values reported through RTNL.

What do you mean with RTNL here?

> 
> This does not add new accounting and does not touch the data path. It
> only makes the existing counters available through the common qstats
> interface, which lets generic userspace tooling query r8169 the same way
> it queries other drivers with qstats support.
> 
Counters are exposed to userspace already. So what's the benefit of
duplicating this functionality? To me the patch looks like AI slop.

> Signed-off-by: Gustavo Arantes <dev.gustavoa@gmail.com>
> ---
> v2:
>   - Submit again now that net-next has reopened.
>   - Expand commit message to explain the qstats userspace benefit.
> 
> Tested on a Realtek RTL8111/8168/8211/8411 PCIe Gigabit Ethernet
> controller using the r8169 driver.
> 
> v1: https://lore.kernel.org/netdev/20260418021232.5425-1-dev.gustavoa@gmail.com/
> 
>  drivers/net/ethernet/realtek/r8169_main.c | 70 +++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 791277e750ba..9d833b446383 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5175,6 +5175,75 @@ rtl8169_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
>  	pm_runtime_put_noidle(&pdev->dev);
>  }
> 
> +static void rtl8169_fetch_sw_stats(struct net_device *dev,
> +				   struct netdev_queue_stats_rx *rx,
> +				   struct netdev_queue_stats_tx *tx)
> +{
> +	const struct pcpu_sw_netstats *stats;
> +	unsigned int start;
> +	int cpu;
> +
> +	for_each_possible_cpu(cpu) {
> +		u64 rx_packets, rx_bytes, tx_packets, tx_bytes;
> +
> +		stats = per_cpu_ptr(dev->tstats, cpu);
> +		do {
> +			start = u64_stats_fetch_begin(&stats->syncp);
> +			rx_packets = u64_stats_read(&stats->rx_packets);
> +			rx_bytes = u64_stats_read(&stats->rx_bytes);
> +			tx_packets = u64_stats_read(&stats->tx_packets);
> +			tx_bytes = u64_stats_read(&stats->tx_bytes);
> +		} while (u64_stats_fetch_retry(&stats->syncp, start));
> +
> +		rx->packets += rx_packets;
> +		rx->bytes += rx_bytes;
> +		tx->packets += tx_packets;
> +		tx->bytes += tx_bytes;
> +	}
> +}

This is generic, driver-independent code. If exposing dev->tstats this
way makes sense, then this should go to net core.

> +
> +static void rtl8169_get_queue_stats_rx(struct net_device *dev, int idx,
> +				       struct netdev_queue_stats_rx *rx)
> +{
> +	struct netdev_queue_stats_tx tx = {};
> +
> +	if (idx)
> +		return;
> +
> +	rx->packets = 0;
> +	rx->bytes = 0;
> +	rtl8169_fetch_sw_stats(dev, rx, &tx);
> +}
> +
> +static void rtl8169_get_queue_stats_tx(struct net_device *dev, int idx,
> +				       struct netdev_queue_stats_tx *tx)
> +{
> +	struct netdev_queue_stats_rx rx = {};
> +
> +	if (idx)
> +		return;
> +
> +	tx->packets = 0;
> +	tx->bytes = 0;
> +	rtl8169_fetch_sw_stats(dev, &rx, tx);
> +}
> +
> +static void rtl8169_get_base_stats(struct net_device *dev,
> +				   struct netdev_queue_stats_rx *rx,
> +				   struct netdev_queue_stats_tx *tx)
> +{
> +	rx->packets = 0;
> +	rx->bytes = 0;
> +	tx->packets = 0;
> +	tx->bytes = 0;
> +}
> +
> +static const struct netdev_stat_ops rtl8169_stat_ops = {
> +	.get_queue_stats_rx	= rtl8169_get_queue_stats_rx,
> +	.get_queue_stats_tx	= rtl8169_get_queue_stats_tx,
> +	.get_base_stats		= rtl8169_get_base_stats,
> +};
> +
>  static void rtl8169_net_suspend(struct rtl8169_private *tp)
>  {
>  	netif_device_detach(tp->dev);
> @@ -5615,6 +5684,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> 
>  	SET_NETDEV_DEV(dev, &pdev->dev);
>  	dev->netdev_ops = &rtl_netdev_ops;
> +	dev->stat_ops = &rtl8169_stat_ops;
>  	tp = netdev_priv(dev);
>  	tp->dev = dev;
>  	tp->pci_dev = pdev;
> --
> 2.54.0
>