<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<div class="moz-cite-prefix">On 29/08/2022 11:53, Thomas Monjalon
wrote:<br>
</div>
<blockquote type="cite" cite="mid:2555175.9Mp67QZiUf@thomas">
<pre class="moz-quote-pre" wrap="">29/08/2022 12:41, Bruce Richardson:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Fri, Aug 26, 2022 at 05:46:48PM +0200, Morten Brørup wrote:
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">From: Kevin Laatz [<a class="moz-txt-link-freetext" href="mailto:kevin.laatz@intel.com">mailto:kevin.laatz@intel.com</a>]
Sent: Friday, 26 August 2022 17.27
On 26/08/2022 09:29, Morten Brørup wrote:
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">From: Jerin Jacob [<a class="moz-txt-link-freetext" href="mailto:jerinjacobk@gmail.com">mailto:jerinjacobk@gmail.com</a>]
Sent: Friday, 26 August 2022 10.16
On Fri, Aug 26, 2022 at 1:37 PM Bruce Richardson
<a class="moz-txt-link-rfc2396E" href="mailto:bruce.richardson@intel.com"><bruce.richardson@intel.com></a> wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Fri, Aug 26, 2022 at 12:35:16PM +0530, Jerin Jacob wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Thu, Aug 25, 2022 at 8:56 PM Kevin Laatz
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap=""><a class="moz-txt-link-rfc2396E" href="mailto:kevin.laatz@intel.com"><kevin.laatz@intel.com></a>
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">wrote:
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">From: Anatoly Burakov <a class="moz-txt-link-rfc2396E" href="mailto:anatoly.burakov@intel.com"><anatoly.burakov@intel.com></a>
Currently, there is no way to measure lcore poll busyness in a
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">passive way,
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">without any modifications to the application. This patch adds a
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">new EAL API
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">that will be able to passively track core polling busyness.
The poll busyness is calculated by relying on the fact that most
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">DPDK API's
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">will poll for packets. Empty polls can be counted as "idle",
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">while
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">non-empty polls can be counted as busy. To measure lcore poll
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">busyness, we
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">simply call the telemetry timestamping function with the number
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">of polls a
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">particular code section has processed, and count the number of
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">cycles we've
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">spent processing empty bursts. The more empty bursts we
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">encounter, the less
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">cycles we spend in "busy" state, and the less core poll busyness
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">will be
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">reported.
In order for all of the above to work without modifications to
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">the
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">application, the library code needs to be instrumented with calls
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">to the
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">lcore telemetry busyness timestamping function. The following
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">parts of DPDK
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">are instrumented with lcore telemetry calls:
- All major driver API's:
- ethdev
- cryptodev
- compressdev
- regexdev
- bbdev
- rawdev
- eventdev
- dmadev
- Some additional libraries:
- ring
- distributor
To avoid performance impact from having lcore telemetry support,
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">a global
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">variable is exported by EAL, and a call to timestamping function
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">is wrapped
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">into a macro, so that whenever telemetry is disabled, it only
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">takes one
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">additional branch and no function calls are performed. It is also
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">possible
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">to disable it at compile time by commenting out
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">RTE_LCORE_BUSYNESS from
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">build config.
This patch also adds a telemetry endpoint to report lcore poll
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">busyness, as
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">well as telemetry endpoints to enable/disable lcore telemetry. A
documentation entry has been added to the howto guides to explain
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">the usage
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">of the new telemetry endpoints and API.
Signed-off-by: Kevin Laatz <a class="moz-txt-link-rfc2396E" href="mailto:kevin.laatz@intel.com"><kevin.laatz@intel.com></a>
Signed-off-by: Conor Walsh <a class="moz-txt-link-rfc2396E" href="mailto:conor.walsh@intel.com"><conor.walsh@intel.com></a>
Signed-off-by: David Hunt <a class="moz-txt-link-rfc2396E" href="mailto:david.hunt@intel.com"><david.hunt@intel.com></a>
Signed-off-by: Anatoly Burakov <a class="moz-txt-link-rfc2396E" href="mailto:anatoly.burakov@intel.com"><anatoly.burakov@intel.com></a>
---
v3:
* Fix missed renaming to poll busyness
* Fix clang compilation
* Fix arm compilation
v2:
* Use rte_get_tsc_hz() to adjust the telemetry period
* Rename to reflect polling busyness vs general busyness
* Fix segfault when calling telemetry timestamp from an
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">unregistered
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> non-EAL thread.
* Minor cleanup
---
diff --git a/meson_options.txt b/meson_options.txt
index 7c220ad68d..725b851f69 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -20,6 +20,8 @@ option('enable_driver_sdk', type: 'boolean',
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">value: false, description:
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> 'Install headers to build drivers.')
option('enable_kmods', type: 'boolean', value: false,
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">description:
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> 'build kernel modules')
+option('enable_lcore_poll_busyness', type: 'boolean', value:
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">true, description:
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ 'enable collection of lcore poll busyness telemetry')
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">IMO, All fastpath features should be opt-in. i.e default should be
</pre>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">false.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">For the trace fastpath related changes, We have done the similar
</pre>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">thing
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">even though it cost additional one cycle for disabled trace points
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">We do need to consider runtime and build defaults differently,
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">though.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Since this has also runtime enabling, I think having build-time
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">enabling
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">true as default is ok, so long as the runtime enabling is false
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">(assuming
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">no noticable overhead when the feature is disabled.)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">I was talking about buildtime only. "enable_trace_fp" meson option
selected as
false as default.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Agree. "enable_lcore_poll_busyness" is in the fast path, so it should
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">follow the design pattern of "enable_trace_fp".
+1 to making this opt-in. However, I'd lean more towards having the
buildtime option enabled and the runtime option disabled by default.
There is no measurable impact cause by the extra branch (the check for
enabled/disabled in the macro) by disabling at runtime, and we gain the
benefit of avoiding a recompile to enable it later.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
The exact same thing could be said about "enable_trace_fp"; however, the development effort was put into separating it from "enable_trace", so it could be disabled by default.
Your patch is unlikely to get approved if you don't follow the "enable_trace_fp" design pattern as suggested.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">If the concern is enabling on generic distros then distro generic
config can opt in this
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">/Bruce
</pre>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">@Kevin, are you considering a roadmap for using
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">RTE_LCORE_TELEMETRY_TIMESTAMP() for other purposes? Otherwise, it
should also be renamed to indicate that it is part of the "poll
busyness" telemetry.
No further purposes are planned for this macro, I'll rename it in the
next revision.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
OK. Thank you.
Also, there's a new discussion about EAL bloat [1]. Perhaps I'm stretching it here, but it would be nice if your library was made a separate library, instead of part of the EAL library. (Since this kind of feature is not new to the EAL, I will categorize this suggestion as "nice to have", not "must have".)
[1] <a class="moz-txt-link-freetext" href="http://inbox.dpdk.org/dev/2594603.Isy0gbHreE@thomas/T/">http://inbox.dpdk.org/dev/2594603.Isy0gbHreE@thomas/T/</a>
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I was actually discussing this with Kevin and Dave H. on Friay, and trying
to make this a separate library is indeed a big stretch. :-)
>From that discussion, the key point/gap is that we are really missing a
clean way of providing undefs or macro fallbacks for when a library is just
not present. For example, if this was a separate library we would gain a
number of advantages e.g. no need for separate enable/disable flag, but the
big disadvantage is that every header include for it, and every reference
to the macros used in that header need to be surrounded by big ugly ifdefs.
For now, adding this into EAL is the far more practical approach, since it
means that even if support for the feature is disabled at build time the
header is still available to provide the appropriate no-op macros.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
We can make the library always available with different implementations
based on a build option.
But is it a good idea to have a different behaviour based on build option?
Why not making it a runtime option?
Can the performance hit be cached in some way?
</pre>
</blockquote>
The patches currently include runtime options to enable/disable the
feature via API and via telemetry endpoints. We have run performance
tests and have failed to measure any performance impact with the
feature <u>runtime</u> disabled.<br>
<br>
We added the buildtime option following previous feedback where an
absolute guarantee is required that performance would not be
affected by the addition of this feature. To avoid clutter in the
meson options, we could either a) remove the buildtime option, or b)
allow a CFLAG to disable the feature rather than an explicit
buildtime option. Either way, they would only serve a reassurance
purpose.<br>
<pre class="moz-quote-pre" wrap="">
</pre>
</body>
</html>