[PATCH 0/1] net/ena: adding a histogram framework to 2.11.0

[PATCH 0/1] net/ena: adding a histogram framework to 2.11.0

[shaibran]

From: Shai Brandes <shaibran@amazon.com>

This update is an enhancement to the ENA driver version 2.11.0 release.
It is a "nice to have" feature for the 24.11 release, should it be
included before the code freeze deadline.

This addition leverages an internal framework used in our testing
processes, which may offer significant benefits for customers.
It calculates the differences between specified start and end
points in the code, generating a histogram of time intervals
to provide detailed insights into performance.

Shai Brandes (1):
  net/ena: add histogram support

 doc/guides/nics/ena.rst                | 130 ++++++++++++++
 doc/guides/rel_notes/release_24_11.rst |   2 +
 drivers/net/ena/ena_histogram.h        | 235 +++++++++++++++++++++++++
 3 files changed, 367 insertions(+)
 create mode 100644 drivers/net/ena/ena_histogram.h

-- 
2.17.1

[PATCH 1/1] net/ena: add histogram support

[shaibran]

From: Shai Brandes <shaibran@amazon.com>

The framework computes the differences between specified
start and end points in the code, providing insights into
performance by outputting a histogram of the time intervals.
The histogram can be used to track metrics like packet
processing latency in the network driver.
This framework requires enablement via a compilation
flag and is excluded in release builds by default.

Signed-off-by: Amit Bernstein <amitbern@amazon.com>
Signed-off-by: Shai Brandes <shaibran@amazon.com>
---
 doc/guides/nics/ena.rst                | 130 ++++++++++++++
 doc/guides/rel_notes/release_24_11.rst |   2 +
 drivers/net/ena/ena_histogram.h        | 228 +++++++++++++++++++++++++
 3 files changed, 360 insertions(+)
 create mode 100644 drivers/net/ena/ena_histogram.h

diff --git a/doc/guides/nics/ena.rst b/doc/guides/nics/ena.rst
index a40f09be09..fee4a27ffb 100644
--- a/doc/guides/nics/ena.rst
+++ b/doc/guides/nics/ena.rst
@@ -305,3 +305,133 @@ Example output:
 
    Done
    testpmd>
