[PATCH net-next 0/6] sfc: per-queue stats

[PATCH net-next 0/6] sfc: per-queue stats

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

This series implements the netdev_stat_ops interface for per-queue
 statistics in the sfc driver, mostly using existing counters that
 were originally added for ethtool -S output.

Edward Cree (6):
  sfc: remove obsolete counters from struct efx_channel
  sfc: implement basic per-queue stats
  sfc: add n_rx_overlength to ethtool stats
  sfc: implement per-queue rx drop and overrun stats
  sfc: implement per-queue TSO (hw_gso) stats
  sfc: add per-queue RX and TX bytes stats

 drivers/net/ethernet/sfc/ef100_rx.c       |   5 +-
 drivers/net/ethernet/sfc/ef100_tx.c       |   1 +
 drivers/net/ethernet/sfc/efx.c            | 106 ++++++++++++++++++++++
 drivers/net/ethernet/sfc/efx_channels.c   |   4 +
 drivers/net/ethernet/sfc/efx_channels.h   |   8 +-
 drivers/net/ethernet/sfc/ethtool_common.c |   3 +-
 drivers/net/ethernet/sfc/net_driver.h     |  31 ++++++-
 drivers/net/ethernet/sfc/rx.c             |   5 +-
 drivers/net/ethernet/sfc/rx_common.c      |   3 +
 drivers/net/ethernet/sfc/tx.c             |   2 +
 drivers/net/ethernet/sfc/tx_common.c      |   5 +
 11 files changed, 162 insertions(+), 11 deletions(-)

[PATCH net-next 1/6] sfc: remove obsolete counters from struct efx_channel

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

The n_rx_tobe_disc and n_rx_mcast_mismatch counters are a legacy
 from farch, and are never written in EF10 or EF100 code.  Remove
 them from the struct and from ethtool -S output, saving a bit of
 memory and avoiding user confusion.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/ethtool_common.c | 2 --
 drivers/net/ethernet/sfc/net_driver.h     | 4 ----
 2 files changed, 6 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ethtool_common.c b/drivers/net/ethernet/sfc/ethtool_common.c
