[v2,00/16] update and simplify telemetry library.

  This patchset extensively reworks the telemetry library adding new
functionality and simplifying much of the existing code, while
maintaining backward compatibility.

This work is based on the previously sent RFC for a "process info"
library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
However, rather than creating a new library, this patchset takes
that work and merges it into the existing telemetry library, as
mentioned above.

The telemetry library as shipped in 19.11 is based upon the metrics
library and outputs all statistics based on that as a source. However,
this limits the telemetry output to only port-level statistics
information, rather than allowing it to be used as a general scheme for
telemetry information across all DPDK libraries.

With this patchset applied, rather than the telemetry library being
responsible for pulling ethdev stats and pushing them into the metrics
library for retrieval later, each library e.g. ethdev, rawdev, and even
the metrics library itself (for backwards compatiblity) now handle their
own stats.  Any library or app can register a callback function with
telemetry, which will be called if requested by the client connected via
the telemetry socket. The callback function in the library/app then
formats its stats, or other data, into a JSON string, and returns it to
telemetry to be sent to the client.

To maintain backward compatibility, e.g. to allow the dpdk telemetry
collectd plugin to continue to work, some of the existing telemetry
code is kept, but is moved into the metrics library, and callbacks are
registered with telemetry for the legacy commands that were supported
previously.

The new version of the library, apart from the legacy interface support
for backward compatibility, does not have an external dependency on the
Jansson library, allowing the library to be enabled by default.

Note: In this version of the patchset, telemetry output is provided by
the ethdev, rawdev and eal libraries, but this may be expanded further
in later versions which are planned ahead of the merge deadline for
20.05

v2:
  - Added JSON API, and unit tests, to simplify creation of valid json
    responses from libraries.
  - Added printing of basic info, including max output buffer size, app
    PID and DPDK version on connection.
  - Added /info command to report that basic info post-connect. This
    replaces the eal version command from v1.
  - Renamed stats to xstats in commands to allow a future generic
    "stats" call.
  - Added documentation, including updating existing howto and adding
    programmers guide section and API docs.
  - Added link status command for ethdev ports.
  - Fixed windows builds.

Bruce Richardson (8):
  build: add arch-specific header path to global includes
  telemetry: invert dependency on metrics
  telemetry: introduce new telemetry functionality
  telemetry: add utility functions for creating json
  app/test: add telemetry json tests
  ethdev: add callback support for telemetry
  usertools: add new telemetry python script
  eal: add eal telemetry callbacks

Ciara Power (8):
  telemetry: move code to metrics for later reuse
  metrics: reduce code taken from telemetry
  rawdev: add callback support for telemetry
  examples/l3fwd-power: enable use of new telemetry
  telemetry: introduce telemetry backward compatibility
  telemetry: remove existing telemetry files
  lib: add telemetry as eal dependency
  doc: update telemetry documentation

 app/test/Makefile                             |    2 +
 app/test/meson.build                          |    4 +
 app/test/test_telemetry_json.c                |  136 ++
 config/common_base                            |    2 +-
 config/meson.build                            |    7 -
 doc/api/doxy-api-index.md                     |    1 +
 doc/guides/howto/telemetry.rst                |  108 +-
 doc/guides/linux_gsg/eal_args.include.rst     |    8 +
 doc/guides/linux_gsg/sys_reqs.rst             |    2 -
 doc/guides/prog_guide/index.rst               |    1 +
 doc/guides/prog_guide/telemetry_lib.rst       |   62 +
 doc/guides/rel_notes/release_20_05.rst        |   15 +
 examples/l3fwd-power/Makefile                 |    2 +-
 examples/l3fwd-power/main.c                   |   53 +-
 examples/l3fwd-power/meson.build              |    2 +-
 lib/Makefile                                  |   10 +-
 lib/librte_eal/arm/include/meson.build        |    2 -
 lib/librte_eal/common/eal_common_options.c    |   79 +
 lib/librte_eal/common/eal_internal_cfg.h      |    1 +
 lib/librte_eal/common/eal_options.h           |    7 +
 lib/librte_eal/freebsd/Makefile               |    1 +
 lib/librte_eal/freebsd/eal.c                  |   12 +
 lib/librte_eal/freebsd/meson.build            |    2 +-
 lib/librte_eal/linux/Makefile                 |    1 +
 lib/librte_eal/linux/eal.c                    |   12 +
 lib/librte_eal/linux/meson.build              |    2 +-
 lib/librte_eal/meson.build                    |    5 +-
 lib/librte_eal/ppc/include/meson.build        |    2 -
 lib/librte_eal/x86/include/meson.build        |    2 -
 lib/librte_ethdev/Makefile                    |    2 +-
 lib/librte_ethdev/meson.build                 |    2 +-
 lib/librte_ethdev/rte_ethdev.c                |  100 +
 lib/librte_metrics/Makefile                   |   11 +
 lib/librte_metrics/meson.build                |   10 +
 lib/librte_metrics/rte_metrics.c              |    6 +-
 lib/librte_metrics/rte_metrics.h              |    5 +-
 lib/librte_metrics/rte_metrics_telemetry.c    |  539 +++++
 lib/librte_metrics/rte_metrics_telemetry.h    |   65 +
 lib/librte_metrics/rte_metrics_version.map    |    7 +
 lib/librte_rawdev/Makefile                    |    3 +-
 lib/librte_rawdev/meson.build                 |    3 +
 lib/librte_rawdev/rte_rawdev.c                |   75 +
 lib/librte_telemetry/Makefile                 |   15 +-
 lib/librte_telemetry/meson.build              |   17 +-
 lib/librte_telemetry/rte_telemetry.c          | 1895 -----------------
 lib/librte_telemetry/rte_telemetry.h          |   77 +-
 lib/librte_telemetry/rte_telemetry_internal.h |  112 -
 lib/librte_telemetry/rte_telemetry_json.h     |  205 ++
 lib/librte_telemetry/rte_telemetry_legacy.h   |   63 +
 lib/librte_telemetry/rte_telemetry_parser.c   |  682 ------
 lib/librte_telemetry/rte_telemetry_parser.h   |   15 -
 .../rte_telemetry_parser_test.c               |  533 -----
 .../rte_telemetry_socket_tests.h              |   36 -
 .../rte_telemetry_version.map                 |    5 +-
 lib/librte_telemetry/telemetry.c              |  311 +++
 lib/librte_telemetry/telemetry_legacy.c       |  226 ++
 lib/meson.build                               |    3 +-
 meson.build                                   |    9 +
 mk/rte.app.mk                                 |   11 +-
 usertools/dpdk-telemetry.py                   |   44 +
 usertools/meson.build                         |    2 +-
 61 files changed, 2200 insertions(+), 3432 deletions(-)
 create mode 100644 app/test/test_telemetry_json.c
 create mode 100644 doc/guides/prog_guide/telemetry_lib.rst
 create mode 100644 lib/librte_metrics/rte_metrics_telemetry.c
 create mode 100644 lib/librte_metrics/rte_metrics_telemetry.h
 delete mode 100644 lib/librte_telemetry/rte_telemetry.c
 delete mode 100644 lib/librte_telemetry/rte_telemetry_internal.h
 create mode 100644 lib/librte_telemetry/rte_telemetry_json.h
 create mode 100644 lib/librte_telemetry/rte_telemetry_legacy.h
 delete mode 100644 lib/librte_telemetry/rte_telemetry_parser.c
 delete mode 100644 lib/librte_telemetry/rte_telemetry_parser.h
 delete mode 100644 lib/librte_telemetry/rte_telemetry_parser_test.c
 delete mode 100644 lib/librte_telemetry/rte_telemetry_socket_tests.h
 create mode 100644 lib/librte_telemetry/telemetry.c
 create mode 100644 lib/librte_telemetry/telemetry_legacy.c
 create mode 100755 usertools/dpdk-telemetry.py
  

08/04/2020 18:49, Ciara Power:
> This patchset extensively reworks the telemetry library adding new
> functionality and simplifying much of the existing code, while
> maintaining backward compatibility.
> 
> This work is based on the previously sent RFC for a "process info"
> library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> However, rather than creating a new library, this patchset takes
> that work and merges it into the existing telemetry library, as
> mentioned above.
> 
> The telemetry library as shipped in 19.11 is based upon the metrics
> library and outputs all statistics based on that as a source. However,
> this limits the telemetry output to only port-level statistics
> information, rather than allowing it to be used as a general scheme for
> telemetry information across all DPDK libraries.
> 
> With this patchset applied, rather than the telemetry library being
> responsible for pulling ethdev stats and pushing them into the metrics
> library for retrieval later, each library e.g. ethdev, rawdev, and even
> the metrics library itself (for backwards compatiblity) now handle their
> own stats.  Any library or app can register a callback function with
> telemetry, which will be called if requested by the client connected via
> the telemetry socket. The callback function in the library/app then
> formats its stats, or other data, into a JSON string, and returns it to
> telemetry to be sent to the client.