+
+ENA Histogram Support
+=====================
+
+This framework computes the differences between specified start and end points in the code, providing insights into performance by outputting a histogram of the time intervals. The histogram can be used to track metrics like packet processing latency in network drivers.
+
+
+Use Case
+--------
+
+The histogram framework is helpful for performance monitoring by capturing and analyzing intervals between events. For instance, in network packet processing, it can measure the time spent processing packets and present a distribution of processing latencies. By dumping the histogram, developers can analyze these intervals and optimize system performance.
+
+
+Overall Flow
+------------
+
+* Include the histogram header file:
+    * In the relevant source file, conditionally include the histogram functionality by defining INCLUDE_ENA_HISTOGRAM:
+
+        #ifdef INCLUDE_ENA_HISTOGRAM
+
+        #include "ena_histogram.h"
+
+        #endif
+
+* Add the INCLUDE_ENA_HISTOGRAM definition to the Meson build system:
+   * In your build configuration (e.g., meson.build), add the following flag to enable histogram support:
+
+        flags += '-DINCLUDE_ENA_HISTOGRAM'
+
+* Define and initialize the histogram:
+    * Use ENA_HISTOGRAM to declare the histogram structure.
+    * Use ENA_HISTOGRAM_INIT to initialize the histogram.
+* Capture start and stop points:
+    * Mark the start of a capture using ENA_HISTOGRAM_CAPTURE_START.
+    * Mark the stop point using ENA_HISTOGRAM_CAPTURE_STOP.
+* Reset the histogram:
+    * Clear or reset the histogram data at any point using ENA_HISTOGRAM_RESET.
+* Dump the histogram:
+    * Dump all captured data using ENA_HISTOGRAM_DUMP.
+
+
+API
+---
+
+**Histogram Struct Definition:** ENA_HISTOGRAM(name, bins_count)
+
+* What it does: This macro defines the histogram structure.
+* Parameters:
+* name: The name of the histogram (used to identify the specific histogram).
+    * bins_count: The number of bins (or slots) in the histogram. Bins are used to group data into ranges, max = UINT16_MAX (65536)
+* Purpose: This defines the histogram's metadata, particularly how many bins will be used to collect data. The histogram will store data in these bins based on defined ranges or intervals.
+     * Histogram structure doesn't include any lock to avoid affecting the flow, make sure the flow is protected.
+
+
+**Initialize Histogram:** ENA_HISTOGRAM_INIT(histogram, histogram_id)
+
+* What it does: This macro initializes the histogram.
+* Parameters:
+    * histogram: The histogram variable defined in ENA_HISTOGRAM
+    * histogram_id: a unique ID of the histogram, will be used for histogram dump
+* Purpose: This prepares the histogram for capturing data by setting up necessary parameters and getting it ready for use.
+
+
+**Capture Starting Point:** ENA_HISTOGRAM_CAPTURE_START(histogram, capture)
+
+* What it does: Marks the starting point of data capture for the histogram.
+* Parameters:
+    * histogram: The histogram variable defined in ENA_HISTOGRAM
+    * capture: Starting capture value
+* Purpose: This function begins measuring at a specific capture point, which will later be compared to the stop point in order to calculate the difference and update the appropriate histogram bin.
+    * Start capture following start capture will increment histogram error statistic
+
+
+**Capture Ending Point:** ENA_HISTOGRAM_CAPTURE_STOP(histogram, capture, rate)
+
+* What it does: Marks the ending point of data capture for the histogram.
+* Parameters:
+    * histogram: The histogram variable defined in ENA_HISTOGRAM
+    * capture: Ending capture point
+    * rate: frequency of captured events
+* Purpose: This completes the capture process. It calculates the difference between the start and stop points and stores the result in the appropriate histogram bin
+    * Histogram bin is chosen according to the start/stop capture difference and divided by rate
+    * Histogram is incremented by rate.
+    * Zero rate doesn't change the histogram
+    * Stop capture not following start capture will increment histogram error statistic
+    * Last bin also consist all bins above
+
+
+**Reset Histogram Statistics:** ENA_HISTOGRAM_RESET(histogram)
+
+* What it does: Resets the histogram statistics.
+* Parameters:
+    * histogram: The histogram variable defined in ENA_HISTOGRAM
+* Purpose: Resets all stored statistics and errors. This might be used after dumping the data or when starting a new capture session.
+
+
+**Dump Histogram:** ENA_HISTOGRAM_DUMP(histogram, percent)
+
+* What it does: Outputs the data from the histogram.
+* Parameters:
+    * histogram: The histogram variable defined in ENA_HISTOGRAM
+    * percent: A percentage threshold (float), dump histogram above this value
+* Purpose: This outputs all histogram bins with its rate.
+
+
+
+Example Usage
+-------------
+
+* **Define and initialize the histogram:**
+
+   `ENA_HISTOGRAM(my_histogram, 8);`
+   `ENA_HISTOGRAM_INIT(&my_histogram, 1);`
+
+* **Capture an event:**
+
+   `uint64_t start_time = rte_rdtsc();  // Capture start time`
+   `ENA_HISTOGRAM_CAPTURE_START(&my_histogram, start_time); // add start time to my_histogram`
+
+
+   `// Process something...`
+
+   `uint64_t end_time = rte_rdtsc();  // Capture end time`
+   `ENA_HISTOGRAM_CAPTURE_STOP(&my_histogram, end_time, 1); // add 1 captured event with its captured end time to my_histogram`
+
+
+* **Dump the histogram:**
+
+   `ENA_HISTOGRAM_DUMP(&my_histogram, 1);  // Dump all bins with more than 1%`
\ No newline at end of file
diff --git a/doc/guides/rel_notes/release_24_11.rst b/doc/guides/rel_notes/release_24_11.rst
index fa4822d928..7be10e60fd 100644
--- a/doc/guides/rel_notes/release_24_11.rst
+++ b/doc/guides/rel_notes/release_24_11.rst
@@ -142,6 +142,8 @@ New Features
   * Modified the PMD API that controls the LLQ header policy.
   * Replaced ``enable_llq``, ``normal_llq_hdr`` and ``large_llq_hdr`` devargs
     with a new shared devarg ``llq_policy`` that keeps the same logic.
+  * Added a framework that computes the differences between specified start and end points in the code, generating a histogram of the intervals.
+    This framework requires enablement via a compilation flag and is excluded in release builds by default.
 
 * **Updated Cisco enic driver.**
 
