RE: [EXT] Re: [RFC 0/3] Introduce event link profiles

[Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>]

> -----Original Message-----
> From: Mattias Rönnblom <hofors@lysator.liu.se>
> Sent: Saturday, August 12, 2023 11:23 AM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; Shijith Thotton
> <sthotton@marvell.com>; timothy.mcdaniel@intel.com;
> hemant.agrawal@nxp.com; sachin.saxena@nxp.com;
> mattias.ronnblom@ericsson.com; liangma@liangbit.com;
> peter.mccarthy@intel.com; harry.van.haaren@intel.com;
> erik.g.carrillo@intel.com; abhinandan.gujjar@intel.com;
> s.v.naga.harish.k@intel.com; anatoly.burakov@intel.com
> Cc: dev@dpdk.org
> Subject: Re: [EXT] Re: [RFC 0/3] Introduce event link profiles
> 
> On 2023-08-10 07:17, Pavan Nikhilesh Bhagavatula wrote:
> >> On 2023-08-09 16:26, pbhagavatula@marvell.com wrote:
> >>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>
> >>> A collection of event queues linked to an event port can be associated
> >>> with unique identifier called as a profile, multiple such profiles can
> >>> be configured based on the event device capability using the function
> >>> `rte_event_port_link_with_profile` which takes arguments similar to
> >>> `rte_event_port_link` in addition to the profile identifier.
> >>>
> >>
> >> What is the overall goal with this new API? What problems does it intend
> >> to solve, that the old one doesn't.
> >
> > Linking and unlinking currently has huge overhead and when it needs to be
> done
> > in fastpath, we have to wait for unlinks to complete and handle other
> corner cases.
> >
> 
> OK, so this API change is specific to some particular hardware? Is this
> true for some other event devices? That "huge overhead" goes to "simple
> function call" for unlinking+linking, provided the target configuration
> is known in advance.

CNXK supports this feature in HW as an optional feature. 
Drivers can return -ENOSUP or set info::max_profiles_per_port to 1 if the 
feature cannot be implemented or decided to not implement this.
Although, I believe it can be easily integrated into other SW eventdevices.

> 
> What is the overall use case?
> 

One of the primary use case that our customers are interested is to modify the
worker ratio on the fly without the overhead of unlink/relink and also to make
sure low-priority queues are at least scheduled once in a while without worrying
about affinities and weights.

