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

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

[Gustavo Arantes]

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.

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.

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;
+	}
+}
+
+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

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

[Heiner Kallweit]

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
> 

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

[Gustavo Arantes]

On Mon, Apr 27, 2026 at 11:21:44PM +0200, Heiner Kallweit wrote:
> What do you mean with RTNL here?

I meant the rtnetlink link statistics exposed through ndo_get_stats64(),
i.e. struct rtnl_link_stats64.

> Counters are exposed to userspace already. So what's the benefit of
> duplicating this functionality? To me the patch looks like AI slop.

The benefit I had in mind was coverage for userspace using the netdev
qstats generic-netlink interface, so that it could query r8169 through
the same qstats path as drivers which already implement netdev_stat_ops.

That said, it does not expose additional driver-specific information.

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

Agreed, I'll drop this version. Thanks for the review.

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

[Eric Joyner]

On 4/27/2026 1:59 PM, 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.
> 
> 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.
> 
> 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;
> +	}
> +}
> +
> +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
> 

The comment for struct netdev_stat_ops says:

> * Device drivers are encouraged to reset the per-queue statistics when
> * number of queues change. This is because the primary use case for
> * per-queue statistics is currently to detect traffic imbalance.

But the commit message says there's only one Tx/Rx queue. Then that makes
this patch nonsensical? What is this actually for?

- Eric