index 6ded44b86052..a8baeacd83c0 100644
--- a/drivers/net/ethernet/sfc/ethtool_common.c
+++ b/drivers/net/ethernet/sfc/ethtool_common.c
@@ -75,7 +75,6 @@ static const struct efx_sw_stat_desc efx_sw_stat_desc[] = {
 	EFX_ETHTOOL_UINT_TXQ_STAT(pio_packets),
 	EFX_ETHTOOL_UINT_TXQ_STAT(cb_packets),
 	EFX_ETHTOOL_ATOMIC_NIC_ERROR_STAT(rx_reset),
-	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_tobe_disc),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_ip_hdr_chksum_err),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_tcp_udp_chksum_err),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_inner_ip_hdr_chksum_err),
@@ -83,7 +82,6 @@ static const struct efx_sw_stat_desc efx_sw_stat_desc[] = {
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_outer_ip_hdr_chksum_err),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_outer_tcp_udp_chksum_err),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_eth_crc_err),
-	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_mcast_mismatch),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_frm_trunc),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_merge_events),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_merge_packets),
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index b85c51cbe7f9..4d904e1404d4 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -451,10 +451,8 @@ enum efx_sync_events_state {
  * @filter_work: Work item for efx_filter_rfs_expire()
  * @rps_flow_id: Flow IDs of filters allocated for accelerated RFS,
  *      indexed by filter ID
- * @n_rx_tobe_disc: Count of RX_TOBE_DISC errors
  * @n_rx_ip_hdr_chksum_err: Count of RX IP header checksum errors
  * @n_rx_tcp_udp_chksum_err: Count of RX TCP and UDP checksum errors
- * @n_rx_mcast_mismatch: Count of unmatched multicast frames
  * @n_rx_frm_trunc: Count of RX_FRM_TRUNC errors
  * @n_rx_overlength: Count of RX_OVERLENGTH errors
  * @n_skbuff_leaks: Count of skbuffs leaked due to RX overrun
@@ -511,7 +509,6 @@ struct efx_channel {
 	u32 *rps_flow_id;
 #endif
 
-	unsigned int n_rx_tobe_disc;
 	unsigned int n_rx_ip_hdr_chksum_err;
 	unsigned int n_rx_tcp_udp_chksum_err;
 	unsigned int n_rx_outer_ip_hdr_chksum_err;
@@ -519,7 +516,6 @@ struct efx_channel {
 	unsigned int n_rx_inner_ip_hdr_chksum_err;
 	unsigned int n_rx_inner_tcp_udp_chksum_err;
 	unsigned int n_rx_eth_crc_err;
-	unsigned int n_rx_mcast_mismatch;
 	unsigned int n_rx_frm_trunc;
 	unsigned int n_rx_overlength;
 	unsigned int n_skbuff_leaks;

[PATCH net-next 2/6] sfc: implement basic per-queue stats

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

Just RX and TX packet counts for now.  We do not have per-queue
 byte counts, which causes us to fail stats.pkt_byte_sum selftest
 with "Drivers should always report basic keys" error.
Per-queue counts are since the last time the queue was inited
 (typically by efx_start_datapath(), on ifup or reconfiguration);
 device-wide total (efx_get_base_stats()) is since driver probe.
 This is not the same lifetime as rtnl_link_stats64, which uses
 firmware stats which count since FW (re)booted; this can cause a
 "Qstats are lower" or "RTNL stats are lower" failure in
 stats.pkt_byte_sum selftest.
Move the increment of rx_queue->rx_packets to match the semantics
 specified for netdev per-queue stats, i.e. just before handing
 the packet to XDP (if present) or the netstack (through GRO).
 This will affect the existing ethtool -S output which also
 reports these counters.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/ef100_rx.c     |  4 +-
 drivers/net/ethernet/sfc/efx.c          | 72 +++++++++++++++++++++++++
 drivers/net/ethernet/sfc/efx_channels.h |  1 -
 drivers/net/ethernet/sfc/net_driver.h   |  6 +++
 drivers/net/ethernet/sfc/rx.c           |  4 +-
 drivers/net/ethernet/sfc/rx_common.c    |  2 +
 drivers/net/ethernet/sfc/tx_common.c    |  2 +
 7 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ef100_rx.c b/drivers/net/ethernet/sfc/ef100_rx.c
index 83d9db71d7d7..992151775cb8 100644
--- a/drivers/net/ethernet/sfc/ef100_rx.c
+++ b/drivers/net/ethernet/sfc/ef100_rx.c
@@ -134,6 +134,8 @@ void __ef100_rx_packet(struct efx_channel *channel)
 		goto free_rx_buffer;
 	}
 
+	++rx_queue->rx_packets;
+
 	efx_rx_packet_gro(channel, rx_buf, channel->rx_pkt_n_frags, eh, csum);
 	goto out;
 
@@ -149,8 +151,6 @@ static void ef100_rx_packet(struct efx_rx_queue *rx_queue, unsigned int index)
 	struct efx_channel *channel = efx_rx_queue_channel(rx_queue);
 	struct efx_nic *efx = rx_queue->efx;
 
-	++rx_queue->rx_packets;
-
 	netif_vdbg(efx, rx_status, efx->net_dev,
 		   "RX queue %d received id %x\n",
 		   efx_rx_queue_index(rx_queue), index);
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 6f1a01ded7d4..e4656efce969 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -22,6 +22,7 @@
 #include "net_driver.h"
 #include <net/gre.h>
 #include <net/udp_tunnel.h>
+#include <net/netdev_queues.h>
 #include "efx.h"
 #include "efx_common.h"
 #include "efx_channels.h"
@@ -626,6 +627,76 @@ static const struct net_device_ops efx_netdev_ops = {
 	.ndo_bpf		= efx_xdp
 };
 
+static void efx_get_queue_stats_rx(struct net_device *net_dev, int idx,
+				   struct netdev_queue_stats_rx *stats)
+{
+	struct efx_nic *efx = efx_netdev_priv(net_dev);
+	struct efx_rx_queue *rx_queue;
+	struct efx_channel *channel;
+
+	channel = efx_get_channel(efx, idx);
+	rx_queue = efx_channel_get_rx_queue(channel);
+	/* Count only packets since last time datapath was started */
+	stats->packets = rx_queue->rx_packets - rx_queue->old_rx_packets;
+}
+
+static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,
+				   struct netdev_queue_stats_tx *stats)
+{
+	struct efx_nic *efx = efx_netdev_priv(net_dev);
+	struct efx_tx_queue *tx_queue;
+	struct efx_channel *channel;
+
+	channel = efx_get_tx_channel(efx, idx);
+	stats->packets = 0;
+	/* If a TX channel has XDP TXQs, the stats for these will be counted
+	 * under the channel rather than in base stats.  Unclear whether this
+	 * is correct behaviour, but we can't reliably exclude XDP TXes from
+	 * these stats anyway because in EFX_XDP_TX_QUEUES_BORROWED we use
+	 * the same TXQ as the core.
+	 */
+	efx_for_each_channel_tx_queue(tx_queue, channel)
+		stats->packets += tx_queue->tx_packets - tx_queue->old_tx_packets;
+}
+
+static void efx_get_base_stats(struct net_device *net_dev,
+			       struct netdev_queue_stats_rx *rx,
+			       struct netdev_queue_stats_tx *tx)
+{
+	struct efx_nic *efx = efx_netdev_priv(net_dev);
+	struct efx_tx_queue *tx_queue;
+	struct efx_rx_queue *rx_queue;
+	struct efx_channel *channel;
+
+	rx->packets = 0;
+	tx->packets = 0;
+
+	/* Count all packets on non-core queues, and packets before last
+	 * datapath start on core queues.
+	 */
+	efx_for_each_channel(channel, efx) {
+		rx_queue = efx_channel_get_rx_queue(channel);
+		if (channel->channel >= net_dev->real_num_rx_queues)
+			rx->packets += rx_queue->rx_packets;
+		else
+			rx->packets += rx_queue->old_rx_packets;
+		efx_for_each_channel_tx_queue(tx_queue, channel) {
+			if (channel->channel < efx->tx_channel_offset ||
+			    channel->channel >= efx->tx_channel_offset +
+						net_dev->real_num_tx_queues)
+				tx->packets += tx_queue->tx_packets;
+			else
+				tx->packets += tx_queue->old_tx_packets;
+		}
+	}
+}
+
+static const struct netdev_stat_ops efx_stat_ops = {
+	.get_queue_stats_rx	= efx_get_queue_stats_rx,
+	.get_queue_stats_tx	= efx_get_queue_stats_tx,
+	.get_base_stats		= efx_get_base_stats,
+};
+
 static int efx_xdp_setup_prog(struct efx_nic *efx, struct bpf_prog *prog)
 {
 	struct bpf_prog *old_prog;
@@ -716,6 +787,7 @@ static int efx_register_netdev(struct efx_nic *efx)
 	net_dev->watchdog_timeo = 5 * HZ;
 	net_dev->irq = efx->pci_dev->irq;
 	net_dev->netdev_ops = &efx_netdev_ops;
+	net_dev->stat_ops = &efx_stat_ops;
 	if (efx_nic_rev(efx) >= EFX_REV_HUNT_A0)
 		net_dev->priv_flags |= IFF_UNICAST_FLT;
 	net_dev->ethtool_ops = &efx_ethtool_ops;
diff --git a/drivers/net/ethernet/sfc/efx_channels.h b/drivers/net/ethernet/sfc/efx_channels.h
index 46b702648721..b3b5e18a69cc 100644
--- a/drivers/net/ethernet/sfc/efx_channels.h
+++ b/drivers/net/ethernet/sfc/efx_channels.h
@@ -49,5 +49,4 @@ void efx_fini_napi_channel(struct efx_channel *channel);
 void efx_fini_napi(struct efx_nic *efx);
 
 void efx_channel_dummy_op_void(struct efx_channel *channel);
-
 #endif
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 4d904e1404d4..cc96716d8dbe 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -232,6 +232,8 @@ struct efx_tx_buffer {
  * @xmit_pending: Are any packets waiting to be pushed to the NIC
  * @cb_packets: Number of times the TX copybreak feature has been used
  * @notify_count: Count of notified descriptors to the NIC
+ * @tx_packets: Number of packets sent since this struct was created
+ * @old_tx_packets: Value of @tx_packets as of last efx_init_tx_queue()
  * @empty_read_count: If the completion path has seen the queue as empty
  *	and the transmission path has not yet checked this, the value of
  *	@read_count bitwise-added to %EFX_EMPTY_COUNT_VALID; otherwise 0.
@@ -281,6 +283,7 @@ struct efx_tx_queue {
 	unsigned int notify_count;
 	/* Statistics to supplement MAC stats */
 	unsigned long tx_packets;
+	unsigned long old_tx_packets;
 
 	/* Members shared between paths and sometimes updated */
 	unsigned int empty_read_count ____cacheline_aligned_in_smp;
@@ -370,6 +373,8 @@ struct efx_rx_page_state {
  * @recycle_count: RX buffer recycle counter.
  * @slow_fill: Timer used to defer efx_nic_generate_fill_event().
  * @grant_work: workitem used to grant credits to the MAE if @grant_credits
+ * @rx_packets: Number of packets received since this struct was created
+ * @old_rx_packets: Value of @rx_packets as of last efx_init_rx_queue()
  * @xdp_rxq_info: XDP specific RX queue information.
  * @xdp_rxq_info_valid: Is xdp_rxq_info valid data?.
  */
@@ -406,6 +411,7 @@ struct efx_rx_queue {
 	struct work_struct grant_work;
 	/* Statistics to supplement MAC stats */
 	unsigned long rx_packets;
+	unsigned long old_rx_packets;
 	struct xdp_rxq_info xdp_rxq_info;
 	bool xdp_rxq_info_valid;
 };
diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index f77a2d3ef37e..f07495582125 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -125,8 +125,6 @@ void efx_rx_packet(struct efx_rx_queue *rx_queue, unsigned int index,
 	struct efx_channel *channel = efx_rx_queue_channel(rx_queue);
 	struct efx_rx_buffer *rx_buf;
 
-	rx_queue->rx_packets++;
-
 	rx_buf = efx_rx_buffer(rx_queue, index);
 	rx_buf->flags |= flags;
 
@@ -394,6 +392,8 @@ void __efx_rx_packet(struct efx_channel *channel)
 		goto out;
 	}
 
+	rx_queue->rx_packets++;
+
 	if (!efx_do_xdp(efx, channel, rx_buf, &eh))
 		goto out;
 
diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c
index 0b7dc75c40f9..bdb4325a7c2c 100644
--- a/drivers/net/ethernet/sfc/rx_common.c
+++ b/drivers/net/ethernet/sfc/rx_common.c
@@ -241,6 +241,8 @@ void efx_init_rx_queue(struct efx_rx_queue *rx_queue)
 	rx_queue->page_recycle_failed = 0;
 	rx_queue->page_recycle_full = 0;
 
+	rx_queue->old_rx_packets = rx_queue->rx_packets;
+
 	/* Initialise limit fields */
 	max_fill = efx->rxq_entries - EFX_RXD_HEAD_ROOM;
 	max_trigger =
diff --git a/drivers/net/ethernet/sfc/tx_common.c b/drivers/net/ethernet/sfc/tx_common.c
index 2adb132b2f7e..f1694900e0f0 100644
--- a/drivers/net/ethernet/sfc/tx_common.c
+++ b/drivers/net/ethernet/sfc/tx_common.c
@@ -86,6 +86,8 @@ void efx_init_tx_queue(struct efx_tx_queue *tx_queue)
 	tx_queue->completed_timestamp_major = 0;
 	tx_queue->completed_timestamp_minor = 0;
 
+	tx_queue->old_tx_packets = tx_queue->tx_packets;
+
 	tx_queue->xdp_tx = efx_channel_is_xdp_tx(tx_queue->channel);
 	tx_queue->tso_version = 0;
 

[PATCH net-next 3/6] sfc: add n_rx_overlength to ethtool stats

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

This counter is the main difference between the old and new locations
 of the rx_packets increment (the other is scatter errors which
 produce a WARN_ON).  It previously was not reported anywhere; add it
 to ethtool -S output to ensure users still have this information.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/ethtool_common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/sfc/ethtool_common.c b/drivers/net/ethernet/sfc/ethtool_common.c
index a8baeacd83c0..ae32e08540fa 100644
--- a/drivers/net/ethernet/sfc/ethtool_common.c
+++ b/drivers/net/ethernet/sfc/ethtool_common.c
@@ -83,6 +83,7 @@ static const struct efx_sw_stat_desc efx_sw_stat_desc[] = {
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_outer_tcp_udp_chksum_err),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_eth_crc_err),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_frm_trunc),
+	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_overlength),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_merge_events),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_merge_packets),
 	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_xdp_drops),