I think this is a global need in DPDK, and it is usually called RPC,
IPC or control messaging.
We had a similar need for multi-process communication, thus rte_mp IPC.
We also need a control channel for user configuration applications.
We also need to control some features like logging or tracing.

In my opinion, it is time to introduce a general control channel in DPDK.
The application must be in the loop of the control mechanism.
Making such channel standard will ease application adoption.

Please read some comments here:
http://inbox.dpdk.org/dev/2580933.jp2sp48Hzj@xps/
  

On Wed, Apr 08, 2020 at 08:03:26PM +0200, Thomas Monjalon wrote:
> 08/04/2020 18:49, Ciara Power:
> > This patchset extensively reworks the telemetry library adding new
> > functionality and simplifying much of the existing code, while
> > maintaining backward compatibility.
> > 
> > This work is based on the previously sent RFC for a "process info"
> > library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> > However, rather than creating a new library, this patchset takes
> > that work and merges it into the existing telemetry library, as
> > mentioned above.
> > 
> > The telemetry library as shipped in 19.11 is based upon the metrics
> > library and outputs all statistics based on that as a source. However,
> > this limits the telemetry output to only port-level statistics
> > information, rather than allowing it to be used as a general scheme for
> > telemetry information across all DPDK libraries.
> > 
> > With this patchset applied, rather than the telemetry library being
> > responsible for pulling ethdev stats and pushing them into the metrics
> > library for retrieval later, each library e.g. ethdev, rawdev, and even
> > the metrics library itself (for backwards compatiblity) now handle their
> > own stats.  Any library or app can register a callback function with
> > telemetry, which will be called if requested by the client connected via
> > the telemetry socket. The callback function in the library/app then
> > formats its stats, or other data, into a JSON string, and returns it to
> > telemetry to be sent to the client.
> 
> I think this is a global need in DPDK, and it is usually called RPC,
> IPC or control messaging.
> We had a similar need for multi-process communication, thus rte_mp IPC.
> We also need a control channel for user configuration applications.
> We also need to control some features like logging or tracing.
> 
> In my opinion, it is time to introduce a general control channel in DPDK.
> The application must be in the loop of the control mechanism.
> Making such channel standard will ease application adoption.
> 
> Please read some comments here:
> http://inbox.dpdk.org/dev/2580933.jp2sp48Hzj@xps/
> 
Hi Thomas,

I agree that having a single control mechanism or messaging mechanism in
DPDK would be nice to have. However, I don't believe the plans for such a
scheme should impact this patchset right now as the idea of a common
channel was only first mooted about a week ago, and while there has been
some email discussion about it, there is as yet no requirements list that
I've seen, nobody actually doing coding work on it, no rfc and most
importantly no timeline for creating and merging such into DPDK.

At present though, DPDK has a telemetry solution that works for the use case
of ethdev stats and some power management info, but requires a more general
solution to allow monitoring tools like PMDT to introspect DPDK, and also
to prove statistics for other parts of DPDK such as cryptodev, eventdev,
and other libraries, plus the application itself if the app so desires.

Regards,
/Bruce
  

09/04/2020 11:19, Bruce Richardson:
> On Wed, Apr 08, 2020 at 08:03:26PM +0200, Thomas Monjalon wrote:
> > 08/04/2020 18:49, Ciara Power:
> > > This patchset extensively reworks the telemetry library adding new
> > > functionality and simplifying much of the existing code, while
> > > maintaining backward compatibility.
> > > 
> > > This work is based on the previously sent RFC for a "process info"
> > > library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> > > However, rather than creating a new library, this patchset takes
> > > that work and merges it into the existing telemetry library, as
> > > mentioned above.
> > > 
> > > The telemetry library as shipped in 19.11 is based upon the metrics
> > > library and outputs all statistics based on that as a source. However,
> > > this limits the telemetry output to only port-level statistics
> > > information, rather than allowing it to be used as a general scheme for
> > > telemetry information across all DPDK libraries.
> > > 
> > > With this patchset applied, rather than the telemetry library being
> > > responsible for pulling ethdev stats and pushing them into the metrics
> > > library for retrieval later, each library e.g. ethdev, rawdev, and even
> > > the metrics library itself (for backwards compatiblity) now handle their
> > > own stats.  Any library or app can register a callback function with
> > > telemetry, which will be called if requested by the client connected via
> > > the telemetry socket. The callback function in the library/app then
> > > formats its stats, or other data, into a JSON string, and returns it to
> > > telemetry to be sent to the client.
> > 
> > I think this is a global need in DPDK, and it is usually called RPC,
> > IPC or control messaging.
> > We had a similar need for multi-process communication, thus rte_mp IPC.
> > We also need a control channel for user configuration applications.
> > We also need to control some features like logging or tracing.
> > 
> > In my opinion, it is time to introduce a general control channel in DPDK.
> > The application must be in the loop of the control mechanism.
> > Making such channel standard will ease application adoption.
> > 
> > Please read some comments here:
> > http://inbox.dpdk.org/dev/2580933.jp2sp48Hzj@xps/
> > 
> Hi Thomas,
> 
> I agree that having a single control mechanism or messaging mechanism in
> DPDK would be nice to have. However, I don't believe the plans for such a
> scheme should impact this patchset right now as the idea of a common
> channel was only first mooted about a week ago, and while there has been
> some email discussion about it, there is as yet no requirements list that
> I've seen, nobody actually doing coding work on it, no rfc and most
> importantly no timeline for creating and merging such into DPDK.

Yes, this is a new idea.
Throwing the idea in this "telemetry" thread and in "IF proxy" thread
is the first step before starting a dedicated thread to design
a generic mechanism.

> At present though, DPDK has a telemetry solution that works for the use case
> of ethdev stats and some power management info, but requires a more general
> solution to allow monitoring tools like PMDT to introspect DPDK, and also
> to prove statistics for other parts of DPDK such as cryptodev, eventdev,
> and other libraries, plus the application itself if the app so desires.

Doing rework on telemetry is similar to a general control mechanism.
Can we take this opportunity to work on what we believe to be a bigger
idea? It should be done anyway, so why pushing this temporary solution?
Sometimes we need a quick answer to an urgent problem.
But I don't think telemetry is currently in such situation that
a rework in 20.05 is mandatory.
  

> From: Ciara Power [mailto:ciara.power@intel.com]
> Sent: Wednesday, April 8, 2020 6:50 PM
> 
> This patchset extensively reworks the telemetry library adding new
> functionality and simplifying much of the existing code, while
> maintaining backward compatibility.
> 
> This work is based on the previously sent RFC for a "process info"
> library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> However, rather than creating a new library, this patchset takes
> that work and merges it into the existing telemetry library, as
> mentioned above.
> 
> The telemetry library as shipped in 19.11 is based upon the metrics
> library and outputs all statistics based on that as a source. However,
> this limits the telemetry output to only port-level statistics
> information, rather than allowing it to be used as a general scheme for
> telemetry information across all DPDK libraries.
> 
> With this patchset applied, rather than the telemetry library being
> responsible for pulling ethdev stats and pushing them into the metrics
> library for retrieval later, each library e.g. ethdev, rawdev, and even
> the metrics library itself (for backwards compatiblity) now handle
> their
> own stats.  Any library or app can register a callback function with
> telemetry, which will be called if requested by the client connected
> via
> the telemetry socket.

Great. Standardization across libraries is a good improvement.

> The callback function in the library/app then
> formats its stats, or other data, into a JSON string, and returns it to
> telemetry to be sent to the client.

I am strongly opposed to using JSON as the standard format in DPDK, and instead prefer a binary format with zero (or minimal) type conversion.

Here is one reason why I dislike JSON for this: A part of our application samples 100k+ counters every second to be able to provide drill-down statistics through the GUI. Converting these counters from uint64_t to JSON and back to uint64_t for data processing is not going to fly. And I assume that we are not the only company developing equipment with drill-down statistics.

