[v2,3/4] net/af_xdp: fix stats reset
Checks
Commit Message
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.org
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
Acked-by: Maryam Tahhan <mtahhan@redhat.com>
---
v2:
* Removed whitespace in rx_stats struct
* Fixed typo in commit message
drivers/net/af_xdp/rte_eth_af_xdp.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
Comments
On 5/14/2024 9:41 AM, 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.org
>
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> Acked-by: Maryam Tahhan <mtahhan@redhat.com>
> ---
> v2:
> * Removed whitespace in rx_stats struct
> * Fixed typo in commit message
>
> drivers/net/af_xdp/rte_eth_af_xdp.c | 18 ++++++++++++++++--
> 1 file changed, 16 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 9bcf971ae5..193e3576bc 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -124,6 +124,7 @@ struct rx_stats {
> uint64_t rx_pkts;
> uint64_t rx_bytes;
> uint64_t rx_dropped;
> + uint64_t imissed_offset;
> };
>
> struct pkt_rx_queue {
> @@ -884,7 +885,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);
>
why not use 'rxq', like "rxq->stats.imissed_offset"?
I can update while merging if you are OK.
@@ -124,6 +124,7 @@ struct rx_stats {
uint64_t rx_pkts;
uint64_t rx_bytes;
uint64_t rx_dropped;
+ uint64_t imissed_offset;
};
struct pkt_rx_queue {
@@ -884,7 +885,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 +899,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;