[PATCH net-next 4/6] sfc: implement per-queue rx drop and overrun stats

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/efx.c          | 15 +++++++++++++--
 drivers/net/ethernet/sfc/efx_channels.c |  4 ++++
 drivers/net/ethernet/sfc/efx_channels.h |  7 +++++++
 drivers/net/ethernet/sfc/net_driver.h   |  7 +++++++
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index e4656efce969..8b46d143b6c7 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -638,6 +638,10 @@ static void efx_get_queue_stats_rx(struct net_device *net_dev, int idx,
 	rx_queue = efx_channel_get_rx_queue(channel);
 	/* Count only packets since last time datapath was started */
 	stats->packets = rx_queue->rx_packets - rx_queue->old_rx_packets;
+	stats->hw_drops = efx_get_queue_stat_rx_hw_drops(channel) -
+			  channel->old_n_rx_hw_drops;
+	stats->hw_drop_overruns = channel->n_rx_nodesc_trunc -
+				  channel->old_n_rx_hw_drop_overruns;
 }
 
 static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,
@@ -669,6 +673,8 @@ static void efx_get_base_stats(struct net_device *net_dev,
 	struct efx_channel *channel;
 
 	rx->packets = 0;
+	rx->hw_drops = 0;
+	rx->hw_drop_overruns = 0;
 	tx->packets = 0;
 
 	/* Count all packets on non-core queues, and packets before last
@@ -676,10 +682,15 @@ static void efx_get_base_stats(struct net_device *net_dev,
 	 */
 	efx_for_each_channel(channel, efx) {
 		rx_queue = efx_channel_get_rx_queue(channel);
-		if (channel->channel >= net_dev->real_num_rx_queues)
+		if (channel->channel >= net_dev->real_num_rx_queues) {
 			rx->packets += rx_queue->rx_packets;
-		else
+			rx->hw_drops += efx_get_queue_stat_rx_hw_drops(channel);
+			rx->hw_drop_overruns += channel->n_rx_nodesc_trunc;
+		} else {
 			rx->packets += rx_queue->old_rx_packets;
+			rx->hw_drops += channel->old_n_rx_hw_drops;
+			rx->hw_drop_overruns += channel->old_n_rx_hw_drop_overruns;
+		}
 		efx_for_each_channel_tx_queue(tx_queue, channel) {
 			if (channel->channel < efx->tx_channel_offset ||
 			    channel->channel >= efx->tx_channel_offset +
diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
index c9e17a8208a9..90b9986ceaa3 100644
--- a/drivers/net/ethernet/sfc/efx_channels.c
+++ b/drivers/net/ethernet/sfc/efx_channels.c
@@ -1100,6 +1100,10 @@ void efx_start_channels(struct efx_nic *efx)
 			atomic_inc(&efx->active_queues);
 		}
 
+		/* reset per-queue stats */
+		channel->old_n_rx_hw_drops = efx_get_queue_stat_rx_hw_drops(channel);
+		channel->old_n_rx_hw_drop_overruns = channel->n_rx_nodesc_trunc;
+
 		efx_for_each_channel_rx_queue(rx_queue, channel) {
 			efx_init_rx_queue(rx_queue);
 			atomic_inc(&efx->active_queues);
diff --git a/drivers/net/ethernet/sfc/efx_channels.h b/drivers/net/ethernet/sfc/efx_channels.h
index b3b5e18a69cc..cccbc7d66e77 100644
--- a/drivers/net/ethernet/sfc/efx_channels.h
+++ b/drivers/net/ethernet/sfc/efx_channels.h
@@ -43,6 +43,13 @@ struct efx_channel *efx_copy_channel(const struct efx_channel *old_channel);
 void efx_start_channels(struct efx_nic *efx);
 void efx_stop_channels(struct efx_nic *efx);
 
+static inline u64 efx_get_queue_stat_rx_hw_drops(struct efx_channel *channel)
+{
+	return channel->n_rx_eth_crc_err + channel->n_rx_frm_trunc +
+	       channel->n_rx_overlength + channel->n_rx_nodesc_trunc +
+	       channel->n_rx_mport_bad;
+}
+
 void efx_init_napi_channel(struct efx_channel *channel);
 void efx_init_napi(struct efx_nic *efx);
 void efx_fini_napi_channel(struct efx_channel *channel);
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index cc96716d8dbe..25701f37aa40 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -472,6 +472,10 @@ enum efx_sync_events_state {
  * @n_rx_xdp_redirect: Count of RX packets redirected to a different NIC by XDP
  * @n_rx_mport_bad: Count of RX packets dropped because their ingress mport was
  *	not recognised
+ * @old_n_rx_hw_drops: Count of all RX packets dropped for any reason as of last
+ *	efx_start_channels()
+ * @old_n_rx_hw_drop_overruns: Value of @n_rx_nodesc_trunc as of last
+ *	efx_start_channels()
  * @rx_pkt_n_frags: Number of fragments in next packet to be delivered by
  *	__efx_rx_packet(), or zero if there is none
  * @rx_pkt_index: Ring index of first buffer for next packet to be delivered
@@ -534,6 +538,9 @@ struct efx_channel {
 	unsigned int n_rx_xdp_redirect;
 	unsigned int n_rx_mport_bad;
 
+	unsigned int old_n_rx_hw_drops;
+	unsigned int old_n_rx_hw_drop_overruns;
+
 	unsigned int rx_pkt_n_frags;
 	unsigned int rx_pkt_index;
 

[PATCH net-next 5/6] sfc: implement per-queue TSO (hw_gso) stats

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/efx.c        | 20 +++++++++++++++++---
 drivers/net/ethernet/sfc/net_driver.h |  4 ++++
 drivers/net/ethernet/sfc/tx_common.c  |  2 ++
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 8b46d143b6c7..bf06fbcdcbff 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -653,14 +653,21 @@ static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,
 
 	channel = efx_get_tx_channel(efx, idx);
 	stats->packets = 0;
+	stats->hw_gso_packets = 0;
+	stats->hw_gso_wire_packets = 0;
 	/* If a TX channel has XDP TXQs, the stats for these will be counted
 	 * under the channel rather than in base stats.  Unclear whether this
 	 * is correct behaviour, but we can't reliably exclude XDP TXes from
 	 * these stats anyway because in EFX_XDP_TX_QUEUES_BORROWED we use
 	 * the same TXQ as the core.
 	 */
-	efx_for_each_channel_tx_queue(tx_queue, channel)
+	efx_for_each_channel_tx_queue(tx_queue, channel) {
 		stats->packets += tx_queue->tx_packets - tx_queue->old_tx_packets;
+		stats->hw_gso_packets += tx_queue->tso_bursts -
+					 tx_queue->old_tso_bursts;
+		stats->hw_gso_wire_packets += tx_queue->tso_packets -
+					      tx_queue->old_tso_packets;
+	}
 }
 
 static void efx_get_base_stats(struct net_device *net_dev,
@@ -676,6 +683,8 @@ static void efx_get_base_stats(struct net_device *net_dev,
 	rx->hw_drops = 0;
 	rx->hw_drop_overruns = 0;
 	tx->packets = 0;
+	tx->hw_gso_packets = 0;
+	tx->hw_gso_wire_packets = 0;
 
 	/* Count all packets on non-core queues, and packets before last
 	 * datapath start on core queues.
@@ -694,10 +703,15 @@ static void efx_get_base_stats(struct net_device *net_dev,
 		efx_for_each_channel_tx_queue(tx_queue, channel) {
 			if (channel->channel < efx->tx_channel_offset ||
 			    channel->channel >= efx->tx_channel_offset +
-						net_dev->real_num_tx_queues)
+						net_dev->real_num_tx_queues) {
 				tx->packets += tx_queue->tx_packets;
-			else
+				tx->hw_gso_packets += tx_queue->tso_bursts;
+				tx->hw_gso_wire_packets += tx_queue->tso_packets;
+			} else {
 				tx->packets += tx_queue->old_tx_packets;
+				tx->hw_gso_packets += tx_queue->old_tso_bursts;
+				tx->hw_gso_wire_packets += tx_queue->old_tso_packets;
+			}
 		}
 	}
 }
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 25701f37aa40..2cf2935a713c 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -234,6 +234,8 @@ struct efx_tx_buffer {
  * @notify_count: Count of notified descriptors to the NIC
  * @tx_packets: Number of packets sent since this struct was created
  * @old_tx_packets: Value of @tx_packets as of last efx_init_tx_queue()
+ * @old_tso_bursts: Value of @tso_bursts as of last efx_init_tx_queue()
+ * @old_tso_packets: Value of @tso_packets as of last efx_init_tx_queue()
  * @empty_read_count: If the completion path has seen the queue as empty
  *	and the transmission path has not yet checked this, the value of
  *	@read_count bitwise-added to %EFX_EMPTY_COUNT_VALID; otherwise 0.
@@ -284,6 +286,8 @@ struct efx_tx_queue {
 	/* Statistics to supplement MAC stats */
 	unsigned long tx_packets;
 	unsigned long old_tx_packets;
+	unsigned int old_tso_bursts;
+	unsigned int old_tso_packets;
 
 	/* Members shared between paths and sometimes updated */
 	unsigned int empty_read_count ____cacheline_aligned_in_smp;
diff --git a/drivers/net/ethernet/sfc/tx_common.c b/drivers/net/ethernet/sfc/tx_common.c
index f1694900e0f0..cd0857131aa8 100644
--- a/drivers/net/ethernet/sfc/tx_common.c
+++ b/drivers/net/ethernet/sfc/tx_common.c
@@ -87,6 +87,8 @@ void efx_init_tx_queue(struct efx_tx_queue *tx_queue)
 	tx_queue->completed_timestamp_minor = 0;
 
 	tx_queue->old_tx_packets = tx_queue->tx_packets;
+	tx_queue->old_tso_bursts = tx_queue->tso_bursts;
+	tx_queue->old_tso_packets = tx_queue->tso_packets;
 
 	tx_queue->xdp_tx = efx_channel_is_xdp_tx(tx_queue->channel);
 	tx_queue->tso_version = 0;

[PATCH net-next 6/6] sfc: add per-queue RX and TX bytes stats

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

While this does add overhead to the fast path, it should be minimal
 as the cacheline should already be held for write from updating the
 queue's [tr]x_packets stat.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/ef100_rx.c   |  1 +
 drivers/net/ethernet/sfc/ef100_tx.c   |  1 +
 drivers/net/ethernet/sfc/efx.c        |  9 +++++++++
 drivers/net/ethernet/sfc/net_driver.h | 10 ++++++++++
 drivers/net/ethernet/sfc/rx.c         |  1 +
 drivers/net/ethernet/sfc/rx_common.c  |  1 +
 drivers/net/ethernet/sfc/tx.c         |  2 ++
 drivers/net/ethernet/sfc/tx_common.c  |  1 +
 8 files changed, 26 insertions(+)

diff --git a/drivers/net/ethernet/sfc/ef100_rx.c b/drivers/net/ethernet/sfc/ef100_rx.c
index 992151775cb8..44dc75feb162 100644
--- a/drivers/net/ethernet/sfc/ef100_rx.c
+++ b/drivers/net/ethernet/sfc/ef100_rx.c
@@ -135,6 +135,7 @@ void __ef100_rx_packet(struct efx_channel *channel)
 	}
 
 	++rx_queue->rx_packets;
+	rx_queue->rx_bytes += rx_buf->len;
 
 	efx_rx_packet_gro(channel, rx_buf, channel->rx_pkt_n_frags, eh, csum);
 	goto out;
diff --git a/drivers/net/ethernet/sfc/ef100_tx.c b/drivers/net/ethernet/sfc/ef100_tx.c
index e6b6be549581..a7e30289e231 100644
--- a/drivers/net/ethernet/sfc/ef100_tx.c
+++ b/drivers/net/ethernet/sfc/ef100_tx.c
@@ -493,6 +493,7 @@ int __ef100_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb,
 	} else {
 		tx_queue->tx_packets++;
 	}
+	tx_queue->tx_bytes += skb->len;
 	return 0;
 
 err:
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index bf06fbcdcbff..b3fbffed4e61 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -638,6 +638,7 @@ static void efx_get_queue_stats_rx(struct net_device *net_dev, int idx,
 	rx_queue = efx_channel_get_rx_queue(channel);
 	/* Count only packets since last time datapath was started */
 	stats->packets = rx_queue->rx_packets - rx_queue->old_rx_packets;
+	stats->bytes = rx_queue->rx_bytes - rx_queue->old_rx_bytes;
 	stats->hw_drops = efx_get_queue_stat_rx_hw_drops(channel) -
 			  channel->old_n_rx_hw_drops;
 	stats->hw_drop_overruns = channel->n_rx_nodesc_trunc -
@@ -653,6 +654,7 @@ static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,
 
 	channel = efx_get_tx_channel(efx, idx);
 	stats->packets = 0;
+	stats->bytes = 0;
 	stats->hw_gso_packets = 0;
 	stats->hw_gso_wire_packets = 0;
 	/* If a TX channel has XDP TXQs, the stats for these will be counted
@@ -663,6 +665,7 @@ static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,
 	 */
 	efx_for_each_channel_tx_queue(tx_queue, channel) {
 		stats->packets += tx_queue->tx_packets - tx_queue->old_tx_packets;
+		stats->bytes += tx_queue->tx_bytes - tx_queue->old_tx_bytes;
 		stats->hw_gso_packets += tx_queue->tso_bursts -
 					 tx_queue->old_tso_bursts;
 		stats->hw_gso_wire_packets += tx_queue->tso_packets -
@@ -680,9 +683,11 @@ static void efx_get_base_stats(struct net_device *net_dev,
 	struct efx_channel *channel;
 
 	rx->packets = 0;
+	rx->bytes = 0;
 	rx->hw_drops = 0;
 	rx->hw_drop_overruns = 0;
 	tx->packets = 0;
+	tx->bytes = 0;
 	tx->hw_gso_packets = 0;
 	tx->hw_gso_wire_packets = 0;
 
@@ -693,10 +698,12 @@ static void efx_get_base_stats(struct net_device *net_dev,
 		rx_queue = efx_channel_get_rx_queue(channel);
 		if (channel->channel >= net_dev->real_num_rx_queues) {
 			rx->packets += rx_queue->rx_packets;
+			rx->bytes += rx_queue->rx_bytes;
 			rx->hw_drops += efx_get_queue_stat_rx_hw_drops(channel);
 			rx->hw_drop_overruns += channel->n_rx_nodesc_trunc;
 		} else {
 			rx->packets += rx_queue->old_rx_packets;
+			rx->bytes += rx_queue->old_rx_bytes;
 			rx->hw_drops += channel->old_n_rx_hw_drops;
 			rx->hw_drop_overruns += channel->old_n_rx_hw_drop_overruns;
 		}
@@ -705,10 +712,12 @@ static void efx_get_base_stats(struct net_device *net_dev,
 			    channel->channel >= efx->tx_channel_offset +
 						net_dev->real_num_tx_queues) {
 				tx->packets += tx_queue->tx_packets;
+				tx->bytes += tx_queue->tx_bytes;
 				tx->hw_gso_packets += tx_queue->tso_bursts;
 				tx->hw_gso_wire_packets += tx_queue->tso_packets;
 			} else {
 				tx->packets += tx_queue->old_tx_packets;
+				tx->bytes += tx_queue->old_tx_bytes;
 				tx->hw_gso_packets += tx_queue->old_tso_bursts;
 				tx->hw_gso_wire_packets += tx_queue->old_tso_packets;
 			}
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 2cf2935a713c..147052c1e25a 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -233,7 +233,11 @@ struct efx_tx_buffer {
  * @cb_packets: Number of times the TX copybreak feature has been used
  * @notify_count: Count of notified descriptors to the NIC
  * @tx_packets: Number of packets sent since this struct was created
+ * @tx_bytes: Number of bytes sent since this struct was created.  For TSO,
+ *	counts the superframe size, not the sizes of generated frames on the
+ *	wire (i.e. the headers are only counted once)
  * @old_tx_packets: Value of @tx_packets as of last efx_init_tx_queue()
+ * @old_tx_bytes: Value of @tx_bytes as of last efx_init_tx_queue()
  * @old_tso_bursts: Value of @tso_bursts as of last efx_init_tx_queue()
  * @old_tso_packets: Value of @tso_packets as of last efx_init_tx_queue()
  * @empty_read_count: If the completion path has seen the queue as empty
@@ -285,7 +289,9 @@ struct efx_tx_queue {
 	unsigned int notify_count;
 	/* Statistics to supplement MAC stats */
 	unsigned long tx_packets;
+	unsigned long tx_bytes;
 	unsigned long old_tx_packets;
+	unsigned long old_tx_bytes;
 	unsigned int old_tso_bursts;
 	unsigned int old_tso_packets;
 
@@ -378,7 +384,9 @@ struct efx_rx_page_state {
  * @slow_fill: Timer used to defer efx_nic_generate_fill_event().
  * @grant_work: workitem used to grant credits to the MAE if @grant_credits
  * @rx_packets: Number of packets received since this struct was created
+ * @rx_bytes: Number of bytes received since this struct was created
  * @old_rx_packets: Value of @rx_packets as of last efx_init_rx_queue()
+ * @old_rx_bytes: Value of @rx_bytes as of last efx_init_rx_queue()
  * @xdp_rxq_info: XDP specific RX queue information.
  * @xdp_rxq_info_valid: Is xdp_rxq_info valid data?.
  */
@@ -415,7 +423,9 @@ struct efx_rx_queue {
 	struct work_struct grant_work;
 	/* Statistics to supplement MAC stats */
 	unsigned long rx_packets;
+	unsigned long rx_bytes;
 	unsigned long old_rx_packets;
+	unsigned long old_rx_bytes;
 	struct xdp_rxq_info xdp_rxq_info;
 	bool xdp_rxq_info_valid;
 };
diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index f07495582125..ffca82207e47 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -393,6 +393,7 @@ void __efx_rx_packet(struct efx_channel *channel)
 	}
 
 	rx_queue->rx_packets++;
+	rx_queue->rx_bytes += rx_buf->len;
 
 	if (!efx_do_xdp(efx, channel, rx_buf, &eh))
 		goto out;
diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c
index bdb4325a7c2c..ab358fe13e1d 100644
--- a/drivers/net/ethernet/sfc/rx_common.c
+++ b/drivers/net/ethernet/sfc/rx_common.c
@@ -242,6 +242,7 @@ void efx_init_rx_queue(struct efx_rx_queue *rx_queue)
 	rx_queue->page_recycle_full = 0;
 
 	rx_queue->old_rx_packets = rx_queue->rx_packets;
+	rx_queue->old_rx_bytes = rx_queue->rx_bytes;
 
 	/* Initialise limit fields */
 	max_fill = efx->rxq_entries - EFX_RXD_HEAD_ROOM;
diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index fe2d476028e7..1aea19488a56 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -394,6 +394,7 @@ netdev_tx_t __efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb
 	} else {
 		tx_queue->tx_packets++;
 	}
+	tx_queue->tx_bytes += skb_len;
 
 	return NETDEV_TX_OK;
 
@@ -490,6 +491,7 @@ int efx_xdp_tx_buffers(struct efx_nic *efx, int n, struct xdp_frame **xdpfs,
 		tx_buffer->dma_offset = 0;
 		tx_buffer->unmap_len = len;
 		tx_queue->tx_packets++;
+		tx_queue->tx_bytes += len;
 	}
 
 	/* Pass mapped frames to hardware. */
diff --git a/drivers/net/ethernet/sfc/tx_common.c b/drivers/net/ethernet/sfc/tx_common.c
index cd0857131aa8..7ef2baa3439a 100644
--- a/drivers/net/ethernet/sfc/tx_common.c
+++ b/drivers/net/ethernet/sfc/tx_common.c
@@ -87,6 +87,7 @@ void efx_init_tx_queue(struct efx_tx_queue *tx_queue)
 	tx_queue->completed_timestamp_minor = 0;
 
 	tx_queue->old_tx_packets = tx_queue->tx_packets;
+	tx_queue->old_tx_bytes = tx_queue->tx_bytes;
 	tx_queue->old_tso_bursts = tx_queue->tso_bursts;
 	tx_queue->old_tso_packets = tx_queue->tso_packets;
 

Re: [PATCH net-next 1/6] sfc: remove obsolete counters from struct efx_channel

[Jacob Keller]

On 8/28/2024 6:45 AM, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> The n_rx_tobe_disc and n_rx_mcast_mismatch counters are a legacy
>  from farch, and are never written in EF10 or EF100 code.  Remove
>  them from the struct and from ethtool -S output, saving a bit of
>  memory and avoiding user confusion.
> 
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH net-next 2/6] sfc: implement basic per-queue stats

[Jacob Keller]

On 8/28/2024 6:45 AM, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> Just RX and TX packet counts for now.  We do not have per-queue
>  byte counts, which causes us to fail stats.pkt_byte_sum selftest
>  with "Drivers should always report basic keys" error.

Seems like the self tests here should be fixed for this?

> Per-queue counts are since the last time the queue was inited
>  (typically by efx_start_datapath(), on ifup or reconfiguration);
>  device-wide total (efx_get_base_stats()) is since driver probe.
>  This is not the same lifetime as rtnl_link_stats64, which uses
>  firmware stats which count since FW (re)booted; this can cause a
>  "Qstats are lower" or "RTNL stats are lower" failure in
>  stats.pkt_byte_sum selftest.

Could we have software somehow manage this so that the actual reported
stats correctly survive the lifetime of the FW instead of surviving only
from the queue initialization?

> Move the increment of rx_queue->rx_packets to match the semantics
>  specified for netdev per-queue stats, i.e. just before handing
>  the packet to XDP (if present) or the netstack (through GRO).
>  This will affect the existing ethtool -S output which also
>  reports these counters.
> 
Seems reasonable.

Re: [PATCH net-next 3/6] sfc: add n_rx_overlength to ethtool stats

[Jacob Keller]

On 8/28/2024 6:45 AM, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> This counter is the main difference between the old and new locations
>  of the rx_packets increment (the other is scatter errors which
>  produce a WARN_ON).  It previously was not reported anywhere; add it
>  to ethtool -S output to ensure users still have this information.
> 

The description makes sense in context with the whole series but doesn't
quite work for me if I think about viewing it without context. Perhaps a
little more clarification about the rx_packets behavioral change?

> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/sfc/ethtool_common.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/sfc/ethtool_common.c b/drivers/net/ethernet/sfc/ethtool_common.c
> index a8baeacd83c0..ae32e08540fa 100644
> --- a/drivers/net/ethernet/sfc/ethtool_common.c
> +++ b/drivers/net/ethernet/sfc/ethtool_common.c
> @@ -83,6 +83,7 @@ static const struct efx_sw_stat_desc efx_sw_stat_desc[] = {
>  	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_outer_tcp_udp_chksum_err),
>  	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_eth_crc_err),
>  	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_frm_trunc),
> +	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_overlength),
>  	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_merge_events),
>  	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_merge_packets),
>  	EFX_ETHTOOL_UINT_CHANNEL_STAT(rx_xdp_drops),
> 

Re: [PATCH net-next 4/6] sfc: implement per-queue rx drop and overrun stats

[Jacob Keller]

On 8/28/2024 6:45 AM, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/sfc/efx.c          | 15 +++++++++++++--
>  drivers/net/ethernet/sfc/efx_channels.c |  4 ++++
>  drivers/net/ethernet/sfc/efx_channels.h |  7 +++++++
>  drivers/net/ethernet/sfc/net_driver.h   |  7 +++++++
>  4 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
> index e4656efce969..8b46d143b6c7 100644
> --- a/drivers/net/ethernet/sfc/efx.c
> +++ b/drivers/net/ethernet/sfc/efx.c
> @@ -638,6 +638,10 @@ static void efx_get_queue_stats_rx(struct net_device *net_dev, int idx,
>  	rx_queue = efx_channel_get_rx_queue(channel);
>  	/* Count only packets since last time datapath was started */
>  	stats->packets = rx_queue->rx_packets - rx_queue->old_rx_packets;
> +	stats->hw_drops = efx_get_queue_stat_rx_hw_drops(channel) -
> +			  channel->old_n_rx_hw_drops;
> +	stats->hw_drop_overruns = channel->n_rx_nodesc_trunc -
> +				  channel->old_n_rx_hw_drop_overruns;
>  }
>  
>  static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,
> @@ -669,6 +673,8 @@ static void efx_get_base_stats(struct net_device *net_dev,
>  	struct efx_channel *channel;
>  
>  	rx->packets = 0;
> +	rx->hw_drops = 0;
> +	rx->hw_drop_overruns = 0;
>  	tx->packets = 0;
>  
>  	/* Count all packets on non-core queues, and packets before last
> @@ -676,10 +682,15 @@ static void efx_get_base_stats(struct net_device *net_dev,
>  	 */
>  	efx_for_each_channel(channel, efx) {
>  		rx_queue = efx_channel_get_rx_queue(channel);
> -		if (channel->channel >= net_dev->real_num_rx_queues)
> +		if (channel->channel >= net_dev->real_num_rx_queues) {
>  			rx->packets += rx_queue->rx_packets;
> -		else
> +			rx->hw_drops += efx_get_queue_stat_rx_hw_drops(channel);
> +			rx->hw_drop_overruns += channel->n_rx_nodesc_trunc;
> +		} else {
>  			rx->packets += rx_queue->old_rx_packets;
> +			rx->hw_drops += channel->old_n_rx_hw_drops;
> +			rx->hw_drop_overruns += channel->old_n_rx_hw_drop_overruns;
> +		}
>  		efx_for_each_channel_tx_queue(tx_queue, channel) {
>  			if (channel->channel < efx->tx_channel_offset ||
>  			    channel->channel >= efx->tx_channel_offset +
> diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
> index c9e17a8208a9..90b9986ceaa3 100644
> --- a/drivers/net/ethernet/sfc/efx_channels.c
> +++ b/drivers/net/ethernet/sfc/efx_channels.c
> @@ -1100,6 +1100,10 @@ void efx_start_channels(struct efx_nic *efx)
>  			atomic_inc(&efx->active_queues);
>  		}
>  
> +		/* reset per-queue stats */
> +		channel->old_n_rx_hw_drops = efx_get_queue_stat_rx_hw_drops(channel);
> +		channel->old_n_rx_hw_drop_overruns = channel->n_rx_nodesc_trunc;
> +
>  		efx_for_each_channel_rx_queue(rx_queue, channel) {
>  			efx_init_rx_queue(rx_queue);
>  			atomic_inc(&efx->active_queues);
> diff --git a/drivers/net/ethernet/sfc/efx_channels.h b/drivers/net/ethernet/sfc/efx_channels.h
> index b3b5e18a69cc..cccbc7d66e77 100644
> --- a/drivers/net/ethernet/sfc/efx_channels.h
> +++ b/drivers/net/ethernet/sfc/efx_channels.h
> @@ -43,6 +43,13 @@ struct efx_channel *efx_copy_channel(const struct efx_channel *old_channel);
>  void efx_start_channels(struct efx_nic *efx);
>  void efx_stop_channels(struct efx_nic *efx);
>  
> +static inline u64 efx_get_queue_stat_rx_hw_drops(struct efx_channel *channel)
> +{
> +	return channel->n_rx_eth_crc_err + channel->n_rx_frm_trunc +
> +	       channel->n_rx_overlength + channel->n_rx_nodesc_trunc +
> +	       channel->n_rx_mport_bad;
> +}
> +
>  void efx_init_napi_channel(struct efx_channel *channel);
>  void efx_init_napi(struct efx_nic *efx);
>  void efx_fini_napi_channel(struct efx_channel *channel);
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index cc96716d8dbe..25701f37aa40 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -472,6 +472,10 @@ enum efx_sync_events_state {
>   * @n_rx_xdp_redirect: Count of RX packets redirected to a different NIC by XDP
>   * @n_rx_mport_bad: Count of RX packets dropped because their ingress mport was
>   *	not recognised
> + * @old_n_rx_hw_drops: Count of all RX packets dropped for any reason as of last
> + *	efx_start_channels()
> + * @old_n_rx_hw_drop_overruns: Value of @n_rx_nodesc_trunc as of last
> + *	efx_start_channels()
>   * @rx_pkt_n_frags: Number of fragments in next packet to be delivered by
>   *	__efx_rx_packet(), or zero if there is none
>   * @rx_pkt_index: Ring index of first buffer for next packet to be delivered
> @@ -534,6 +538,9 @@ struct efx_channel {
>  	unsigned int n_rx_xdp_redirect;
>  	unsigned int n_rx_mport_bad;
>  
> +	unsigned int old_n_rx_hw_drops;
> +	unsigned int old_n_rx_hw_drop_overruns;
> +
>  	unsigned int rx_pkt_n_frags;
>  	unsigned int rx_pkt_index;
>  
> 

Re: [PATCH net-next 5/6] sfc: implement per-queue TSO (hw_gso) stats

[Jacob Keller]

On 8/28/2024 6:45 AM, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/sfc/efx.c        | 20 +++++++++++++++++---
>  drivers/net/ethernet/sfc/net_driver.h |  4 ++++
>  drivers/net/ethernet/sfc/tx_common.c  |  2 ++
>  3 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
> index 8b46d143b6c7..bf06fbcdcbff 100644
> --- a/drivers/net/ethernet/sfc/efx.c
> +++ b/drivers/net/ethernet/sfc/efx.c
> @@ -653,14 +653,21 @@ static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,
>  
>  	channel = efx_get_tx_channel(efx, idx);
>  	stats->packets = 0;
> +	stats->hw_gso_packets = 0;
> +	stats->hw_gso_wire_packets = 0;
>  	/* If a TX channel has XDP TXQs, the stats for these will be counted
>  	 * under the channel rather than in base stats.  Unclear whether this
>  	 * is correct behaviour, but we can't reliably exclude XDP TXes from
>  	 * these stats anyway because in EFX_XDP_TX_QUEUES_BORROWED we use
>  	 * the same TXQ as the core.
>  	 */
> -	efx_for_each_channel_tx_queue(tx_queue, channel)
> +	efx_for_each_channel_tx_queue(tx_queue, channel) {
>  		stats->packets += tx_queue->tx_packets - tx_queue->old_tx_packets;
> +		stats->hw_gso_packets += tx_queue->tso_bursts -
> +					 tx_queue->old_tso_bursts;
> +		stats->hw_gso_wire_packets += tx_queue->tso_packets -
> +					      tx_queue->old_tso_packets;
> +	}
>  }
>  
>  static void efx_get_base_stats(struct net_device *net_dev,
> @@ -676,6 +683,8 @@ static void efx_get_base_stats(struct net_device *net_dev,
>  	rx->hw_drops = 0;
>  	rx->hw_drop_overruns = 0;
>  	tx->packets = 0;
> +	tx->hw_gso_packets = 0;
> +	tx->hw_gso_wire_packets = 0;
>  
>  	/* Count all packets on non-core queues, and packets before last
>  	 * datapath start on core queues.
> @@ -694,10 +703,15 @@ static void efx_get_base_stats(struct net_device *net_dev,
>  		efx_for_each_channel_tx_queue(tx_queue, channel) {
>  			if (channel->channel < efx->tx_channel_offset ||
>  			    channel->channel >= efx->tx_channel_offset +
> -						net_dev->real_num_tx_queues)
> +						net_dev->real_num_tx_queues) {
>  				tx->packets += tx_queue->tx_packets;
> -			else
> +				tx->hw_gso_packets += tx_queue->tso_bursts;
> +				tx->hw_gso_wire_packets += tx_queue->tso_packets;
> +			} else {
>  				tx->packets += tx_queue->old_tx_packets;
> +				tx->hw_gso_packets += tx_queue->old_tso_bursts;
> +				tx->hw_gso_wire_packets += tx_queue->old_tso_packets;
> +			}
>  		}
>  	}
>  }
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index 25701f37aa40..2cf2935a713c 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -234,6 +234,8 @@ struct efx_tx_buffer {
>   * @notify_count: Count of notified descriptors to the NIC
>   * @tx_packets: Number of packets sent since this struct was created
>   * @old_tx_packets: Value of @tx_packets as of last efx_init_tx_queue()
> + * @old_tso_bursts: Value of @tso_bursts as of last efx_init_tx_queue()
> + * @old_tso_packets: Value of @tso_packets as of last efx_init_tx_queue()
>   * @empty_read_count: If the completion path has seen the queue as empty
>   *	and the transmission path has not yet checked this, the value of
>   *	@read_count bitwise-added to %EFX_EMPTY_COUNT_VALID; otherwise 0.
> @@ -284,6 +286,8 @@ struct efx_tx_queue {
>  	/* Statistics to supplement MAC stats */
>  	unsigned long tx_packets;
>  	unsigned long old_tx_packets;
> +	unsigned int old_tso_bursts;
> +	unsigned int old_tso_packets;
>  
>  	/* Members shared between paths and sometimes updated */
>  	unsigned int empty_read_count ____cacheline_aligned_in_smp;
> diff --git a/drivers/net/ethernet/sfc/tx_common.c b/drivers/net/ethernet/sfc/tx_common.c
> index f1694900e0f0..cd0857131aa8 100644
> --- a/drivers/net/ethernet/sfc/tx_common.c
> +++ b/drivers/net/ethernet/sfc/tx_common.c
> @@ -87,6 +87,8 @@ void efx_init_tx_queue(struct efx_tx_queue *tx_queue)
>  	tx_queue->completed_timestamp_minor = 0;
>  
>  	tx_queue->old_tx_packets = tx_queue->tx_packets;
> +	tx_queue->old_tso_bursts = tx_queue->tso_bursts;
> +	tx_queue->old_tso_packets = tx_queue->tso_packets;
>  
>  	tx_queue->xdp_tx = efx_channel_is_xdp_tx(tx_queue->channel);
>  	tx_queue->tso_version = 0;
> 

Re: [PATCH net-next 6/6] sfc: add per-queue RX and TX bytes stats

[Jacob Keller]

On 8/28/2024 6:45 AM, edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> While this does add overhead to the fast path, it should be minimal
>  as the cacheline should already be held for write from updating the
>  queue's [tr]x_packets stat.
> 
> Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
> ---

Ah, the broken self tests from the early commit get fixed here. Ok.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/sfc/ef100_rx.c   |  1 +
>  drivers/net/ethernet/sfc/ef100_tx.c   |  1 +
>  drivers/net/ethernet/sfc/efx.c        |  9 +++++++++
>  drivers/net/ethernet/sfc/net_driver.h | 10 ++++++++++
>  drivers/net/ethernet/sfc/rx.c         |  1 +
>  drivers/net/ethernet/sfc/rx_common.c  |  1 +
>  drivers/net/ethernet/sfc/tx.c         |  2 ++
>  drivers/net/ethernet/sfc/tx_common.c  |  1 +
>  8 files changed, 26 insertions(+)
> 
> diff --git a/drivers/net/ethernet/sfc/ef100_rx.c b/drivers/net/ethernet/sfc/ef100_rx.c
> index 992151775cb8..44dc75feb162 100644
> --- a/drivers/net/ethernet/sfc/ef100_rx.c
> +++ b/drivers/net/ethernet/sfc/ef100_rx.c
> @@ -135,6 +135,7 @@ void __ef100_rx_packet(struct efx_channel *channel)
>  	}
>  
>  	++rx_queue->rx_packets;
> +	rx_queue->rx_bytes += rx_buf->len;
>  
>  	efx_rx_packet_gro(channel, rx_buf, channel->rx_pkt_n_frags, eh, csum);
>  	goto out;
> diff --git a/drivers/net/ethernet/sfc/ef100_tx.c b/drivers/net/ethernet/sfc/ef100_tx.c
> index e6b6be549581..a7e30289e231 100644
> --- a/drivers/net/ethernet/sfc/ef100_tx.c
> +++ b/drivers/net/ethernet/sfc/ef100_tx.c
> @@ -493,6 +493,7 @@ int __ef100_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb,
>  	} else {
>  		tx_queue->tx_packets++;
>  	}
> +	tx_queue->tx_bytes += skb->len;
>  	return 0;
>  
>  err:
> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
> index bf06fbcdcbff..b3fbffed4e61 100644
> --- a/drivers/net/ethernet/sfc/efx.c
> +++ b/drivers/net/ethernet/sfc/efx.c
> @@ -638,6 +638,7 @@ static void efx_get_queue_stats_rx(struct net_device *net_dev, int idx,
>  	rx_queue = efx_channel_get_rx_queue(channel);
>  	/* Count only packets since last time datapath was started */
>  	stats->packets = rx_queue->rx_packets - rx_queue->old_rx_packets;
> +	stats->bytes = rx_queue->rx_bytes - rx_queue->old_rx_bytes;
>  	stats->hw_drops = efx_get_queue_stat_rx_hw_drops(channel) -
>  			  channel->old_n_rx_hw_drops;
>  	stats->hw_drop_overruns = channel->n_rx_nodesc_trunc -
> @@ -653,6 +654,7 @@ static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,
>  
>  	channel = efx_get_tx_channel(efx, idx);
>  	stats->packets = 0;
> +	stats->bytes = 0;
>  	stats->hw_gso_packets = 0;
>  	stats->hw_gso_wire_packets = 0;
>  	/* If a TX channel has XDP TXQs, the stats for these will be counted
> @@ -663,6 +665,7 @@ static void efx_get_queue_stats_tx(struct net_device *net_dev, int idx,
>  	 */
>  	efx_for_each_channel_tx_queue(tx_queue, channel) {
>  		stats->packets += tx_queue->tx_packets - tx_queue->old_tx_packets;
> +		stats->bytes += tx_queue->tx_bytes - tx_queue->old_tx_bytes;
>  		stats->hw_gso_packets += tx_queue->tso_bursts -
>  					 tx_queue->old_tso_bursts;
>  		stats->hw_gso_wire_packets += tx_queue->tso_packets -
> @@ -680,9 +683,11 @@ static void efx_get_base_stats(struct net_device *net_dev,
>  	struct efx_channel *channel;
>  
>  	rx->packets = 0;
> +	rx->bytes = 0;
>  	rx->hw_drops = 0;
>  	rx->hw_drop_overruns = 0;
>  	tx->packets = 0;
> +	tx->bytes = 0;
>  	tx->hw_gso_packets = 0;
>  	tx->hw_gso_wire_packets = 0;
>  
> @@ -693,10 +698,12 @@ static void efx_get_base_stats(struct net_device *net_dev,
>  		rx_queue = efx_channel_get_rx_queue(channel);
>  		if (channel->channel >= net_dev->real_num_rx_queues) {
>  			rx->packets += rx_queue->rx_packets;
> +			rx->bytes += rx_queue->rx_bytes;
>  			rx->hw_drops += efx_get_queue_stat_rx_hw_drops(channel);
>  			rx->hw_drop_overruns += channel->n_rx_nodesc_trunc;
>  		} else {
>  			rx->packets += rx_queue->old_rx_packets;
> +			rx->bytes += rx_queue->old_rx_bytes;
>  			rx->hw_drops += channel->old_n_rx_hw_drops;
>  			rx->hw_drop_overruns += channel->old_n_rx_hw_drop_overruns;
>  		}
> @@ -705,10 +712,12 @@ static void efx_get_base_stats(struct net_device *net_dev,
>  			    channel->channel >= efx->tx_channel_offset +
>  						net_dev->real_num_tx_queues) {
>  				tx->packets += tx_queue->tx_packets;
> +				tx->bytes += tx_queue->tx_bytes;
>  				tx->hw_gso_packets += tx_queue->tso_bursts;
>  				tx->hw_gso_wire_packets += tx_queue->tso_packets;
>  			} else {
>  				tx->packets += tx_queue->old_tx_packets;
> +				tx->bytes += tx_queue->old_tx_bytes;
>  				tx->hw_gso_packets += tx_queue->old_tso_bursts;
>  				tx->hw_gso_wire_packets += tx_queue->old_tso_packets;
>  			}
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index 2cf2935a713c..147052c1e25a 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -233,7 +233,11 @@ struct efx_tx_buffer {
>   * @cb_packets: Number of times the TX copybreak feature has been used
>   * @notify_count: Count of notified descriptors to the NIC
>   * @tx_packets: Number of packets sent since this struct was created
> + * @tx_bytes: Number of bytes sent since this struct was created.  For TSO,
> + *	counts the superframe size, not the sizes of generated frames on the
> + *	wire (i.e. the headers are only counted once)
>   * @old_tx_packets: Value of @tx_packets as of last efx_init_tx_queue()
> + * @old_tx_bytes: Value of @tx_bytes as of last efx_init_tx_queue()
>   * @old_tso_bursts: Value of @tso_bursts as of last efx_init_tx_queue()
>   * @old_tso_packets: Value of @tso_packets as of last efx_init_tx_queue()
>   * @empty_read_count: If the completion path has seen the queue as empty
> @@ -285,7 +289,9 @@ struct efx_tx_queue {
>  	unsigned int notify_count;
>  	/* Statistics to supplement MAC stats */
>  	unsigned long tx_packets;
> +	unsigned long tx_bytes;
>  	unsigned long old_tx_packets;
> +	unsigned long old_tx_bytes;
>  	unsigned int old_tso_bursts;
>  	unsigned int old_tso_packets;
>  
> @@ -378,7 +384,9 @@ struct efx_rx_page_state {
>   * @slow_fill: Timer used to defer efx_nic_generate_fill_event().
>   * @grant_work: workitem used to grant credits to the MAE if @grant_credits
>   * @rx_packets: Number of packets received since this struct was created
> + * @rx_bytes: Number of bytes received since this struct was created
>   * @old_rx_packets: Value of @rx_packets as of last efx_init_rx_queue()
> + * @old_rx_bytes: Value of @rx_bytes as of last efx_init_rx_queue()
>   * @xdp_rxq_info: XDP specific RX queue information.
>   * @xdp_rxq_info_valid: Is xdp_rxq_info valid data?.
>   */
> @@ -415,7 +423,9 @@ struct efx_rx_queue {
>  	struct work_struct grant_work;
>  	/* Statistics to supplement MAC stats */
>  	unsigned long rx_packets;
> +	unsigned long rx_bytes;
>  	unsigned long old_rx_packets;
> +	unsigned long old_rx_bytes;
>  	struct xdp_rxq_info xdp_rxq_info;
>  	bool xdp_rxq_info_valid;
>  };
> diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
> index f07495582125..ffca82207e47 100644
> --- a/drivers/net/ethernet/sfc/rx.c
> +++ b/drivers/net/ethernet/sfc/rx.c
> @@ -393,6 +393,7 @@ void __efx_rx_packet(struct efx_channel *channel)
>  	}
>  
>  	rx_queue->rx_packets++;
> +	rx_queue->rx_bytes += rx_buf->len;
>  
>  	if (!efx_do_xdp(efx, channel, rx_buf, &eh))
>  		goto out;
> diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c
> index bdb4325a7c2c..ab358fe13e1d 100644
> --- a/drivers/net/ethernet/sfc/rx_common.c
> +++ b/drivers/net/ethernet/sfc/rx_common.c
> @@ -242,6 +242,7 @@ void efx_init_rx_queue(struct efx_rx_queue *rx_queue)
>  	rx_queue->page_recycle_full = 0;
>  
>  	rx_queue->old_rx_packets = rx_queue->rx_packets;
> +	rx_queue->old_rx_bytes = rx_queue->rx_bytes;
>  
>  	/* Initialise limit fields */
>  	max_fill = efx->rxq_entries - EFX_RXD_HEAD_ROOM;
> diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
> index fe2d476028e7..1aea19488a56 100644
> --- a/drivers/net/ethernet/sfc/tx.c
> +++ b/drivers/net/ethernet/sfc/tx.c
> @@ -394,6 +394,7 @@ netdev_tx_t __efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb
>  	} else {
>  		tx_queue->tx_packets++;
>  	}
> +	tx_queue->tx_bytes += skb_len;
>  
>  	return NETDEV_TX_OK;
>  
> @@ -490,6 +491,7 @@ int efx_xdp_tx_buffers(struct efx_nic *efx, int n, struct xdp_frame **xdpfs,
>  		tx_buffer->dma_offset = 0;
>  		tx_buffer->unmap_len = len;
>  		tx_queue->tx_packets++;
> +		tx_queue->tx_bytes += len;
>  	}
>  
>  	/* Pass mapped frames to hardware. */
> diff --git a/drivers/net/ethernet/sfc/tx_common.c b/drivers/net/ethernet/sfc/tx_common.c
> index cd0857131aa8..7ef2baa3439a 100644
> --- a/drivers/net/ethernet/sfc/tx_common.c
> +++ b/drivers/net/ethernet/sfc/tx_common.c
> @@ -87,6 +87,7 @@ void efx_init_tx_queue(struct efx_tx_queue *tx_queue)
>  	tx_queue->completed_timestamp_minor = 0;
>  
>  	tx_queue->old_tx_packets = tx_queue->tx_packets;
> +	tx_queue->old_tx_bytes = tx_queue->tx_bytes;
>  	tx_queue->old_tso_bursts = tx_queue->tso_bursts;
>  	tx_queue->old_tso_packets = tx_queue->tso_packets;
>  
> 

Re: [PATCH net-next 2/6] sfc: implement basic per-queue stats

[Jakub Kicinski]

On Wed, 28 Aug 2024 14:45:11 +0100 edward.cree@amd.com wrote:
> +	/* If a TX channel has XDP TXQs, the stats for these will be counted
> +	 * under the channel rather than in base stats.  Unclear whether this
> +	 * is correct behaviour, but we can't reliably exclude XDP TXes from
> +	 * these stats anyway because in EFX_XDP_TX_QUEUES_BORROWED we use
> +	 * the same TXQ as the core.
> +	 */
> +	efx_for_each_channel_tx_queue(tx_queue, channel)
> +		stats->packets += tx_queue->tx_packets - tx_queue->old_tx_packets;

We haven't had to deal with shared host/XDP queues in the other
drivers, yet. But talking to Olek about his stats work it sounds
like he's planning to add support for reporting XDP queues. 
At which point it will be relatively intuitive - if XDP queues
are listed - they count XDP packets, if not, and XDP_TX happens
- the queues must be shared and so are the counters.

IOW let's not count dedicated XDP queues here at all, if we can.
XDP traffic on shared queues can get added in.

Re: [PATCH net-next 2/6] sfc: implement basic per-queue stats

[Jakub Kicinski]

On Wed, 28 Aug 2024 15:20:53 -0700 Jacob Keller wrote:
> On 8/28/2024 6:45 AM, edward.cree@amd.com wrote:
> > From: Edward Cree <ecree.xilinx@gmail.com>
> > 
> > Just RX and TX packet counts for now.  We do not have per-queue
> >  byte counts, which causes us to fail stats.pkt_byte_sum selftest
> >  with "Drivers should always report basic keys" error.  
> 
> Seems like the self tests here should be fixed for this?

FWIW - I just didn't take lack of byte counters into account :)
It's probably fine to remove the requirement, imbalance (which
is the main use for the per queue stats) is better tacked using
packets, anyway.

Not a requirement from my perspective to merge the series tho 🤷️

Re: [PATCH net-next 3/6] sfc: add n_rx_overlength to ethtool stats

[Jakub Kicinski]

On Wed, 28 Aug 2024 14:45:12 +0100 edward.cree@amd.com wrote:
> This counter is the main difference between the old and new locations
>  of the rx_packets increment (the other is scatter errors which
>  produce a WARN_ON).  It previously was not reported anywhere; add it
>  to ethtool -S output to ensure users still have this information.

What is it tho? Not IEEE 802.3 30.3.1.1.25 aFrameTooLongErrors ?

Re: [PATCH net-next 6/6] sfc: add per-queue RX and TX bytes stats

[Jakub Kicinski]

On Wed, 28 Aug 2024 15:25:59 -0700 Jacob Keller wrote:
> Ah, the broken self tests from the early commit get fixed here. Ok.

+1 :)

Re: [PATCH net-next 2/6] sfc: implement basic per-queue stats

[Alexander Lobakin]

From: Jakub Kicinski <kuba@kernel.org>
Date: Wed, 28 Aug 2024 17:41:14 -0700

> On Wed, 28 Aug 2024 14:45:11 +0100 edward.cree@amd.com wrote:
>> +	/* If a TX channel has XDP TXQs, the stats for these will be counted
>> +	 * under the channel rather than in base stats.  Unclear whether this
>> +	 * is correct behaviour, but we can't reliably exclude XDP TXes from
>> +	 * these stats anyway because in EFX_XDP_TX_QUEUES_BORROWED we use
>> +	 * the same TXQ as the core.
>> +	 */
>> +	efx_for_each_channel_tx_queue(tx_queue, channel)
>> +		stats->packets += tx_queue->tx_packets - tx_queue->old_tx_packets;
> 
> We haven't had to deal with shared host/XDP queues in the other
> drivers, yet. But talking to Olek about his stats work it sounds
> like he's planning to add support for reporting XDP queues. 
> At which point it will be relatively intuitive - if XDP queues
> are listed - they count XDP packets, if not, and XDP_TX happens
> - the queues must be shared and so are the counters.
> 
> IOW let's not count dedicated XDP queues here at all, if we can.
> XDP traffic on shared queues can get added in.

Yes, I agree. If a queue is shared between XDP and regular traffic,
everything should go to the NL stats. But if the driver allocates
dedicated XDP queues not exposed to the stack (i.e. above
dev->num_tx_queues), they shouldn't count for now.

Thanks,
Olek

Re: [PATCH net-next 3/6] sfc: add n_rx_overlength to ethtool stats

[Edward Cree]

On 29/08/2024 01:47, Jakub Kicinski wrote:
> On Wed, 28 Aug 2024 14:45:12 +0100 edward.cree@amd.com wrote:
>> This counter is the main difference between the old and new locations
>>  of the rx_packets increment (the other is scatter errors which
>>  produce a WARN_ON).  It previously was not reported anywhere; add it
>>  to ethtool -S output to ensure users still have this information.
> 
> What is it tho? Not IEEE 802.3 30.3.1.1.25 aFrameTooLongErrors ?

No, it doesn't appear to be.
If I'm understanding the code correctly, it counts "RX packets which
 SG placed in a single RX buffer but whose length (from the RX event)
 is too big to fit in that RX buffer".  Which doesn't sound like a
 thing that should ever happen (and when it does we netif_err() under
 ratelimit, see efx_rx_packet__check_len()).
I'll put this into the commit message.

On 28/08/2024 23:22, Jacob Keller wrote:
> The description makes sense in context with the whole series but doesn't
> quite work for me if I think about viewing it without context. Perhaps a
> little more clarification about the rx_packets behavioral change?

Sure, will do.

Re: [PATCH net-next 2/6] sfc: implement basic per-queue stats

[Edward Cree]

On 29/08/2024 01:41, Jakub Kicinski wrote:
> IOW let's not count dedicated XDP queues here at all, if we can.

But they should still go in base_stats, like other not-core
 queues, right?

Re: [PATCH net-next 2/6] sfc: implement basic per-queue stats

[Jakub Kicinski]

On Thu, 5 Sep 2024 11:57:59 +0100 Edward Cree wrote:
> On 29/08/2024 01:41, Jakub Kicinski wrote:
> > IOW let's not count dedicated XDP queues here at all, if we can.  
> 
> But they should still go in base_stats, like other not-core
>  queues, right?

Yeah, I think so. I'm not 100% confident, but I think it makes sense
for tx-packets / tx-bytes to be inclusive of all more granular counters
(IOW even if we report xdp-tx-packets, the main tx-packets should also
count those frames).