I am aware that there is a difference between statistics for *drill-down and data processing* purposes and statistics for *telemetry eyeball viewing only* purposes, but the line is blurry, and I see a big risk of setting a path that leads to JSON being used in places where it shouldn't.

Here is another reason why I dislike JSON for this: JSON is fine for the LAMP stack with REST protocols. But other systems use other protocols with other formats, e.g. the TICK stack uses an even simpler text based format. So DPDK based systems supporting the TICK stack will need to convert to first JSON format (in the DPDK libraries), and then from JSON format to InfluxDB format (in the DPDK application).

I think that type conversion does not belong inside deep inside the DPDK libraries, but is a job for the DPDK application. However, DPDK could provide libraries for efficient bulk conversion to popular formats like JSON. And other formats, if they are relevant, e.g. ASN.1 used by old school SNMP.
  

> On Apr 10, 2020, at 5:49 AM, Morten Brørup <mb@smartsharesystems.com> wrote:
> 
>> From: Ciara Power [mailto:ciara.power@intel.com]
>> Sent: Wednesday, April 8, 2020 6:50 PM
>> 
>> This patchset extensively reworks the telemetry library adding new
>> functionality and simplifying much of the existing code, while
>> maintaining backward compatibility.
>> 
>> This work is based on the previously sent RFC for a "process info"
>> library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
>> However, rather than creating a new library, this patchset takes
>> that work and merges it into the existing telemetry library, as
>> mentioned above.
>> 
>> The telemetry library as shipped in 19.11 is based upon the metrics
>> library and outputs all statistics based on that as a source. However,
>> this limits the telemetry output to only port-level statistics
>> information, rather than allowing it to be used as a general scheme for
>> telemetry information across all DPDK libraries.
>> 
>> With this patchset applied, rather than the telemetry library being
>> responsible for pulling ethdev stats and pushing them into the metrics
>> library for retrieval later, each library e.g. ethdev, rawdev, and even
>> the metrics library itself (for backwards compatiblity) now handle
>> their
>> own stats.  Any library or app can register a callback function with
>> telemetry, which will be called if requested by the client connected
>> via
>> the telemetry socket.
> 
> Great. Standardization across libraries is a good improvement.
> 
>> The callback function in the library/app then
>> formats its stats, or other data, into a JSON string, and returns it to
>> telemetry to be sent to the client.
> 
> I am strongly opposed to using JSON as the standard format in DPDK, and instead prefer a binary format with zero (or minimal) type conversion.
> 
> Here is one reason why I dislike JSON for this: A part of our application samples 100k+ counters every second to be able to provide drill-down statistics through the GUI. Converting these counters from uint64_t to JSON and back to uint64_t for data processing is not going to fly. And I assume that we are not the only company developing equipment with drill-down statistics.
> 
> I am aware that there is a difference between statistics for *drill-down and data processing* purposes and statistics for *telemetry eyeball viewing only* purposes, but the line is blurry, and I see a big risk of setting a path that leads to JSON being used in places where it shouldn't.
> 
> Here is another reason why I dislike JSON for this: JSON is fine for the LAMP stack with REST protocols. But other systems use other protocols with other formats, e.g. the TICK stack uses an even simpler text based format. So DPDK based systems supporting the TICK stack will need to convert to first JSON format (in the DPDK libraries), and then from JSON format to InfluxDB format (in the DPDK application).
> 
> I think that type conversion does not belong inside deep inside the DPDK libraries, but is a job for the DPDK application. However, DPDK could provide libraries for efficient bulk conversion to popular formats like JSON. And other formats, if they are relevant, e.g. ASN.1 used by old school SNMP.

I believe JSON has it place in this library and in DPDK as it is a good conversion tool and easy to utilize with a huge number of tools/languages. Binary output gets into endianness issues and a number of other problems, so I would not want all of the data exported from DPDK to be in binary format. If the layout of the structure changes then the code would need to know that on both side to be able to convert the data into the correct values.

With that stated, the new telemetry code allows the application to add new commands and with that you can create a binary set of commands along side the JSON or any other output format. With the new register command we can create say a ‘/ethdevraw/stats,X’ set of commands that can emit binary format.

Using this method we get the best of both worlds and when using languages like Go or Python to collect these stats we have a standard format for conversion. In Go it is pretty hard to do binary conversion and JSON conversion is just a few lines. JSON may not be the fastest, but if you are requesting stats faster than a second then use the raw commands to get the data, which anyone can add to its application or we can add them to DPDK as a standard command set.
  

> On Apr 9, 2020, at 4:37 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 09/04/2020 11:19, Bruce Richardson:
>> On Wed, Apr 08, 2020 at 08:03:26PM +0200, Thomas Monjalon wrote:
>>> 08/04/2020 18:49, Ciara Power:
>>>> This patchset extensively reworks the telemetry library adding new
>>>> functionality and simplifying much of the existing code, while
>>>> maintaining backward compatibility.
>>>> 
>>>> This work is based on the previously sent RFC for a "process info"
>>>> library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
>>>> However, rather than creating a new library, this patchset takes
>>>> that work and merges it into the existing telemetry library, as
>>>> mentioned above.
>>>> 
>>>> The telemetry library as shipped in 19.11 is based upon the metrics
>>>> library and outputs all statistics based on that as a source. However,
>>>> this limits the telemetry output to only port-level statistics
>>>> information, rather than allowing it to be used as a general scheme for
>>>> telemetry information across all DPDK libraries.
>>>> 
>>>> With this patchset applied, rather than the telemetry library being
>>>> responsible for pulling ethdev stats and pushing them into the metrics
>>>> library for retrieval later, each library e.g. ethdev, rawdev, and even
>>>> the metrics library itself (for backwards compatiblity) now handle their
>>>> own stats.  Any library or app can register a callback function with
>>>> telemetry, which will be called if requested by the client connected via
>>>> the telemetry socket. The callback function in the library/app then
>>>> formats its stats, or other data, into a JSON string, and returns it to
>>>> telemetry to be sent to the client.
>>> 
>>> I think this is a global need in DPDK, and it is usually called RPC,
>>> IPC or control messaging.
>>> We had a similar need for multi-process communication, thus rte_mp IPC.
>>> We also need a control channel for user configuration applications.
>>> We also need to control some features like logging or tracing.
>>> 
>>> In my opinion, it is time to introduce a general control channel in DPDK.
>>> The application must be in the loop of the control mechanism.
>>> Making such channel standard will ease application adoption.
>>> 
>>> Please read some comments here:
>>> http://inbox.dpdk.org/dev/2580933.jp2sp48Hzj@xps/
>>> 
>> Hi Thomas,
>> 
>> I agree that having a single control mechanism or messaging mechanism in
>> DPDK would be nice to have. However, I don't believe the plans for such a
>> scheme should impact this patchset right now as the idea of a common
>> channel was only first mooted about a week ago, and while there has been
>> some email discussion about it, there is as yet no requirements list that
>> I've seen, nobody actually doing coding work on it, no rfc and most
>> importantly no timeline for creating and merging such into DPDK.
> 
> Yes, this is a new idea.
> Throwing the idea in this "telemetry" thread and in "IF proxy" thread
> is the first step before starting a dedicated thread to design
> a generic mechanism.
> 
>> At present though, DPDK has a telemetry solution that works for the use case
>> of ethdev stats and some power management info, but requires a more general
>> solution to allow monitoring tools like PMDT to introspect DPDK, and also
>> to prove statistics for other parts of DPDK such as cryptodev, eventdev,
>> and other libraries, plus the application itself if the app so desires.
> 
> Doing rework on telemetry is similar to a general control mechanism.
> Can we take this opportunity to work on what we believe to be a bigger
> idea? It should be done anyway, so why pushing this temporary solution?
> Sometimes we need a quick answer to an urgent problem.
> But I don't think telemetry is currently in such situation that
> a rework in 20.05 is mandatory.

Updating telemetry to be more consumable and standardize on a single method to get stats/info out of DPDK is a clean and simple solution. Starting over and creating yet another solution means we are pushing this support out again and many customer are asking for this support now.

The current telemetry solution in this patch gives us a great starting point and going back to the drawing board is a waste of time IMO and we need something now. To me this is a urgent problem we need to solve now, as I want to push PMDT and if we keep pushing out this type of support then it will never be upstreamed.

In PMDT I believe I have resolved all of the tech boards concerns and just waiting for this patch and a patch to PCM to push the code back to DPDK again.

So please let's not redesign this again.
>  
>
  