> > This patch set solves it by avoiding linking/unlinking altogether in fastpath
> by
> > preconfigured set of link profiles out of which only one would be active and
> can
> > be changed in fastpath with a simple function call. There is no link/unlink
> waiting for
> > unlink overhead.
> >
> >>
> >>> The maximum link profiles that are supported by an event device is
> >>> advertised through the structure member
> >>> `rte_event_dev_info::max_profiles_per_port`.
> >>>
> >>> By default, event ports are configured to use the link profile 0 on
> >>> initialization.
> >>>
> >>> Once multiple link profiles are set up and the event device is started, the
> >>> application can use the function `rte_event_port_change_profile` to
> >> change
> >>> the currently active profile on an event port. This effects the next
> >>> `rte_event_dequeue_burst` call, where the event queues associated
> with
> >> the
> >>> newly active link profile will participate in scheduling.
> >>>
> >>> Rudementary work flow would something like:
> >>>
> >>> Config path:
> >>>
> >>>       uint8_t lowQ[4] = {4, 5, 6, 7};
> >>>       uint8_t highQ[4] = {0, 1, 2, 3};
> >>>
> >>>       if (rte_event_dev_info.max_profiles_per_port < 2)
> >>>           return -ENOTSUP;
> >>>
> >>>       rte_event_port_link_with_profile(0, 0, highQ, NULL, 4, 0);
> >>>       rte_event_port_link_with_profile(0, 0, lowQ, NULL, 4, 1);
> >>>
> >>> Worker path:
> >>>
> >>>       empty_high_deq = 0;
> >>>       empty_low_deq = 0;
> >>>       is_low_deq = 0;
> >>>       while (1) {
> >>>           deq = rte_event_dequeue_burst(0, 0, &ev, 1, 0);
> >>>           if (deq == 0) {
> >>>               /**
> >>>                * Change link profile based on work activity on current
> >>>                * active profile
> >>>                */
> >>>               if (is_low_deq) {
> >>>                   empty_low_deq++;
> >>>                   if (empty_low_deq == MAX_LOW_RETRY) {
> >>>                       rte_event_port_change_profile(0, 0, 0);
> >>>                       is_low_deq = 0;
> >>>                       empty_low_deq = 0;
> >>>                   }
> >>>                   continue;
> >>>               }
> >>>
> >>>               if (empty_high_deq == MAX_HIGH_RETRY) {
> >>>                   rte_event_port_change_profile(0, 0, 1);
> >>>                   is_low_deq = 1;
> >>>                   empty_high_deq = 0;
> >>>               }
> >>>               continue;
> >>>           }
> >>>
> >>>           // Process the event received.
> >>>
> >>>           if (is_low_deq++ == MAX_LOW_EVENTS) {
> >>>               rte_event_port_change_profile(0, 0, 0);
> >>>               is_low_deq = 0;
> >>>           }
> >>>       }
> >>>
> >>
> >> This thing looks like the application is asked to do work scheduling.
> >> That doesn't sound right. That's the job of the work scheduler (i.e.,
> >> the event device).
> >>
> >> If this thing is merely a matter of changing what queues are linked to
> >> which ports, wouldn't a new call:
> >> rte_event_port_link_modify()
> >> suffice?
> >
> >
> > Some applications divide their available lcores into multiple types of
> > workers which each work on a unique set of event queues, application
> might
> > need to modify the worker ratio based on various parameters at run time
> > without a lot of overhead.
> >
> > Modifying links wouldn’t work because we might want to restore previous
> links
> > based on the new traffic pattern etc.,.
> >
> >>
> >>> An application could use heuristic data of load/activity of a given event
> >>> port and change its active profile to adapt to the traffic pattern.
> >>>
> >>> An unlink function `rte_event_port_unlink_with_profile` is provided to
> >>> modify the links associated to a profile, and
> >>> `rte_event_port_links_get_with_profile` can be used to retrieve the
> links
> >>> associated with a profile.
> >>>
> >>> Pavan Nikhilesh (3):
> >>>     eventdev: introduce link profiles
> >>>     event/cnxk: implement event link profiles
> >>>     test/event: add event link profile test
> >>>
> >>>    app/test/test_eventdev.c                   | 110 ++++++++++
> >>>    config/rte_config.h                        |   1 +
> >>>    doc/guides/eventdevs/cnxk.rst              |   1 +
> >>>    doc/guides/prog_guide/eventdev.rst         |  58 ++++++
> >>>    drivers/common/cnxk/roc_nix_inl_dev.c      |   4 +-
> >>>    drivers/common/cnxk/roc_sso.c              |  18 +-
> >>>    drivers/common/cnxk/roc_sso.h              |   8 +-
> >>>    drivers/common/cnxk/roc_sso_priv.h         |   4 +-
> >>>    drivers/event/cnxk/cn10k_eventdev.c        |  45 ++--
> >>>    drivers/event/cnxk/cn10k_worker.c          |  11 +
> >>>    drivers/event/cnxk/cn10k_worker.h          |   1 +
> >>>    drivers/event/cnxk/cn9k_eventdev.c         |  72 ++++---
> >>>    drivers/event/cnxk/cn9k_worker.c           |  22 ++
> >>>    drivers/event/cnxk/cn9k_worker.h           |   2 +
> >>>    drivers/event/cnxk/cnxk_eventdev.c         |  34 ++--
> >>>    drivers/event/cnxk/cnxk_eventdev.h         |  10 +-
> >>>    drivers/event/dlb2/dlb2.c                  |   1 +
> >>>    drivers/event/dpaa/dpaa_eventdev.c         |   1 +
> >>>    drivers/event/dpaa2/dpaa2_eventdev.c       |   2 +-
> >>>    drivers/event/dsw/dsw_evdev.c              |   1 +
> >>>    drivers/event/octeontx/ssovf_evdev.c       |   2 +-
> >>>    drivers/event/opdl/opdl_evdev.c            |   1 +
> >>>    drivers/event/skeleton/skeleton_eventdev.c |   1 +
> >>>    drivers/event/sw/sw_evdev.c                |   1 +
> >>>    lib/eventdev/eventdev_pmd.h                |  59 +++++-
> >>>    lib/eventdev/eventdev_private.c            |   9 +
> >>>    lib/eventdev/eventdev_trace.h              |  22 ++
> >>>    lib/eventdev/eventdev_trace_points.c       |   6 +
> >>>    lib/eventdev/rte_eventdev.c                | 146 ++++++++++---
> >>>    lib/eventdev/rte_eventdev.h                | 226 +++++++++++++++++++++
> >>>    lib/eventdev/rte_eventdev_core.h           |   4 +
> >>>    lib/eventdev/rte_eventdev_trace_fp.h       |   8 +
> >>>    lib/eventdev/version.map                   |   5 +
> >>>    33 files changed, 788 insertions(+), 108 deletions(-)
> >>>
> >>> --
> >>> 2.25.1
> >>>