diff --git a/drivers/net/ena/ena_histogram.h b/drivers/net/ena/ena_histogram.h
new file mode 100644
index 0000000000..306ff5f043
--- /dev/null
+++ b/drivers/net/ena/ena_histogram.h
@@ -0,0 +1,228 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
+ * All rights reserved.
+ */
+
+#ifndef _ENA_HISTOGRAM_H_
+#define _ENA_HISTOGRAM_H_
+
+#include "ena_ethdev.h"
+#include "ena_logs.h"
+
+#ifdef INCLUDE_ENA_HISTOGRAM
+
+/* ena_histogram_info struct size should use a single cache line (64 bytes) */
+#define ENA_HISTOGRAM_MAX_NAME 46
+
+struct ena_histogram_info {
+	char name[ENA_HISTOGRAM_MAX_NAME];
+	uint16_t bin_count;
+	uint64_t last_capture;
+	uint64_t errors;
+};
+
+struct ena_histogram {
+	struct ena_histogram_info info;
+	uint64_t bins[];
+};
+
+/**
+ * Histogram percentle calculation internal util
+ *
+ * @histogram: pointer to histogram variable
+ * @samples: total samples count
+ * @percentile: requested percentile [0-100]
+ */
+static inline int16_t percentile(struct ena_histogram *histogram,
+				 uint64_t samples,
+				 uint8_t percentile)
+{
+	uint64_t cumulative = 0;
+	uint64_t target = 0;
+
+	/* Invalid input */
+	if (percentile > 100)
+		return -1;
+
+	if (samples <= 0)
+		return 0;
+
+	/* Determine the target count for the given percentile */
+	target = RTE_MAX((samples * percentile) / 100, 1ULL);
+
+	/* Find the bin that corresponds to the target percentile */
+	for (uint16_t bin = 0; bin < histogram->info.bin_count; bin++) {
+		cumulative += histogram->bins[bin];
+		if (cumulative >= target)
+			return bin;
+	}
+
+	return -1;
+}
+
+/**
+ * Histogram struct definition
+ *
+ * @name: name of histogram variable
+ * @bin_count: Numbers of bins - max UINT16_MAX (65536)
+ *             For example, histogram with 8 bins will consist of the following bins boundaries:
+ *             [0],[1],[2],[3],[4],[5],[6],[7-UINT16_MAX]
+ */
+#define ENA_HISTOGRAM(name, bin_count)								\
+	struct ena_histogram_##name {								\
+		struct ena_histogram_info info;							\
+		uint64_t bins[bin_count];							\
+	} name
+
+/**
+ * Initialize histogram
+ *
+ * @histogram: An uninitialized histogram
+ * @histogram_name: Name of the histogram, will be used for histogram dump
+ * @histogram_id: Unique ID of the histogram, will be used for histogram dump
+ */
+#define ENA_HISTOGRAM_INIT(histogram, histogram_name, histogram_id)				\
+	do {											\
+		typeof(histogram) _histogram = (histogram);					\
+		(_histogram)->info.last_capture = 0;						\
+		(_histogram)->info.errors = 0;							\
+		(_histogram)->info.bin_count = ARRAY_SIZE((_histogram)->bins);			\
+		snprintf((_histogram)->info.name, sizeof((_histogram)->info.name), "%s_%u",	\
+			 (histogram_name), (histogram_id));					\
+												\
+		PMD_DRV_LOG(NOTICE, "%s: Bin count = %u, Total size = %" PRIu64 " (bytes)\n",	\
+			    (_histogram)->info.name,						\
+			    (_histogram)->info.bin_count,					\
+			    sizeof(*_histogram));						\
+	} while (0)
+
+/**
+ * Capture starting point
+ *
+ * @histogram: pointer to histogram variable
+ * @capture: start capture value
+ *
+ * Notice:
+ * 1. Start capture following start capture will increment the error statistic
+ * 2. Histogram structure doesn't include any lock to avoid affecting the flow,
+ *    make sure the flow is protected
+ */
+#define ENA_HISTOGRAM_CAPTURE_START(histogram, capture)						\
+	do {											\
+		typeof(histogram) _histogram = (histogram);					\
+		if ((_histogram)->info.last_capture)						\
+			(_histogram)->info.errors++;						\
+		(_histogram)->info.last_capture = (capture);					\
+	} while (0)
+
+/**
+ * Capture ending point
+ * Calculate the diff between start and end point
+ * last bin also consist all bins above
+ * In case start capture value is NULL increment error statistic
+ *
+ * @histogram: pointer to histogram variable
+ * @capture: stop capture value
+ * @rate: frequency of captured events
+ *
+ * Notice:
+ * 1. Stop capture not following start capture will increment the error statistic
+ * 2. histogram structure doesn't include any lock to avoid affecting the flow,
+ *    make sure the flow is protected
+ * 3. Final histogram bin is divided by the rate and increased by rate,
+ *    zero rate doesn't change the histogram
+ */
+#define ENA_HISTOGRAM_CAPTURE_STOP(histogram, capture, rate)					\
+	do {											\
+		typeof(histogram) _histogram = (histogram);					\
+		typeof(rate) _rate = (rate);							\
+		if ((_rate) > 0) {								\
+			uint16_t bin_count = (_histogram)->info.bin_count;			\
+												\
+			if ((_histogram)->info.last_capture) {					\
+				uint16_t bin = (((capture) - (_histogram)->info.last_capture)	\
+						/ (_rate));					\
+												\
+				if (bin >= bin_count)						\
+					bin = bin_count - 1;					\
+												\
+				(_histogram)->bins[bin] += (_rate);				\
+			} else {								\
+				(_histogram)->info.errors++;					\
+			}									\
+		}										\
+		(_histogram)->info.last_capture = 0;						\
+	} while (0)
+
+/**
+ * Reset histogram statistics
+ *
+ * @histogram: pointer to histogram variable
+ */
+#define ENA_HISTOGRAM_RESET(histogram)								\
+	do {											\
+		typeof(histogram) _histogram = (histogram);					\
+		memset((_histogram)->bins, 0, sizeof((_histogram)->bins));			\
+		(_histogram)->info.last_capture = 0;						\
+		(_histogram)->info.errors = 0;							\
+	} while (0)
+
+/**
+ * Dump histogram
+ * Print all histogram cells above percent parameter
+ * Calculate P0/P50/P99/Avg percentile and print error statistics
+ *
+ * @histogram: pointer to histogram variable
+ * @percent: dump histogram above this percent (float)
+ */
+#define ENA_HISTOGRAM_DUMP(histogram, percent)							\
+	do {											\
+		typeof(histogram) _histogram = (histogram);					\
+		typeof(percent) _percent = (percent);						\
+		uint16_t bin_count = (_histogram)->info.bin_count;				\
+		uint64_t samples = 0;								\
+		uint64_t sum = 0;								\
+		float percentage;								\
+												\
+		for (uint16_t bin = 0; bin < bin_count; bin++) {				\
+			if ((_histogram)->bins[bin]) {						\
+				samples += (_histogram)->bins[bin];				\
+				sum += bin * (_histogram)->bins[bin];				\
+			}									\
+		}										\
+												\
+		PMD_DRV_LOG(NOTICE,								\
+			    "%s: Samples[%" PRIu64 "], P0[%d], P50[%d], P99[%d], P100[%d], "	\
+			    "AVG[%" PRIu64 "], Errors[%" PRIu64 "]\n",				\
+			    (_histogram)->info.name,						\
+			    samples,								\
+			    percentile((struct ena_histogram *)(_histogram), samples, 0),	\
+			    percentile((struct ena_histogram *)(_histogram), samples, 50),	\
+			    percentile((struct ena_histogram *)(_histogram), samples, 99),	\
+			    percentile((struct ena_histogram *)(_histogram), samples, 100),	\
+			    (sum / samples),							\
+			    (_histogram)->info.errors);						\
+												\
+		if (samples) {									\
+			for (uint16_t bin = 0; bin < bin_count; bin++) {			\
+				percentage = 100 * ((float)(_histogram)->bins[bin]) / samples;	\
+				if (percentage <= (_percent))					\
+					continue;						\
+												\
+				PMD_DRV_LOG(NOTICE, "%s: bin[%u] = %" PRIu64 ", %.2f%%\n",	\
+					    (_histogram)->info.name,				\
+					    bin,						\
+					    (_histogram)->bins[bin], percentage);		\
+			}									\
+		}										\
+	} while (0)
+
+#else
+#define ENA_HISTOGRAM(name, bins_count)
+#define ENA_HISTOGRAM_INIT(histogram, histogram_id)
+#define ENA_HISTOGRAM_CAPTURE_START(hist_name, value)
+#define ENA_HISTOGRAM_CAPTURE_STOP(hist_name, value, rate)
+#define ENA_HISTOGRAM_RESET(histogram)
+#endif /* INCLUDE_ENA_HISTOGRAM */
+
+#endif /* _ENA_HISTOGRAM_H_ */
-- 
2.17.1