10/04/2020 16:39, Wiles, Keith:
> > On Apr 9, 2020, at 4:37 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 09/04/2020 11:19, Bruce Richardson:
> >> On Wed, Apr 08, 2020 at 08:03:26PM +0200, Thomas Monjalon wrote:
> >>> 08/04/2020 18:49, Ciara Power:
> >>>> This patchset extensively reworks the telemetry library adding new
> >>>> functionality and simplifying much of the existing code, while
> >>>> maintaining backward compatibility.
> >>>> 
> >>>> This work is based on the previously sent RFC for a "process info"
> >>>> library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> >>>> However, rather than creating a new library, this patchset takes
> >>>> that work and merges it into the existing telemetry library, as
> >>>> mentioned above.
> >>>> 
> >>>> The telemetry library as shipped in 19.11 is based upon the metrics
> >>>> library and outputs all statistics based on that as a source. However,
> >>>> this limits the telemetry output to only port-level statistics
> >>>> information, rather than allowing it to be used as a general scheme for
> >>>> telemetry information across all DPDK libraries.
> >>>> 
> >>>> With this patchset applied, rather than the telemetry library being
> >>>> responsible for pulling ethdev stats and pushing them into the metrics
> >>>> library for retrieval later, each library e.g. ethdev, rawdev, and even
> >>>> the metrics library itself (for backwards compatiblity) now handle their
> >>>> own stats.  Any library or app can register a callback function with
> >>>> telemetry, which will be called if requested by the client connected via
> >>>> the telemetry socket. The callback function in the library/app then
> >>>> formats its stats, or other data, into a JSON string, and returns it to
> >>>> telemetry to be sent to the client.
> >>> 
> >>> I think this is a global need in DPDK, and it is usually called RPC,
> >>> IPC or control messaging.
> >>> We had a similar need for multi-process communication, thus rte_mp IPC.
> >>> We also need a control channel for user configuration applications.
> >>> We also need to control some features like logging or tracing.
> >>> 
> >>> In my opinion, it is time to introduce a general control channel in DPDK.
> >>> The application must be in the loop of the control mechanism.
> >>> Making such channel standard will ease application adoption.
> >>> 
> >>> Please read some comments here:
> >>> http://inbox.dpdk.org/dev/2580933.jp2sp48Hzj@xps/
> >>> 
> >> Hi Thomas,
> >> 
> >> I agree that having a single control mechanism or messaging mechanism in
> >> DPDK would be nice to have. However, I don't believe the plans for such a
> >> scheme should impact this patchset right now as the idea of a common
> >> channel was only first mooted about a week ago, and while there has been
> >> some email discussion about it, there is as yet no requirements list that
> >> I've seen, nobody actually doing coding work on it, no rfc and most
> >> importantly no timeline for creating and merging such into DPDK.
> > 
> > Yes, this is a new idea.
> > Throwing the idea in this "telemetry" thread and in "IF proxy" thread
> > is the first step before starting a dedicated thread to design
> > a generic mechanism.
> > 
> >> At present though, DPDK has a telemetry solution that works for the use case
> >> of ethdev stats and some power management info, but requires a more general
> >> solution to allow monitoring tools like PMDT to introspect DPDK, and also
> >> to prove statistics for other parts of DPDK such as cryptodev, eventdev,
> >> and other libraries, plus the application itself if the app so desires.
> > 
> > Doing rework on telemetry is similar to a general control mechanism.
> > Can we take this opportunity to work on what we believe to be a bigger
> > idea? It should be done anyway, so why pushing this temporary solution?
> > Sometimes we need a quick answer to an urgent problem.
> > But I don't think telemetry is currently in such situation that
> > a rework in 20.05 is mandatory.
> 
> Updating telemetry to be more consumable and standardize on a single method to get stats/info out of DPDK is a clean and simple solution. Starting over and creating yet another solution means we are pushing this support out again and many customer are asking for this support now.
> 
> The current telemetry solution in this patch gives us a great starting point and going back to the drawing board is a waste of time IMO and we need something now. To me this is a urgent problem we need to solve now, as I want to push PMDT and if we keep pushing out this type of support then it will never be upstreamed.
> 
> In PMDT I believe I have resolved all of the tech boards concerns and just waiting for this patch and a patch to PCM to push the code back to DPDK again.
> 
> So please let's not redesign this again.

I understand your concern.

I think we need to go to the drawing board,
and consider at least these 5 use cases:
	1/ multi-process IPC
	2/ telemetry
	3/ IF proxy
	4/ external user configuration
	5/ log/trace start/stop

Merging telemetry means we'll rework 1 and 2 later.
I am OK with merging telemetry in 20.05 if we can be sure
that there will be no resistance and help for reworking it
with a more general communication channel if required later.

We need a kind of community vote here. Please give +1 / -1.
Giving +1 means you will help when needed.
  

> On Apr 10, 2020, at 9:51 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 10/04/2020 16:39, Wiles, Keith:
>>> On Apr 9, 2020, at 4:37 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 09/04/2020 11:19, Bruce Richardson:
>>>> On Wed, Apr 08, 2020 at 08:03:26PM +0200, Thomas Monjalon wrote:
>>>>> 08/04/2020 18:49, Ciara Power:
>>>>>> This patchset extensively reworks the telemetry library adding new
>>>>>> functionality and simplifying much of the existing code, while
>>>>>> maintaining backward compatibility.
>>>>>> 
>>>>>> This work is based on the previously sent RFC for a "process info"
>>>>>> library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
>>>>>> However, rather than creating a new library, this patchset takes
>>>>>> that work and merges it into the existing telemetry library, as
>>>>>> mentioned above.
>>>>>> 
>>>>>> The telemetry library as shipped in 19.11 is based upon the metrics
>>>>>> library and outputs all statistics based on that as a source. However,
>>>>>> this limits the telemetry output to only port-level statistics
>>>>>> information, rather than allowing it to be used as a general scheme for
>>>>>> telemetry information across all DPDK libraries.
>>>>>> 
>>>>>> With this patchset applied, rather than the telemetry library being
>>>>>> responsible for pulling ethdev stats and pushing them into the metrics
>>>>>> library for retrieval later, each library e.g. ethdev, rawdev, and even
>>>>>> the metrics library itself (for backwards compatiblity) now handle their
>>>>>> own stats.  Any library or app can register a callback function with
>>>>>> telemetry, which will be called if requested by the client connected via
>>>>>> the telemetry socket. The callback function in the library/app then
>>>>>> formats its stats, or other data, into a JSON string, and returns it to
>>>>>> telemetry to be sent to the client.
>>>>> 
>>>>> I think this is a global need in DPDK, and it is usually called RPC,
>>>>> IPC or control messaging.
>>>>> We had a similar need for multi-process communication, thus rte_mp IPC.
>>>>> We also need a control channel for user configuration applications.
>>>>> We also need to control some features like logging or tracing.
>>>>> 
>>>>> In my opinion, it is time to introduce a general control channel in DPDK.
>>>>> The application must be in the loop of the control mechanism.
>>>>> Making such channel standard will ease application adoption.
>>>>> 
>>>>> Please read some comments here:
>>>>> http://inbox.dpdk.org/dev/2580933.jp2sp48Hzj@xps/
>>>>> 
>>>> Hi Thomas,
>>>> 
>>>> I agree that having a single control mechanism or messaging mechanism in
>>>> DPDK would be nice to have. However, I don't believe the plans for such a
>>>> scheme should impact this patchset right now as the idea of a common
>>>> channel was only first mooted about a week ago, and while there has been
>>>> some email discussion about it, there is as yet no requirements list that
>>>> I've seen, nobody actually doing coding work on it, no rfc and most
>>>> importantly no timeline for creating and merging such into DPDK.
>>> 
>>> Yes, this is a new idea.
>>> Throwing the idea in this "telemetry" thread and in "IF proxy" thread
>>> is the first step before starting a dedicated thread to design
>>> a generic mechanism.
>>> 
>>>> At present though, DPDK has a telemetry solution that works for the use case
>>>> of ethdev stats and some power management info, but requires a more general
>>>> solution to allow monitoring tools like PMDT to introspect DPDK, and also
>>>> to prove statistics for other parts of DPDK such as cryptodev, eventdev,
>>>> and other libraries, plus the application itself if the app so desires.
>>> 
>>> Doing rework on telemetry is similar to a general control mechanism.
>>> Can we take this opportunity to work on what we believe to be a bigger
>>> idea? It should be done anyway, so why pushing this temporary solution?
>>> Sometimes we need a quick answer to an urgent problem.
>>> But I don't think telemetry is currently in such situation that
>>> a rework in 20.05 is mandatory.
>> 
>> Updating telemetry to be more consumable and standardize on a single method to get stats/info out of DPDK is a clean and simple solution. Starting over and creating yet another solution means we are pushing this support out again and many customer are asking for this support now.
>> 
>> The current telemetry solution in this patch gives us a great starting point and going back to the drawing board is a waste of time IMO and we need something now. To me this is a urgent problem we need to solve now, as I want to push PMDT and if we keep pushing out this type of support then it will never be upstreamed.
>> 
>> In PMDT I believe I have resolved all of the tech boards concerns and just waiting for this patch and a patch to PCM to push the code back to DPDK again.
>> 
>> So please let's not redesign this again.
> 
> I understand your concern.
> 
> I think we need to go to the drawing board,
> and consider at least these 5 use cases:
> 	1/ multi-process IPC
> 	2/ telemetry
> 	3/ IF proxy
> 	4/ external user configuration
> 	5/ log/trace start/stop
> 
> Merging telemetry means we'll rework 1 and 2 later.
> I am OK with merging telemetry in 20.05 if we can be sure
> that there will be no resistance and help for reworking it
> with a more general communication channel if required later.
> 
> We need a kind of community vote here. Please give +1 / -1.
> Giving +1 means you will help when needed.
> 

