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

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 16 Jan 2025 17:17:03 +0100
Stefan Laesser <stefan.laesser@omicronenergy.com> wrote:

> The Linux kernel provides the ability to query the packet drop counter
> of a socket. This information can be provided when the user requests
> stats.
> 
> It is important to note that each call to getsockopt with
> PACKET_STATISTICS resets the internal counters. So the caller needs to
> keep track of the total count on its own.
> 
> Next, I have added a counter for the case when mbuf couldn't be
> allocated.
> 
> Signed-off-by: Stefan Laesser <stefan.laesser@omicronenergy.com>
> ---
>  drivers/net/af_packet/rte_eth_af_packet.c | 32 +++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
> index ceb8d9356a..a771dd854d 100644
> --- a/drivers/net/af_packet/rte_eth_af_packet.c
> +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> @@ -58,6 +58,8 @@ struct __rte_cache_aligned pkt_rx_queue {
>  
>  	volatile unsigned long rx_pkts;
>  	volatile unsigned long rx_bytes;
> +	volatile unsigned long rx_nombuf;
> +	volatile unsigned long rx_dropped_pkts;
>  };
>  
>  struct __rte_cache_aligned pkt_tx_queue {
> @@ -145,8 +147,10 @@ eth_af_packet_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  
>  		/* allocate the next mbuf */
>  		mbuf = rte_pktmbuf_alloc(pkt_q->mb_pool);
> -		if (unlikely(mbuf == NULL))
> +		if (unlikely(mbuf == NULL)) {
> +			pkt_q->rx_nombuf++;
>  			break;
> +		}
>  
>  		/* packet will fit in the mbuf, go ahead and receive it */
>  		rte_pktmbuf_pkt_len(mbuf) = rte_pktmbuf_data_len(mbuf) = ppd->tp_snaplen;
> @@ -417,17 +421,37 @@ static int
>  eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
>  {
>  	unsigned i, imax;
> -	unsigned long rx_total = 0, tx_total = 0, tx_err_total = 0;
> +	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;
>  
> +	struct tpacket_stats iface_stats;
> +	socklen_t iface_stats_len = sizeof(struct tpacket_stats);

This declaration could be moved inside the loop.
> +
>  	imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ?
>  	        internal->nb_queues : RTE_ETHDEV_QUEUE_STAT_CNTRS);
>  	for (i = 0; i < imax; i++) {
> +		/* 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;
> +		}
> +
>  		igb_stats->q_ipackets[i] = internal->rx_queue[i].rx_pkts;
>  		igb_stats->q_ibytes[i] = internal->rx_queue[i].rx_bytes;
> +		igb_stats->q_errors[i] = internal->rx_queue[i].rx_dropped_pkts;

Dropped packets are not errors; at least most other drivers do not report missed
packets as errors. Should be imissed statistic.

> +
>  		rx_total += igb_stats->q_ipackets[i];
>  		rx_bytes_total += igb_stats->q_ibytes[i];
> +		rx_dropped_total += igb_stats->q_errors[i];
> +		rx_nombuf_total += internal->rx_queue[i].rx_nombuf;
>  	}
>  
>  	imax = (internal->nb_queues < RTE_ETHDEV_QUEUE_STAT_CNTRS ?
> @@ -442,6 +466,8 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *igb_stats)
>  
>  	igb_stats->ipackets = rx_total;
>  	igb_stats->ibytes = rx_bytes_total;
> +	igb_stats->imissed = rx_dropped_total;
> +	igb_stats->rx_nombuf = rx_nombuf_total;
>  	igb_stats->opackets = tx_total;
>  	igb_stats->oerrors = tx_err_total;
>  	igb_stats->obytes = tx_bytes_total;
> @@ -457,6 +483,8 @@ eth_stats_reset(struct rte_eth_dev *dev)
>  	for (i = 0; i < internal->nb_queues; i++) {
>  		internal->rx_queue[i].rx_pkts = 0;
>  		internal->rx_queue[i].rx_bytes = 0;
> +		internal->rx_queue[i].rx_nombuf = 0;
> +		internal->rx_queue[i].rx_dropped_pkts = 0;
>  	}
>  
>  	for (i = 0; i < internal->nb_queues; i++) {

What about stats reset?