RE: [PATCH 0/1] net/ena: adding a histogram framework to 2.11.0

[Morten Brørup]

> From: Shai Brandes <shaibran@amazon.com>
> 
> This update is an enhancement to the ENA driver version 2.11.0 release.
> It is a "nice to have" feature for the 24.11 release, should it be
> included before the code freeze deadline.
> 
> This addition leverages an internal framework used in our testing
> processes, which may offer significant benefits for customers.
> It calculates the differences between specified start and end
> points in the code, generating a histogram of time intervals
> to provide detailed insights into performance.

Yes, histograms can be extremely useful.
The SmartShare applications use histograms for a variety of purposes, e.g. function profiling (like yours), number of packets per RX burst, number of packets per flow, and much more.
The distribution index can be linear (like yours), log2, or - for higher resolution - floating point with 3 or 4 bits mantissa and 5 or more bits exponent.

Please provide generic histograms as a proper library, not tied to a specific driver, and not specifically using "time" as parameter.

Re: [PATCH 1/1] net/ena: add histogram support

[Stephen Hemminger]

On Tue, 29 Oct 2024 10:02:15 +0200
<shaibran@amazon.com> wrote:

> From: Shai Brandes <shaibran@amazon.com>
> 
> The framework computes the differences between specified
> start and end points in the code, providing insights into
> performance by outputting a histogram of the time intervals.
> The histogram can be used to track metrics like packet
> processing latency in the network driver.
> This framework requires enablement via a compilation
> flag and is excluded in release builds by default.
> 
> Signed-off-by: Amit Bernstein <amitbern@amazon.com>
> Signed-off-by: Shai Brandes <shaibran@amazon.com>

