Re: [PATCH 3/3] net/af_xdp: Fix stats reset

[Maryam Tahhan <mtahhan@redhat.com>]

On 10/05/2024 11:03, Ciara Loftus wrote:
> The imissed statistic was not properly reset because it was
> read directly from the kernel statistics. To fix this, take note
> of the kernel statistic when the stats are reset and deduct this
> value from the kernel statistic read during statistics get.
>
> Bugzilla ID: 1430
> Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
> cc: stable@dpdk.og
>
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
>   drivers/net/af_xdp/rte_eth_af_xdp.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 968bbf6d45..8f25134003 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -125,6 +125,8 @@ struct rx_stats {
>   	uint64_t rx_bytes;
>   	uint64_t rx_dropped;
>   	uint64_t alloc_failed;
> +
> +	uint64_t imissed_offset;
>   };


Minor nit (sorry) probably don't need the empty line between the 
`uint64_t alloc_failed` and `uint64_t imissed_offset`

otherwise LGTM

Acked-by: Maryam Tahhan <mtahhan@redhat.com>

>   
>   struct pkt_rx_queue {
> @@ -884,7 +886,8 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>   			AF_XDP_LOG(ERR, "getsockopt() failed for XDP_STATISTICS.\n");
>   			return -1;
>   		}
> -		stats->imissed += xdp_stats.rx_dropped;
> +		stats->imissed +=
> +			(xdp_stats.rx_dropped - internals->rx_queues[i].stats.imissed_offset);
>   
>   		stats->opackets += stats->q_opackets[i];
>   		stats->obytes += stats->q_obytes[i];
> @@ -897,13 +900,25 @@ static int
>   eth_stats_reset(struct rte_eth_dev *dev)
>   {
>   	struct pmd_internals *internals = dev->data->dev_private;
> -	int i;
> +	struct pmd_process_private *process_private = dev->process_private;
> +	struct xdp_statistics xdp_stats;
> +	socklen_t optlen;
> +	int i, ret, fd;
>   
>   	for (i = 0; i < internals->queue_cnt; i++) {
>   		memset(&internals->rx_queues[i].stats, 0,
>   					sizeof(struct rx_stats));
>   		memset(&internals->tx_queues[i].stats, 0,
>   					sizeof(struct tx_stats));
> +		fd = process_private->rxq_xsk_fds[i];
> +		optlen = sizeof(struct xdp_statistics);
> +		ret = fd >= 0 ? getsockopt(fd, SOL_XDP, XDP_STATISTICS,
> +					   &xdp_stats, &optlen) : -1;
> +		if (ret != 0) {
> +			AF_XDP_LOG(ERR, "getsockopt() failed for XDP_STATISTICS.\n");
> +			return -1;
> +		}
> +		internals->rx_queues[i].stats.imissed_offset = xdp_stats.rx_dropped;
>   	}
>   
>   	return 0;