I agree merging the telemetry patch for 20.05 is reasonable and addressing the above concerns later is a good compromise.

+1 for helping on the above concerns later after the 20.05 release and merge of telemetry patch.
>
  

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Keith
> Sent: Friday, April 10, 2020 4:22 PM
> 
> > On Apr 10, 2020, at 5:49 AM, Morten Brørup <mb@smartsharesystems.com>
> wrote:
> >
> >> From: Ciara Power [mailto:ciara.power@intel.com]
> >> Sent: Wednesday, April 8, 2020 6:50 PM
> >>
> >> This patchset extensively reworks the telemetry library adding new
> >> functionality and simplifying much of the existing code, while
> >> maintaining backward compatibility.
> >>
> >> This work is based on the previously sent RFC for a "process info"
> >> library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> >> However, rather than creating a new library, this patchset takes
> >> that work and merges it into the existing telemetry library, as
> >> mentioned above.
> >>
> >> The telemetry library as shipped in 19.11 is based upon the metrics
> >> library and outputs all statistics based on that as a source. However,
> >> this limits the telemetry output to only port-level statistics
> >> information, rather than allowing it to be used as a general scheme for
> >> telemetry information across all DPDK libraries.
> >>
> >> With this patchset applied, rather than the telemetry library being
> >> responsible for pulling ethdev stats and pushing them into the metrics
> >> library for retrieval later, each library e.g. ethdev, rawdev, and even
> >> the metrics library itself (for backwards compatiblity) now handle
> >> their
> >> own stats.  Any library or app can register a callback function with
> >> telemetry, which will be called if requested by the client connected
> >> via
> >> the telemetry socket.
> >
> > Great. Standardization across libraries is a good improvement.
> >
> >> The callback function in the library/app then
> >> formats its stats, or other data, into a JSON string, and returns it to
> >> telemetry to be sent to the client.
> >
> > I am strongly opposed to using JSON as the standard format in DPDK, and
> instead prefer a binary format with zero (or minimal) type conversion.
> >
> > Here is one reason why I dislike JSON for this: A part of our application
> samples 100k+ counters every second to be able to provide drill-down
> statistics through the GUI. Converting these counters from uint64_t to JSON
> and back to uint64_t for data processing is not going to fly. And I assume
> that we are not the only company developing equipment with drill-down
> statistics.
> >
> > I am aware that there is a difference between statistics for *drill-down
> and data processing* purposes and statistics for *telemetry eyeball viewing
> only* purposes, but the line is blurry, and I see a big risk of setting a
> path that leads to JSON being used in places where it shouldn't.
> >
> > Here is another reason why I dislike JSON for this: JSON is fine for the
> LAMP stack with REST protocols. But other systems use other protocols with
> other formats, e.g. the TICK stack uses an even simpler text based format.
> So DPDK based systems supporting the TICK stack will need to convert to
> first JSON format (in the DPDK libraries), and then from JSON format to
> InfluxDB format (in the DPDK application).
> >
> > I think that type conversion does not belong inside deep inside the DPDK
> libraries, but is a job for the DPDK application. However, DPDK could
> provide libraries for efficient bulk conversion to popular formats like
> JSON. And other formats, if they are relevant, e.g. ASN.1 used by old
> school SNMP.
> 
> I believe JSON has it place in this library and in DPDK as it is a good
> conversion tool and easy to utilize with a huge number of tools/languages.

JSON is extremely heavy compared to a raw binary format.

It makes sense for low volume, hierarchical structured data, but not for large tables or arrays of counters.

> Binary output gets into endianness issues and a number of other problems,
> so I would not want all of the data exported from DPDK to be in binary
> format.

Endianness considerations are only relevant for data exchanged across the network; not data exchanged across processes inside the same machine.

And if you are exchanging data across the network, you would usually implement one or more well known protocols for that, e.g. JSON over HTTPS, or ASN.1 over SNMP, or InfluxDB over UDP. This means that the application needs to implement a protocol handler, which - in my opinion - should handle the relevant data type conversions from the raw format provided by DPDK.

I think it would be silly for DPDK core libraries to provide counters in JSON format, so an SNMP Agent would need to convert them from JSON back to binary and then to ASN.1.

> If the layout of the structure changes then the code would need to
> know that on both side to be able to convert the data into the correct
> values.

I may be exaggerating here, but trying to prove a point: This is what we have ABI stability for. Structures should be designed cleverly and future proof, e.g. like the ethdev xstats. Using text based APIs is a circumvention of ABI stability.

> 
> With that stated, the new telemetry code allows the application to add new
> commands and with that you can create a binary set of commands along side
> the JSON or any other output format. With the new register command we can
> create say a ‘/ethdevraw/stats,X’ set of commands that can emit binary
> format.

That would be silly. The protocol handler should make the protocol specific conversion, not the driver! Again, going to the extreme to prove a point: If I understand you correctly, this would mean that PMDs would have provide counters in ASN.1 format for SNMP.

Our application provides a HTTPS/REST based communication interface for multiple purposes, e.g. getting tables of data. And if you want to get a table of some data via this interface, you can specify the output format in the request, so you can get it in e.g. TSV format (tabulator separated with a headline) for scripts, HTML format for human eyeballs. This data conversion happens at a common location, so we can easily add other output formats. You don't want to push this all the way down to the originator of the data.

> 
> Using this method we get the best of both worlds and when using languages
> like Go or Python to collect these stats we have a standard format for
> conversion. In Go it is pretty hard to do binary conversion and JSON
> conversion is just a few lines. 

I don't think DPDK should provide preferential treatment to Go or Python. DPDK is based on C, and should mainly cater for C.

> JSON may not be the fastest, but if you are
> requesting stats faster than a second then use the raw commands to get the
> data, which anyone can add to its application or we can add them to DPDK as
> a standard command set.

APIs in the libraries are currently available to get data in raw format. My main concern is that libraries in the future will not provide functions to get raw data, like they do now, but only JSON formatted data for the telemetry library. This is what I want to avoid.
  

