RE: [PATCH v5 2/9] ethdev: add common counters for statistics

2024-05-16 Thread Wathsala Wathawana Vithanage
> + packets = rte_counter64_fetch(&counters->packets);
> + bytes = rte_counter64_fetch(&counters->bytes);
> + errors = rte_counter64_fetch(&counters->errors);
> +
> + rte_compiler_barrier();
> +
> + stats->ipackets += packets;
> + stats->ibytes += bytes;
> + stats->ierrors += errors;
> +

there seems to be a dependency chain in the above loads and subsequent stores.
If that's the case what's the purpose of the compiler barrier?

--wathsala


[PATCH v5 2/9] ethdev: add common counters for statistics

2024-05-16 Thread Stephen Hemminger
Introduce common helper routines for keeping track of per-queue
statistics in SW PMD's. The code in several drivers had
copy/pasted the same code for this, but had common issues
with 64 bit counters on 32 bit platforms.

Signed-off-by: Stephen Hemminger 
---
 lib/ethdev/ethdev_swstats.c | 109 +++
 lib/ethdev/ethdev_swstats.h | 124 
 lib/ethdev/meson.build  |   2 +
 lib/ethdev/version.map  |   3 +
 4 files changed, 238 insertions(+)
 create mode 100644 lib/ethdev/ethdev_swstats.c
 create mode 100644 lib/ethdev/ethdev_swstats.h

diff --git a/lib/ethdev/ethdev_swstats.c b/lib/ethdev/ethdev_swstats.c
new file mode 100644
index 00..7892b2180b
--- /dev/null
+++ b/lib/ethdev/ethdev_swstats.c
@@ -0,0 +1,109 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) Stephen Hemminger 
+ */
+
+
+#include 
+#include 
+#include 
+
+#include "rte_ethdev.h"
+#include "ethdev_driver.h"
+#include "ethdev_swstats.h"
+
+int
+rte_eth_counters_stats_get(const struct rte_eth_dev *dev,
+  size_t tx_offset, size_t rx_offset,
+  struct rte_eth_stats *stats)
+{
+   uint64_t packets, bytes, errors;
+   unsigned int i;
+
+   for (i = 0; i < dev->data->nb_tx_queues; i++) {
+   const void *txq = dev->data->tx_queues[i];
+   const struct rte_eth_counters *counters;
+
+   if (txq == NULL)
+   continue;
+
+   counters = (const struct rte_eth_counters *)((const char *)txq 
+ tx_offset);
+   packets = rte_counter64_fetch(&counters->packets);
+   bytes = rte_counter64_fetch(&counters->bytes);
+   errors = rte_counter64_fetch(&counters->errors);
+
+   /*
+* Prevent compiler from fetching values twice which would
+* cause per-queue and global statistics to not match.
+*/
+   rte_compiler_barrier();
+
+   stats->opackets += packets;
+   stats->obytes += bytes;
+   stats->oerrors += errors;
+
+   if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+   stats->q_opackets[i] = packets;
+   stats->q_obytes[i] = bytes;
+   }
+   }
+
+   for (i = 0; i < dev->data->nb_rx_queues; i++) {
+   const void *rxq = dev->data->rx_queues[i];
+   const struct rte_eth_counters *counters;
+
+   if (rxq == NULL)
+   continue;
+
+   counters = (const struct rte_eth_counters *)((const char *)rxq 
+ rx_offset);
+   packets = rte_counter64_fetch(&counters->packets);
+   bytes = rte_counter64_fetch(&counters->bytes);
+   errors = rte_counter64_fetch(&counters->errors);
+
+   rte_compiler_barrier();
+
+   stats->ipackets += packets;
+   stats->ibytes += bytes;
+   stats->ierrors += errors;
+
+   if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+   stats->q_ipackets[i] = packets;
+   stats->q_ibytes[i] = bytes;
+   }
+   }
+
+   stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed;
+   return 0;
+}
+
+int
+rte_eth_counters_reset(struct rte_eth_dev *dev, size_t tx_offset, size_t 
rx_offset)
+{
+   struct rte_eth_counters *counters;
+   unsigned int i;
+
+   for (i = 0; i < dev->data->nb_tx_queues; i++) {
+   void *txq  = dev->data->tx_queues[i];
+
+   if (txq == NULL)
+   continue;
+
+   counters = (struct rte_eth_counters *)((char *)txq + tx_offset);
+   rte_counter64_reset(&counters->packets);
+   rte_counter64_reset(&counters->bytes);
+   rte_counter64_reset(&counters->errors);
+   }
+
+   for (i = 0; i < dev->data->nb_rx_queues; i++) {
+   void *rxq  = dev->data->rx_queues[i];
+
+   if (rxq == NULL)
+   continue;
+
+   counters = (struct rte_eth_counters *)((char *)rxq + rx_offset);
+   rte_counter64_reset(&counters->packets);
+   rte_counter64_reset(&counters->bytes);
+   rte_counter64_reset(&counters->errors);
+   }
+
+   return 0;
+}
diff --git a/lib/ethdev/ethdev_swstats.h b/lib/ethdev/ethdev_swstats.h
new file mode 100644
index 00..808c540640
--- /dev/null
+++ b/lib/ethdev/ethdev_swstats.h
@@ -0,0 +1,124 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) Stephen Hemminger 
+ */
+
+#ifndef _RTE_ETHDEV_SWSTATS_H_
+#define _RTE_ETHDEV_SWSTATS_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @file
+ *
+ * Internal statistics counters for software based devices.
+ * Hardware PMD's should use the hardware counters instead.
+ *
+ * This provides a library for PMD's to keep track of packets and bytes.
+ * It is assum