Re: [PATCH v2] net/af_packet: provide packet drop stats

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 21 Jan 2025 13:26:16 +0100
Stefan Laesser <stefan.laesser@omicronenergy.com> wrote:

> +		/* query dropped packets counter from socket */
> +		if (internal->rx_queue[i].sockfd != -1 &&
> +			getsockopt(internal->rx_queue[i].sockfd, SOL_PACKET,
> +						PACKET_STATISTICS, &iface_stats,
> +						&iface_stats_len) > -1) {
> +			/*
> +			 * keep total because each call to getsocketopt with PACKET_STATISTICS
> +			 * reset the counter of the socket
> +			 */
> +			internal->rx_queue[i].rx_dropped_pkts += iface_stats.tp_drops;
> +		}

Since reading the value clears it, maybe stats_reset should call as well?

Also, queues greater that the RTE_ETHDEV_STAT_CNTRS should still count
against the total. And the two loops could be combined.

Maybe something like this?

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 7033910df8..af08e893f1 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -417,50 +417,51 @@ eth_dev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+/* query dropped packets counter from socket */
+static unsigned int
+packet_drop_count(int sockfd)
+{
+	struct tpacket_stats pkt_stats;
+	socklen_t pkt_stats_len = sizeof(struct tpacket_stats);
+
+	if (sockfd == -1)
+		return 0;
+
+	if (getsockopt(sockfd, SOL_PACKET, PACKET_STATISTICS, &pkt_stats, &pkt_stats_len) < -1)
+		return 0;
+
+	return pkt_stats.tp_drops;
+}
+
 static int
 eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
-	unsigned i, imax;
+	unsigned int i;
 	unsigned long rx_total = 0, rx_dropped_total = 0, rx_nombuf_total = 0;
 	unsigned long tx_total = 0, tx_err_total = 0;
 	unsigned long rx_bytes_total = 0, tx_bytes_total = 0;
 	const struct pmd_internals *internal = dev->data->dev_private;
 
-	imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ?
-	        internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS);
-	for (i = 0; i < imax; i++) {
-		struct tpacket_stats iface_stats;
-		socklen_t iface_stats_len = sizeof(struct tpacket_stats);
-
-		/* query dropped packets counter from socket */
-		if (internal->rx_queue[i].sockfd != -1 &&
-			getsockopt(internal->rx_queue[i].sockfd, SOL_PACKET,
-						PACKET_STATISTICS, &iface_stats,
-						&iface_stats_len) > -1) {
-			/*
-			 * keep total because each call to getsocketopt with PACKET_STATISTICS
-			 * reset the counter of the socket
-			 */
-			internal->rx_queue[i].rx_dropped_pkts += iface_stats.tp_drops;
-		}
-
-		stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
-		stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
+	for (i = 0; i < internal->nb_queues; i++) {
+		/* reading drop count clears the value */
+		internal->rx_queue[i].rx_dropped_pkts +=
+			packet_drop_count(internal->rx_queue[i].sockfd);
 
 		rx_total += stats->q_ipackets[i];
 		rx_bytes_total += stats->q_ibytes[i];
 		rx_dropped_total += internal->rx_queue[i].rx_dropped_pkts;
 		rx_nombuf_total += internal->rx_queue[i].rx_nombuf;
-	}
 
-	imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ?
-	        internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS);
-	for (i = 0; i < imax; i++) {
-		stats->q_opackets[i] = internal->tx_queue[i].tx_pkts;
-		stats->q_obytes[i] = internal->tx_queue[i].tx_bytes;
 		tx_total += stats->q_opackets[i];
 		tx_err_total += internal->tx_queue[i].err_pkts;
 		tx_bytes_total += stats->q_obytes[i];
+
+		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+			stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
+			stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
+			stats->q_opackets[i] = internal->tx_queue[i].tx_pkts;
+			stats->q_obytes[i] = internal->tx_queue[i].tx_bytes;
+		}
 	}
 
 	stats->ipackets = rx_total;
@@ -480,6 +481,7 @@ eth_stats_reset(struct rte_eth_dev *dev)
 	struct pmd_internals *internal = dev->data->dev_private;
 
 	for (i = 0; i < internal->nb_queues; i++) {
+		packet_drop_count(internal->rx_queue[i].sockfd);
 		internal->rx_queue[i].rx_pkts = 0;
 		internal->rx_queue[i].rx_bytes = 0;
 		internal->rx_queue[i].rx_nombuf = 0;