On Fri, Apr 10, 2020 at 08:06:23PM +0200, Morten Brørup wrote:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Keith
> > Sent: Friday, April 10, 2020 4:22 PM
> > 
> > > On Apr 10, 2020, at 5:49 AM, Morten Brørup <mb@smartsharesystems.com>
> > wrote:
> > >
> > >> From: Ciara Power [mailto:ciara.power@intel.com]
> > >> Sent: Wednesday, April 8, 2020 6:50 PM
> > >>
> > >> This patchset extensively reworks the telemetry library adding new
> > >> functionality and simplifying much of the existing code, while
> > >> maintaining backward compatibility.
> > >>
> > >> This work is based on the previously sent RFC for a "process info"
> > >> library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> > >> However, rather than creating a new library, this patchset takes
> > >> that work and merges it into the existing telemetry library, as
> > >> mentioned above.
> > >>
> > >> The telemetry library as shipped in 19.11 is based upon the metrics
> > >> library and outputs all statistics based on that as a source. However,
> > >> this limits the telemetry output to only port-level statistics
> > >> information, rather than allowing it to be used as a general scheme for
> > >> telemetry information across all DPDK libraries.
> > >>
> > >> With this patchset applied, rather than the telemetry library being
> > >> responsible for pulling ethdev stats and pushing them into the metrics
> > >> library for retrieval later, each library e.g. ethdev, rawdev, and even
> > >> the metrics library itself (for backwards compatiblity) now handle
> > >> their
> > >> own stats.  Any library or app can register a callback function with
> > >> telemetry, which will be called if requested by the client connected
> > >> via
> > >> the telemetry socket.
> > >
> > > Great. Standardization across libraries is a good improvement.
> > >
> > >> The callback function in the library/app then
> > >> formats its stats, or other data, into a JSON string, and returns it to
> > >> telemetry to be sent to the client.
> > >
> > > I am strongly opposed to using JSON as the standard format in DPDK, and
> > instead prefer a binary format with zero (or minimal) type conversion.
> > >
> > > Here is one reason why I dislike JSON for this: A part of our application
> > samples 100k+ counters every second to be able to provide drill-down
> > statistics through the GUI. Converting these counters from uint64_t to JSON
> > and back to uint64_t for data processing is not going to fly. And I assume
> > that we are not the only company developing equipment with drill-down
> > statistics.
> > >
> > > I am aware that there is a difference between statistics for *drill-down
> > and data processing* purposes and statistics for *telemetry eyeball viewing
> > only* purposes, but the line is blurry, and I see a big risk of setting a
> > path that leads to JSON being used in places where it shouldn't.
> > >
> > > Here is another reason why I dislike JSON for this: JSON is fine for the
> > LAMP stack with REST protocols. But other systems use other protocols with
> > other formats, e.g. the TICK stack uses an even simpler text based format.
> > So DPDK based systems supporting the TICK stack will need to convert to
> > first JSON format (in the DPDK libraries), and then from JSON format to
> > InfluxDB format (in the DPDK application).
> > >
> > > I think that type conversion does not belong inside deep inside the DPDK
> > libraries, but is a job for the DPDK application. However, DPDK could
> > provide libraries for efficient bulk conversion to popular formats like
> > JSON. And other formats, if they are relevant, e.g. ASN.1 used by old
> > school SNMP.
> > 
> > I believe JSON has it place in this library and in DPDK as it is a good
> > conversion tool and easy to utilize with a huge number of tools/languages.
> 
> JSON is extremely heavy compared to a raw binary format.
> 
> It makes sense for low volume, hierarchical structured data, but not for large tables or arrays of counters.
> 
> > Binary output gets into endianness issues and a number of other problems,
> > so I would not want all of the data exported from DPDK to be in binary
> > format.
> 
> Endianness considerations are only relevant for data exchanged across the network; not data exchanged across processes inside the same machine.
> 
> And if you are exchanging data across the network, you would usually implement one or more well known protocols for that, e.g. JSON over HTTPS, or ASN.1 over SNMP, or InfluxDB over UDP. This means that the application needs to implement a protocol handler, which - in my opinion - should handle the relevant data type conversions from the raw format provided by DPDK.
> 
> I think it would be silly for DPDK core libraries to provide counters in JSON format, so an SNMP Agent would need to convert them from JSON back to binary and then to ASN.1.
> 
> > If the layout of the structure changes then the code would need to
> > know that on both side to be able to convert the data into the correct
> > values.
> 
> I may be exaggerating here, but trying to prove a point: This is what we have ABI stability for. Structures should be designed cleverly and future proof, e.g. like the ethdev xstats. Using text based APIs is a circumvention of ABI stability.
> 
> > 
> > With that stated, the new telemetry code allows the application to add new
> > commands and with that you can create a binary set of commands along side
> > the JSON or any other output format. With the new register command we can
> > create say a ‘/ethdevraw/stats,X’ set of commands that can emit binary
> > format.
> 
> That would be silly. The protocol handler should make the protocol specific conversion, not the driver! Again, going to the extreme to prove a point: If I understand you correctly, this would mean that PMDs would have provide counters in ASN.1 format for SNMP.
> 
> Our application provides a HTTPS/REST based communication interface for multiple purposes, e.g. getting tables of data. And if you want to get a table of some data via this interface, you can specify the output format in the request, so you can get it in e.g. TSV format (tabulator separated with a headline) for scripts, HTML format for human eyeballs. This data conversion happens at a common location, so we can easily add other output formats. You don't want to push this all the way down to the originator of the data.
> 
> > 
> > Using this method we get the best of both worlds and when using languages
> > like Go or Python to collect these stats we have a standard format for
> > conversion. In Go it is pretty hard to do binary conversion and JSON
> > conversion is just a few lines. 
> 
> I don't think DPDK should provide preferential treatment to Go or Python. DPDK is based on C, and should mainly cater for C.
> 
> > JSON may not be the fastest, but if you are
> > requesting stats faster than a second then use the raw commands to get the
> > data, which anyone can add to its application or we can add them to DPDK as
> > a standard command set.
> 
> APIs in the libraries are currently available to get data in raw format. My main concern is that libraries in the future will not provide functions to get raw data, like they do now, but only JSON formatted data for the telemetry library. This is what I want to avoid.
> 

Hi Morten,

thanks for all the feedback.

Firstly on the performance side, we did some basic benchmarking of the
output capabilities of the current proposal. Using a dummy client that had
two queries constantly outstanding (to avoid dead time between one response
and next request), we measured the number of replies per second from the
ethdev xstats call.
* with a 2.3 GHz Xeon system, we got 3,500 responses per second, with 146
  stats per response, giving a total of >500k stats per second encoded and
  transmitted.
* for those 500k stats, looking at the cpu time on the core doing the
  telemetry work, ~80% of CPU time was spent inside the ixgbe driver
  getting the stats from the NIC card itself.
* replacing the ixgbe NIC with a ring vdev increased the responses per
  second to >100k, though with only 13 stats per response this time, giving
  1.3M stats per second.
* Splitting the existing xstats call in this RFC into separate xstat
  names and xstat values calls (something I think is a good idea to do
  generally), and only calling the xstat values call each time, gives
  further perf increases to 163k responses per second.

Overall so, at least on the json generation side, it appears we can do
things rather quickly, though I admit that we did not look into the parsing
cost on the other side.

All that being said, however, I do understand your point about having
everything work internally in json - something that ties in with Thomas
concern about having flexibility. Therefore, while we will upstream a v3
very shortly with the few incremental comments on v2, we can thereafter
look to switch the internal communication between callbacks and telemetry
library to use a regular data structure, and then leave the json encoding
to the library itself just before output. That would:
a) allow the addition of other output types in the future without needing
   new callbacks.
b) allow more flexibility for integrating with a future one-IPC mechanism
   for DPDK.

I hope this option will resolve most concerns and allow for a 20.05
integration.