Could this be generic for all PMD's and built off of existing DPDK trace mechanism?

Yes, histogram is useful.
But IMHO DPDK should discourage PMD specific features.

RE: [PATCH 0/1] net/ena: adding a histogram framework to 2.11.0

[Brandes, Shai]

> -----Original Message-----
> From: Morten Brørup <mb@smartsharesystems.com>
> Sent: Tuesday, October 29, 2024 10:55 AM
> To: Brandes, Shai <shaibran@amazon.com>; ferruh.yigit@amd.com
> Cc: dev@dpdk.org
> Subject: RE: [EXTERNAL] [PATCH 0/1] net/ena: adding a histogram framework
> to 2.11.0
> 
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you can confirm the sender and know the
> content is safe.
> 
> 
> 
> > From: Shai Brandes <shaibran@amazon.com>
> >
> > This update is an enhancement to the ENA driver version 2.11.0 release.
> > It is a "nice to have" feature for the 24.11 release, should it be
> > included before the code freeze deadline.
> >
> > This addition leverages an internal framework used in our testing
> > processes, which may offer significant benefits for customers.
> > It calculates the differences between specified start and end points
> > in the code, generating a histogram of time intervals to provide
> > detailed insights into performance.
> 
> Yes, histograms can be extremely useful.
> The SmartShare applications use histograms for a variety of purposes, e.g.
> function profiling (like yours), number of packets per RX burst, number of
> packets per flow, and much more.
> The distribution index can be linear (like yours), log2, or - for higher
> resolution - floating point with 3 or 4 bits mantissa and 5 or more bits
> exponent.
> 
> Please provide generic histograms as a proper library, not tied to a specific
> driver, and not specifically using "time" as parameter.
Acknowledged, thanks for your insights!

RE: [PATCH 1/1] net/ena: add histogram support

[Brandes, Shai]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, October 29, 2024 6:23 PM
> To: Brandes, Shai <shaibran@amazon.com>
> Cc: ferruh.yigit@amd.com; dev@dpdk.org; Bernstein, Amit
> <amitbern@amazon.com>
> Subject: RE: [EXTERNAL] [PATCH 1/1] net/ena: add histogram support
> 
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you can confirm the sender and know the
> content is safe.
> 
> 
> 
> On Tue, 29 Oct 2024 10:02:15 +0200
> <shaibran@amazon.com> wrote:
> 
> > From: Shai Brandes <shaibran@amazon.com>
> >
> > The framework computes the differences between specified start and end
> > points in the code, providing insights into performance by outputting
> > a histogram of the time intervals.
> > The histogram can be used to track metrics like packet processing
> > latency in the network driver.
> > This framework requires enablement via a compilation flag and is
> > excluded in release builds by default.
> >
> > Signed-off-by: Amit Bernstein <amitbern@amazon.com>
> > Signed-off-by: Shai Brandes <shaibran@amazon.com>
> 
> Could this be generic for all PMD's and built off of existing DPDK trace
> mechanism?
> 
> Yes, histogram is useful.
> But IMHO DPDK should discourage PMD specific features.
[Brandes, Shai] Acknowledged, thanks for your insights!