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

Message ID 20240510100358.18815-3-ciara.loftus@intel.com (mailing list archive)
State Superseded
Delegated to: Ferruh Yigit
Headers
Series [1/3] net/af_xdp: Fix port id not set in rx mbuf |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-abi-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing warning Testing issues
ci/intel-Functional success Functional PASS
ci/intel-Testing success Testing PASS

Commit Message

Loftus, Ciara May 10, 2024, 10:03 a.m. UTC
  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(-)
  

Comments

Maryam Tahhan May 10, 2024, 12:36 p.m. UTC | #1
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;
  

Patch

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;
 };
 
 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;