Regards.
/Bruce
  

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Monday, April 20, 2020 3:19 PM
> 
> On Fri, Apr 10, 2020 at 08:06:23PM +0200, Morten Brørup wrote:
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Keith
> > > Sent: Friday, April 10, 2020 4:22 PM
> > >
> > > > On Apr 10, 2020, at 5:49 AM, Morten Brørup
> <mb@smartsharesystems.com>
> > > wrote:
> > > >
> > > >> From: Ciara Power [mailto:ciara.power@intel.com]
> > > >> Sent: Wednesday, April 8, 2020 6:50 PM
> > > >>
> > > >> This patchset extensively reworks the telemetry library adding
> new
> > > >> functionality and simplifying much of the existing code, while
> > > >> maintaining backward compatibility.
> > > >>
> > > >> This work is based on the previously sent RFC for a "process
> info"
> > > >> library:
> https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> > > >> However, rather than creating a new library, this patchset takes
> > > >> that work and merges it into the existing telemetry library, as
> > > >> mentioned above.
> > > >>
> > > >> The telemetry library as shipped in 19.11 is based upon the
> metrics
> > > >> library and outputs all statistics based on that as a source.
> However,
> > > >> this limits the telemetry output to only port-level statistics
> > > >> information, rather than allowing it to be used as a general
> scheme for
> > > >> telemetry information across all DPDK libraries.
> > > >>
> > > >> With this patchset applied, rather than the telemetry library
> being
> > > >> responsible for pulling ethdev stats and pushing them into the
> metrics
> > > >> library for retrieval later, each library e.g. ethdev, rawdev,
> and even
> > > >> the metrics library itself (for backwards compatiblity) now
> handle
> > > >> their
> > > >> own stats.  Any library or app can register a callback function
> with
> > > >> telemetry, which will be called if requested by the client
> connected
> > > >> via
> > > >> the telemetry socket.
> > > >
> > > > Great. Standardization across libraries is a good improvement.
> > > >
> > > >> The callback function in the library/app then
> > > >> formats its stats, or other data, into a JSON string, and
> returns it to
> > > >> telemetry to be sent to the client.
> > > >
> > > > I am strongly opposed to using JSON as the standard format in
> DPDK, and
> > > instead prefer a binary format with zero (or minimal) type
> conversion.
> > > >
> > > > Here is one reason why I dislike JSON for this: A part of our
> application
> > > samples 100k+ counters every second to be able to provide drill-
> down
> > > statistics through the GUI. Converting these counters from uint64_t
> to JSON
> > > and back to uint64_t for data processing is not going to fly. And I
> assume
> > > that we are not the only company developing equipment with drill-
> down
> > > statistics.
> > > >
> > > > I am aware that there is a difference between statistics for
> *drill-down
> > > and data processing* purposes and statistics for *telemetry eyeball
> viewing
> > > only* purposes, but the line is blurry, and I see a big risk of
> setting a
> > > path that leads to JSON being used in places where it shouldn't.
> > > >
> > > > Here is another reason why I dislike JSON for this: JSON is fine
> for the
> > > LAMP stack with REST protocols. But other systems use other
> protocols with
> > > other formats, e.g. the TICK stack uses an even simpler text based
> format.
> > > So DPDK based systems supporting the TICK stack will need to
> convert to
> > > first JSON format (in the DPDK libraries), and then from JSON
> format to
> > > InfluxDB format (in the DPDK application).
> > > >
> > > > I think that type conversion does not belong inside deep inside
> the DPDK
> > > libraries, but is a job for the DPDK application. However, DPDK
> could
> > > provide libraries for efficient bulk conversion to popular formats
> like
> > > JSON. And other formats, if they are relevant, e.g. ASN.1 used by
> old
> > > school SNMP.
> > >
> > > I believe JSON has it place in this library and in DPDK as it is a
> good
> > > conversion tool and easy to utilize with a huge number of
> tools/languages.
> >
> > JSON is extremely heavy compared to a raw binary format.
> >
> > It makes sense for low volume, hierarchical structured data, but not
> for large tables or arrays of counters.
> >
> > > Binary output gets into endianness issues and a number of other
> problems,
> > > so I would not want all of the data exported from DPDK to be in
> binary
> > > format.
> >
> > Endianness considerations are only relevant for data exchanged across
> the network; not data exchanged across processes inside the same
> machine.
> >
> > And if you are exchanging data across the network, you would usually
> implement one or more well known protocols for that, e.g. JSON over
> HTTPS, or ASN.1 over SNMP, or InfluxDB over UDP. This means that the
> application needs to implement a protocol handler, which - in my
> opinion - should handle the relevant data type conversions from the raw
> format provided by DPDK.
> >
> > I think it would be silly for DPDK core libraries to provide counters
> in JSON format, so an SNMP Agent would need to convert them from JSON
> back to binary and then to ASN.1.
> >
> > > If the layout of the structure changes then the code would need to
> > > know that on both side to be able to convert the data into the
> correct
> > > values.
> >
> > I may be exaggerating here, but trying to prove a point: This is what
> we have ABI stability for. Structures should be designed cleverly and
> future proof, e.g. like the ethdev xstats. Using text based APIs is a
> circumvention of ABI stability.
> >
> > >
> > > With that stated, the new telemetry code allows the application to
> add new
> > > commands and with that you can create a binary set of commands
> along side
> > > the JSON or any other output format. With the new register command
> we can
> > > create say a ‘/ethdevraw/stats,X’ set of commands that can emit
> binary
> > > format.
> >
> > That would be silly. The protocol handler should make the protocol
> specific conversion, not the driver! Again, going to the extreme to
> prove a point: If I understand you correctly, this would mean that PMDs
> would have provide counters in ASN.1 format for SNMP.
> >
> > Our application provides a HTTPS/REST based communication interface
> for multiple purposes, e.g. getting tables of data. And if you want to
> get a table of some data via this interface, you can specify the output
> format in the request, so you can get it in e.g. TSV format (tabulator
> separated with a headline) for scripts, HTML format for human eyeballs.
> This data conversion happens at a common location, so we can easily add
> other output formats. You don't want to push this all the way down to
> the originator of the data.
> >
> > >
> > > Using this method we get the best of both worlds and when using
> languages
> > > like Go or Python to collect these stats we have a standard format
> for
> > > conversion. In Go it is pretty hard to do binary conversion and
> JSON
> > > conversion is just a few lines.
> >
> > I don't think DPDK should provide preferential treatment to Go or
> Python. DPDK is based on C, and should mainly cater for C.
> >
> > > JSON may not be the fastest, but if you are
> > > requesting stats faster than a second then use the raw commands to
> get the
> > > data, which anyone can add to its application or we can add them to
> DPDK as
> > > a standard command set.
> >
> > APIs in the libraries are currently available to get data in raw
> format. My main concern is that libraries in the future will not
> provide functions to get raw data, like they do now, but only JSON
> formatted data for the telemetry library. This is what I want to avoid.
> >
> 
> Hi Morten,
> 
> thanks for all the feedback.
> 
> Firstly on the performance side, we did some basic benchmarking of the
> output capabilities of the current proposal. Using a dummy client that
> had
> two queries constantly outstanding (to avoid dead time between one
> response
> and next request), we measured the number of replies per second from
> the
> ethdev xstats call.
> * with a 2.3 GHz Xeon system, we got 3,500 responses per second, with
> 146
>   stats per response, giving a total of >500k stats per second encoded
> and
>   transmitted.
> * for those 500k stats, looking at the cpu time on the core doing the
>   telemetry work, ~80% of CPU time was spent inside the ixgbe driver
>   getting the stats from the NIC card itself.
> * replacing the ixgbe NIC with a ring vdev increased the responses per
>   second to >100k, though with only 13 stats per response this time,
> giving
>   1.3M stats per second.
> * Splitting the existing xstats call in this RFC into separate xstat
>   names and xstat values calls (something I think is a good idea to do
>   generally), and only calling the xstat values call each time, gives
>   further perf increases to 163k responses per second.
> 
> Overall so, at least on the json generation side, it appears we can do
> things rather quickly, though I admit that we did not look into the
> parsing
> cost on the other side.
> 
> All that being said, however, I do understand your point about having
> everything work internally in json - something that ties in with Thomas
> concern about having flexibility. Therefore, while we will upstream a
> v3
> very shortly with the few incremental comments on v2, we can thereafter
> look to switch the internal communication between callbacks and
> telemetry
> library to use a regular data structure, and then leave the json
> encoding
> to the library itself just before output. That would:
> a) allow the addition of other output types in the future without
> needing
>    new callbacks.
> b) allow more flexibility for integrating with a future one-IPC
> mechanism
>    for DPDK.
> 
> I hope this option will resolve most concerns and allow for a 20.05
> integration.
> 
> Regards.
> /Bruce

Yes. Thank you.

Med venlig hilsen / kind regards
- Morten Brørup
  

