DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Kevin Laatz" <kevin.laatz@intel.com>,
	"Robin Jarry" <rjarry@redhat.com>, <dev@dpdk.org>
Cc: "Tyler Retzlaff" <roretzla@linux.microsoft.com>,
	"Ciara Power" <ciara.power@intel.com>
Subject: RE: [PATCH v5 1/4] eal: add lcore info in telemetry
Date: Wed, 18 Jan 2023 12:35:30 +0100	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87687@smartserver.smartshare.dk> (raw)
In-Reply-To: <45b9e94c-da88-5e03-5cac-66015abfdc49@intel.com>

> From: Kevin Laatz [mailto:kevin.laatz@intel.com]
> Sent: Wednesday, 18 January 2023 12.03
> 
> On 18/01/2023 10:21, Morten Brørup wrote:
> >> From: Kevin Laatz [mailto:kevin.laatz@intel.com]
> >>
> >> On 16/12/2022 10:21, Robin Jarry wrote:
> >>> Report the same information than rte_lcore_dump() in the telemetry
> >>> API into /eal/lcore/list and /eal/lcore/info,ID.
> >>>
> >>> Example:
> >>>
> >>>     --> /eal/lcore/info,3
> >>>     {
> >>>       "/eal/lcore/info": {
> >>>         "lcore_id": 3,
> >>>         "socket": 0,
> >>>         "role": "RTE",
> >>>         "cpuset": [
> >>>           3
> >>>         ]
> >>>       }
> >>>     }
> >>>
> >>> Signed-off-by: Robin Jarry <rjarry@redhat.com>
> >>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >>> ---
> >
> >> Hi Robin,
> >>
> >> Thanks for taking the time to work on this. It is a good
> implementation
> >> for debug use-cases.
> >>
> >> I have 2 suggestions which would improve the usability of the data:
> >> 1. Could we make the lcore_id paramater on /eal/lcore/info optional?
> >> This would allow users to read info for all lcores in the
> application
> >> at
> >> once.
> > +1 to this suggestion.
> >
> >> 2. Could we add 2 additional telemetry endpoints? One which returns
> an
> >> array of busy_cycles values and the other returns an array of
> >> total_cycles values. These arrays could be used in conjunction with
> the
> >> /eal/lcore/list endpoint to quickly read the usage related metrics.
> >> I've
> >> included an example diff below [1].
> > I prefer this done in a more generic way, see below.
> >
> >> We have a use-case beyond debugging in which we read telemetry every
> >> few
> >> milliseconds. From a performance point of view, adding the 2
> additional
> >> endpoints would be very beneficial.
> >>
> >> Thanks,
> >> Kevin
> >>
> >> [1]
> >>
> >> diff --git a/lib/eal/common/eal_common_lcore.c
> >> b/lib/eal/common/eal_common_lcore.c
> >> index 210636d21d..94ddb276c5 100644
> >> --- a/lib/eal/common/eal_common_lcore.c
> >> +++ b/lib/eal/common/eal_common_lcore.c
> >> @@ -569,6 +569,32 @@ handle_lcore_info(const char *cmd __rte_unused,
> >> const char *params, struct rte_t
> >>           return rte_lcore_iterate(lcore_telemetry_info_cb, &info);
> >>    }
> >>
> >> +static int
> >> +lcore_telemetry_busy_cycles_cb(unsigned int lcore_id, void *arg)
> >> +{
> >> +       struct rte_tel_data *d = arg;
> >> +       struct rte_lcore_usage usage;
> >> +       rte_lcore_usage_cb usage_cb;
> >> +       unsigned long cycles = 0;
> >> +
> >> +       memset(&usage, 0, sizeof(usage));
> >> +       usage_cb = lcore_usage_cb;
> >> +       if (usage_cb != NULL && usage_cb(lcore_id, &usage) == 0)
> >> +               cycles = usage.busy_cycles;
> >> +
> >> +       return rte_tel_data_add_array_u64(d, cycles);
> >> +}
> >> +
> >> +static int
> >> +handle_lcore_busy_cycles(const char *cmd __rte_unused,
> >> +               const char *params __rte_unused, struct rte_tel_data
> >> *d)
> >> +{
> >> +       int ret = rte_tel_data_start_array(d, RTE_TEL_U64_VAL);
> >> +       if (ret)
> >> +               return ret;
> >> +       return rte_lcore_iterate(lcore_telemetry_busy_cycles_cb, d);
> >> +}
> >> +
> >>    RTE_INIT(lcore_telemetry)
> >>    {
> >>           rte_telemetry_register_cmd(
> >> @@ -577,5 +603,8 @@ RTE_INIT(lcore_telemetry)
> >>           rte_telemetry_register_cmd(
> >>                           "/eal/lcore/info", handle_lcore_info,
> >>                           "Returns lcore info. Parameters: int
> >> lcore_id");
> >> +       rte_telemetry_register_cmd(
> >> +                       "/eal/lcore/busy_cycles",
> >> handle_lcore_busy_cycles,
> >> +                       "List of busy cycle values. Takes no
> >> parameters");
> >>    }
> >>    #endif /* !RTE_EXEC_ENV_WINDOWS */
> > This should be generalized to support any named field in the
> rte_lcore_usage structure.
> >
> > The general path could be: /eal/lcore/usage
> >
> > With optional parameter lcore_id. This should return one object (or
> an array of such objects, if lcore_id is not given) with all usage
> fields and their values, e.g.:
> >
> > {
> > "lcore_id": 7,
> > "total_cycles": 1234,
> > "usage_cycles": 567
> > }
> >
> >
> > The paths to support the array-optimized feature you are requesting
> could be: /eal/lcores/usage/total_cycles and
> /eal/lcores/usage/usage_cycles.
> >
> > These paths should return the arrays as suggested. I only request
> that you change "/lcore" to plural "/lcores" and add "/usage" to the
> path before the field name in the usage table.
> >
> > Alternatively, you could add a path /eal/lcores/usage_array, taking
> the field names as parameters and outputting multiple arrays like this:
> >
> > /eal/lcores/usage_array,total_cycles,usage_cycles
> >
> > {

I forgot the index array:

"lcore_id": [1,6,7],

> > "total_cycles": [1234, 1234, 1234],
> > "usage_cycles": [567, 678, 789]
> > }
> 
> +1, this would also work nicely and allows for extension in future
> without flooding with endpoints.

Our applications do something somewhat similar, i.e. use JSON arrays for performance (and bandwidth conservation) reasons - so I can confirm that the concept works in production.

But come to think of it, your array suggestion seems more like an additional feature, which belongs in another patch. We should not hold back Robin's patch series due to feature creep. :-)

> 
> 
> >
> > But I don't know if this breaks with DPDK's standard REST interface.
> It would be easier if we had decided on something like OData, instead
> of inventing our own.
> >
> >


  reply	other threads:[~2023-01-18 11:35 UTC|newest]

Thread overview: 132+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-23 10:26 [RFC PATCH 0/4] lcore telemetry improvements Robin Jarry
2022-11-23 10:26 ` [RFC PATCH 1/4] eal: add lcore info in telemetry Robin Jarry
2022-11-23 16:44   ` Stephen Hemminger
2022-11-23 23:15     ` Robin Jarry
2022-11-23 10:26 ` [RFC PATCH 2/4] eal: allow applications to report their cpu utilization Robin Jarry
2022-11-23 10:26 ` [RFC PATCH 3/4] testpmd: add show lcores command Robin Jarry
2022-11-23 10:26 ` [RFC PATCH 4/4] testpmd: report lcore usage Robin Jarry
2022-11-28  8:59 ` [PATCH v2 0/4] lcore telemetry improvements Robin Jarry
2022-11-28  8:59   ` [PATCH v2 1/4] eal: add lcore info in telemetry Robin Jarry
2022-11-28  8:59   ` [PATCH v2 2/4] eal: allow applications to report their cpu cycles utilization Robin Jarry
2022-11-28 10:52     ` Morten Brørup
2022-11-29  8:19       ` Robin Jarry
2022-11-28  8:59   ` [PATCH v2 3/4] testpmd: add dump_lcores command Robin Jarry
2022-11-28  8:59   ` [PATCH v2 4/4] testpmd: report lcore usage Robin Jarry
2022-11-29 15:33 ` [PATCH v3 0/4] lcore telemetry improvements Robin Jarry
2022-11-29 15:33   ` [PATCH v3 1/4] eal: add lcore info in telemetry Robin Jarry
2022-11-29 15:33   ` [PATCH v3 2/4] eal: allow applications to report their cpu cycles usage Robin Jarry
2022-11-29 16:10     ` Mattias Rönnblom
2022-12-07 11:00       ` Robin Jarry
2022-12-07 11:21         ` Morten Brørup
2022-11-29 15:33   ` [PATCH v3 3/4] testpmd: add dump_lcores command Robin Jarry
2022-11-29 15:33   ` [PATCH v3 4/4] testpmd: report lcore usage Robin Jarry
2022-11-29 16:14   ` [PATCH v3 0/4] lcore telemetry improvements Mattias Rönnblom
2022-12-07 16:21 ` [PATCH " Robin Jarry
2022-12-07 16:21   ` [PATCH 1/4] eal: add lcore info in telemetry Robin Jarry
2022-12-07 16:21   ` [PATCH 2/4] eal: allow applications to report their cpu usage Robin Jarry
2022-12-13 15:49     ` Robin Jarry
2022-12-13 16:39       ` Morten Brørup
2022-12-13 17:45       ` Tyler Retzlaff
2022-12-07 16:21   ` [PATCH 3/4] testpmd: add dump_lcores command Robin Jarry
2022-12-07 16:21   ` [PATCH 4/4] testpmd: report lcore usage Robin Jarry
2022-12-16 10:21 ` [PATCH v5 0/4] lcore telemetry improvements Robin Jarry
2022-12-16 10:21   ` [PATCH v5 1/4] eal: add lcore info in telemetry Robin Jarry
2023-01-18  9:42     ` Kevin Laatz
2023-01-18 10:21       ` Morten Brørup
2023-01-18 11:03         ` Kevin Laatz
2023-01-18 11:35           ` Morten Brørup [this message]
2023-01-18 14:45       ` Robin Jarry
2023-01-18 16:01         ` Kevin Laatz
2023-01-18 16:17           ` Robin Jarry
2022-12-16 10:21   ` [PATCH v5 2/4] eal: allow applications to report their cpu usage Robin Jarry
2022-12-16 10:47     ` Morten Brørup
2023-01-04 10:13       ` Robin Jarry
2023-01-04 10:28         ` Morten Brørup
2022-12-22 12:41     ` Konstantin Ananyev
2023-01-04 10:10       ` Robin Jarry
2023-01-04 10:53         ` Konstantin Ananyev
2023-01-18 16:46           ` Robin Jarry
2023-02-06 20:07             ` Konstantin Ananyev
2023-02-06 20:29               ` Robin Jarry
2023-02-06 20:34                 ` Konstantin Ananyev
2023-02-06 20:39                   ` Robin Jarry
2023-02-06 20:44                     ` Konstantin Ananyev
2023-02-06 20:55                       ` Robin Jarry
2023-02-07 13:12                         ` Konstantin Ananyev
2023-01-04 10:15     ` Robin Jarry
2022-12-16 10:21   ` [PATCH v5 3/4] testpmd: add dump_lcores command Robin Jarry
2022-12-22 12:43     ` Konstantin Ananyev
2022-12-16 10:21   ` [PATCH v5 4/4] testpmd: report lcore usage Robin Jarry
2022-12-22 12:44     ` Konstantin Ananyev
2023-01-18  9:13   ` [PATCH v5 0/4] lcore telemetry improvements Robin Jarry
2023-01-19 15:06 ` [PATCH v6 0/5] " Robin Jarry
2023-01-19 15:06   ` [PATCH v6 1/5] eal: add lcore info in telemetry Robin Jarry
2023-01-19 19:42     ` Kevin Laatz
2023-01-26 11:19     ` David Marchand
2023-01-19 15:06   ` [PATCH v6 2/5] eal: allow applications to report their cpu usage Robin Jarry
2023-01-19 19:42     ` Kevin Laatz
2023-01-26 11:22     ` David Marchand
2023-01-19 15:06   ` [PATCH v6 3/5] testpmd: add dump_lcores command Robin Jarry
2023-01-19 19:42     ` Kevin Laatz
2023-01-26 11:22     ` David Marchand
2023-01-19 15:06   ` [PATCH v6 4/5] testpmd: report lcore usage Robin Jarry
2023-01-19 19:42     ` Kevin Laatz
2023-01-19 15:06   ` [PATCH v6 5/5] telemetry: add /eal/lcore/usage endpoint Robin Jarry
2023-01-19 16:21     ` Morten Brørup
2023-01-19 16:34       ` Robin Jarry
2023-01-19 16:45         ` Morten Brørup
2023-01-19 19:42     ` Kevin Laatz
2023-02-02 13:43 ` [PATCH v8 0/5] lcore telemetry improvements Robin Jarry
2023-02-02 13:43   ` [PATCH v8 1/5] eal: add lcore info in telemetry Robin Jarry
2023-02-06  3:50     ` fengchengwen
2023-02-06  8:22       ` Robin Jarry
2023-02-06 11:22         ` fengchengwen
2023-02-06 11:46           ` Robin Jarry
2023-02-06 12:08             ` fengchengwen
2023-02-02 13:43   ` [PATCH v8 2/5] eal: report applications lcore usage Robin Jarry
2023-02-06  4:00     ` fengchengwen
2023-02-06  7:36       ` Morten Brørup
2023-02-06  8:21         ` Robin Jarry
2023-02-06 11:18         ` fengchengwen
2023-02-06  8:48     ` David Marchand
2023-02-06  9:03       ` Robin Jarry
2023-02-02 13:43   ` [PATCH v8 3/5] app/testpmd: add dump command for lcores Robin Jarry
2023-02-06  3:34     ` fengchengwen
2023-02-02 13:43   ` [PATCH v8 4/5] app/testpmd: report lcore usage Robin Jarry
2023-02-06  3:31     ` fengchengwen
2023-02-06  8:58     ` David Marchand
2023-02-06  9:08       ` Robin Jarry
2023-02-06 15:06         ` David Marchand
2023-02-02 13:43   ` [PATCH v8 5/5] eal: add lcore usage telemetry endpoint Robin Jarry
2023-02-02 14:00     ` Morten Brørup
2023-02-06  3:27     ` fengchengwen
2023-02-06  8:24       ` Robin Jarry
2023-02-06 11:32         ` fengchengwen
2023-02-05 23:11   ` [PATCH v8 0/5] lcore telemetry improvements Thomas Monjalon
2023-02-07 19:37 ` [PATCH v9 " Robin Jarry
2023-02-07 19:37 ` [PATCH v9 1/5] eal: add lcore info in telemetry Robin Jarry
2023-02-08  2:24   ` lihuisong (C)
2023-02-08 17:04     ` Robin Jarry
2023-02-09  2:18       ` lihuisong (C)
2023-02-09  8:31         ` David Marchand
2023-02-09  8:38           ` David Marchand
2023-02-07 19:37 ` [PATCH v9 2/5] eal: report applications lcore usage Robin Jarry
2023-02-07 19:37 ` [PATCH v9 3/5] app/testpmd: add dump command for lcores Robin Jarry
2023-02-07 19:37 ` [PATCH v9 4/5] app/testpmd: report lcore usage Robin Jarry
2023-02-08  2:59   ` lihuisong (C)
2023-02-07 19:37 ` [PATCH v9 5/5] eal: add lcore usage telemetry endpoint Robin Jarry
2023-02-08  8:45 ` [RESEND PATCH v9 0/5] lcore telemetry improvements Robin Jarry
2023-02-08  8:45   ` [RESEND PATCH v9 1/5] eal: add lcore info in telemetry Robin Jarry
2023-02-08  8:45   ` [RESEND PATCH v9 2/5] eal: report applications lcore usage Robin Jarry
2023-02-08  8:45   ` [RESEND PATCH v9 3/5] app/testpmd: add dump command for lcores Robin Jarry
2023-02-08  8:45   ` [RESEND PATCH v9 4/5] app/testpmd: report lcore usage Robin Jarry
2023-02-09  8:43     ` David Marchand
2023-02-08  8:45   ` [RESEND PATCH v9 5/5] eal: add lcore usage telemetry endpoint Robin Jarry
2023-02-09  8:44   ` [RESEND PATCH v9 0/5] lcore telemetry improvements David Marchand
2023-02-09  9:43 ` [PATCH v10 " Robin Jarry
2023-02-09  9:43   ` [PATCH v10 1/5] eal: add lcore info in telemetry Robin Jarry
2023-02-09  9:43   ` [PATCH v10 2/5] eal: report applications lcore usage Robin Jarry
2023-02-09  9:43   ` [PATCH v10 3/5] app/testpmd: add dump command for lcores Robin Jarry
2023-02-09  9:43   ` [PATCH v10 4/5] app/testpmd: report lcore usage Robin Jarry
2023-02-09  9:43   ` [PATCH v10 5/5] eal: add lcore usage telemetry endpoint Robin Jarry
2023-02-10 13:27   ` [PATCH v10 0/5] lcore telemetry improvements David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=98CBD80474FA8B44BF855DF32C47DC35D87687@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=ciara.power@intel.com \
    --cc=dev@dpdk.org \
    --cc=kevin.laatz@intel.com \
    --cc=rjarry@redhat.com \
    --cc=roretzla@linux.microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).