On Thu, 2020-04-09 at 11:37 +0200, Thomas Monjalon wrote:
> 09/04/2020 11:19, Bruce Richardson:
> > On Wed, Apr 08, 2020 at 08:03:26PM +0200, Thomas Monjalon wrote:
> > > 08/04/2020 18:49, Ciara Power:
> > > > This patchset extensively reworks the telemetry library adding new
> > > > functionality and simplifying much of the existing code, while
> > > > maintaining backward compatibility.
> > > > 
> > > > This work is based on the previously sent RFC for a "process info"
> > > > library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> > > > However, rather than creating a new library, this patchset takes
> > > > that work and merges it into the existing telemetry library, as
> > > > mentioned above.
> > > > 
> > > > The telemetry library as shipped in 19.11 is based upon the metrics
> > > > library and outputs all statistics based on that as a source. However,
> > > > this limits the telemetry output to only port-level statistics
> > > > information, rather than allowing it to be used as a general scheme for
> > > > telemetry information across all DPDK libraries.
> > > > 
> > > > With this patchset applied, rather than the telemetry library being
> > > > responsible for pulling ethdev stats and pushing them into the metrics
> > > > library for retrieval later, each library e.g. ethdev, rawdev, and even
> > > > the metrics library itself (for backwards compatiblity) now handle their
> > > > own stats.  Any library or app can register a callback function with
> > > > telemetry, which will be called if requested by the client connected via
> > > > the telemetry socket. The callback function in the library/app then
> > > > formats its stats, or other data, into a JSON string, and returns it to
> > > > telemetry to be sent to the client.
> > > 
> > > I think this is a global need in DPDK, and it is usually called RPC,
> > > IPC or control messaging.
> > > We had a similar need for multi-process communication, thus rte_mp IPC.
> > > We also need a control channel for user configuration applications.
> > > We also need to control some features like logging or tracing.
> > > 
> > > In my opinion, it is time to introduce a general control channel in DPDK.
> > > The application must be in the loop of the control mechanism.
> > > Making such channel standard will ease application adoption.
> > > 
> > > Please read some comments here:
> > > http://inbox.dpdk.org/dev/2580933.jp2sp48Hzj@xps/
> > > 
> > Hi Thomas,
> > 
> > I agree that having a single control mechanism or messaging mechanism in
> > DPDK would be nice to have. However, I don't believe the plans for such a
> > scheme should impact this patchset right now as the idea of a common
> > channel was only first mooted about a week ago, and while there has been
> > some email discussion about it, there is as yet no requirements list that
> > I've seen, nobody actually doing coding work on it, no rfc and most
> > importantly no timeline for creating and merging such into DPDK.
> 
> Yes, this is a new idea.
> Throwing the idea in this "telemetry" thread and in "IF proxy" thread
> is the first step before starting a dedicated thread to design
> a generic mechanism.

May I offer the services of https://zeromq.org/ ?
  

23/04/2020 12:30, Luca Boccassi:
> On Thu, 2020-04-09 at 11:37 +0200, Thomas Monjalon wrote:
> > 09/04/2020 11:19, Bruce Richardson:
> > > On Wed, Apr 08, 2020 at 08:03:26PM +0200, Thomas Monjalon wrote:
> > > > 08/04/2020 18:49, Ciara Power:
> > > > > This patchset extensively reworks the telemetry library adding new
> > > > > functionality and simplifying much of the existing code, while
> > > > > maintaining backward compatibility.
> > > > > 
> > > > > This work is based on the previously sent RFC for a "process info"
> > > > > library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> > > > > However, rather than creating a new library, this patchset takes
> > > > > that work and merges it into the existing telemetry library, as
> > > > > mentioned above.
> > > > > 
> > > > > The telemetry library as shipped in 19.11 is based upon the metrics
> > > > > library and outputs all statistics based on that as a source. However,
> > > > > this limits the telemetry output to only port-level statistics
> > > > > information, rather than allowing it to be used as a general scheme for
> > > > > telemetry information across all DPDK libraries.
> > > > > 
> > > > > With this patchset applied, rather than the telemetry library being
> > > > > responsible for pulling ethdev stats and pushing them into the metrics
> > > > > library for retrieval later, each library e.g. ethdev, rawdev, and even
> > > > > the metrics library itself (for backwards compatiblity) now handle their
> > > > > own stats.  Any library or app can register a callback function with
> > > > > telemetry, which will be called if requested by the client connected via
> > > > > the telemetry socket. The callback function in the library/app then
> > > > > formats its stats, or other data, into a JSON string, and returns it to
> > > > > telemetry to be sent to the client.
> > > > 
> > > > I think this is a global need in DPDK, and it is usually called RPC,
> > > > IPC or control messaging.
> > > > We had a similar need for multi-process communication, thus rte_mp IPC.
> > > > We also need a control channel for user configuration applications.
> > > > We also need to control some features like logging or tracing.
> > > > 
> > > > In my opinion, it is time to introduce a general control channel in DPDK.
> > > > The application must be in the loop of the control mechanism.
> > > > Making such channel standard will ease application adoption.
> > > > 
> > > > Please read some comments here:
> > > > http://inbox.dpdk.org/dev/2580933.jp2sp48Hzj@xps/
> > > > 
> > > Hi Thomas,
> > > 
> > > I agree that having a single control mechanism or messaging mechanism in
> > > DPDK would be nice to have. However, I don't believe the plans for such a
> > > scheme should impact this patchset right now as the idea of a common
> > > channel was only first mooted about a week ago, and while there has been
> > > some email discussion about it, there is as yet no requirements list that
> > > I've seen, nobody actually doing coding work on it, no rfc and most
> > > importantly no timeline for creating and merging such into DPDK.
> > 
> > Yes, this is a new idea.
> > Throwing the idea in this "telemetry" thread and in "IF proxy" thread
> > is the first step before starting a dedicated thread to design
> > a generic mechanism.
> 
> May I offer the services of https://zeromq.org/ ?

This is what I already proposed:
http://inbox.dpdk.org/dev/20334513.huCnfhLgOn@xps/

I'm sorry, I was supposed to start a new thread for this discussion.
I will summarize my thoughts and discussions just after -rc1 is done.
  

On Thu, 2020-04-23 at 12:44 +0200, Thomas Monjalon wrote:
> 23/04/2020 12:30, Luca Boccassi:
> > On Thu, 2020-04-09 at 11:37 +0200, Thomas Monjalon wrote:
> > > 09/04/2020 11:19, Bruce Richardson:
> > > > On Wed, Apr 08, 2020 at 08:03:26PM +0200, Thomas Monjalon wrote:
> > > > > 08/04/2020 18:49, Ciara Power:
> > > > > > This patchset extensively reworks the telemetry library adding new
> > > > > > functionality and simplifying much of the existing code, while
> > > > > > maintaining backward compatibility.
> > > > > > 
> > > > > > This work is based on the previously sent RFC for a "process info"
> > > > > > library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741
> > > > > > However, rather than creating a new library, this patchset takes
> > > > > > that work and merges it into the existing telemetry library, as
> > > > > > mentioned above.
> > > > > > 
> > > > > > The telemetry library as shipped in 19.11 is based upon the metrics
> > > > > > library and outputs all statistics based on that as a source. However,
> > > > > > this limits the telemetry output to only port-level statistics
> > > > > > information, rather than allowing it to be used as a general scheme for
> > > > > > telemetry information across all DPDK libraries.
> > > > > > 
> > > > > > With this patchset applied, rather than the telemetry library being
> > > > > > responsible for pulling ethdev stats and pushing them into the metrics
> > > > > > library for retrieval later, each library e.g. ethdev, rawdev, and even
> > > > > > the metrics library itself (for backwards compatiblity) now handle their
> > > > > > own stats.  Any library or app can register a callback function with
> > > > > > telemetry, which will be called if requested by the client connected via
> > > > > > the telemetry socket. The callback function in the library/app then
> > > > > > formats its stats, or other data, into a JSON string, and returns it to
> > > > > > telemetry to be sent to the client.
> > > > > 
> > > > > I think this is a global need in DPDK, and it is usually called RPC,
> > > > > IPC or control messaging.
> > > > > We had a similar need for multi-process communication, thus rte_mp IPC.
> > > > > We also need a control channel for user configuration applications.
> > > > > We also need to control some features like logging or tracing.
> > > > > 
> > > > > In my opinion, it is time to introduce a general control channel in DPDK.
> > > > > The application must be in the loop of the control mechanism.
> > > > > Making such channel standard will ease application adoption.
> > > > > 
> > > > > Please read some comments here:
> > > > > http://inbox.dpdk.org/dev/2580933.jp2sp48Hzj@xps/
> > > > > 
> > > > Hi Thomas,
> > > > 
> > > > I agree that having a single control mechanism or messaging mechanism in
> > > > DPDK would be nice to have. However, I don't believe the plans for such a
> > > > scheme should impact this patchset right now as the idea of a common
> > > > channel was only first mooted about a week ago, and while there has been
> > > > some email discussion about it, there is as yet no requirements list that
> > > > I've seen, nobody actually doing coding work on it, no rfc and most
> > > > importantly no timeline for creating and merging such into DPDK.
> > > 
> > > Yes, this is a new idea.
> > > Throwing the idea in this "telemetry" thread and in "IF proxy" thread
> > > is the first step before starting a dedicated thread to design
> > > a generic mechanism.
> > 
> > May I offer the services of https://zeromq.org/ ?
> 
> This is what I already proposed:
> http://inbox.dpdk.org/dev/20334513.huCnfhLgOn@xps/
> 
> I'm sorry, I was supposed to start a new thread for this discussion.
> I will summarize my thoughts and discussions just after -rc1 is done.

Ah! They say great minds think alike :-P