[RFC,0/3] Split logging out of EAL

  Following recent discussion on-list about EAL needing to be broken down
a bit, here is an RFC where logging functionality is split out of EAL
into a separate library. Most parts of this work is fairly straight
forward - there are only two complications:

1. The logging functions use "fnmatch", which is not available on
   windows, and so has a compatibility fallback in EAL.

2. There are individual logging files for each supported OS.

For #1, there were really two options to avoid the circular dependency -
either move fnmatch into the log library, or to create a new lower-level
library for such back function fallbacks. For this RFC I've taken the
second option as a better solution. Ideally, more of EAL compat
functions should be moved to such a library if we create one, but for
now, it's only the one function that was needed to be moved.

For #2, this was fixed using standard naming and the build system to
only build the appropriately named file for each OS. The alternative of
creating a subdir per-OS seems overkill for the single-file situation.

NOTE: this is an early RFC based on work I did some time back, and is
intended just to inspire further discussion and work about splitting
EAL, more than necessarily being a patchset for future merging.

Bruce Richardson (3):
  os: begin separating some OS compatibility from EAL
  log: separate logging functions out of EAL
  telemetry: use standard logging

 lib/eal/common/eal_private.h                  |  7 ----
 lib/eal/common/meson.build                    |  1 -
 lib/eal/freebsd/eal.c                         |  6 +--
 lib/eal/include/meson.build                   |  1 -
 lib/eal/linux/eal.c                           |  6 +--
 lib/eal/linux/meson.build                     |  1 -
 lib/eal/meson.build                           |  2 +-
 lib/eal/version.map                           | 17 --------
 lib/eal/windows/meson.build                   |  2 -
 lib/kvargs/meson.build                        |  3 +-
 lib/{eal/common => log}/eal_common_log.c      |  1 -
 lib/{eal/common => log}/eal_log.h             | 11 ++++++
 .../linux/eal_log.c => log/eal_log_linux.c}   |  0
 .../eal_log.c => log/eal_log_windows.c}       |  0
 lib/log/meson.build                           |  8 ++++
 lib/{eal/include => log}/rte_log.h            |  0
 lib/log/version.map                           | 39 +++++++++++++++++++
 lib/meson.build                               | 12 +++---
 lib/os/freebsd/fnmatch.c                      |  3 ++
 lib/os/linux/fnmatch.c                        |  3 ++
 lib/os/meson.build                            |  8 ++++
 lib/os/os.c                                   |  3 ++
 lib/os/version.map                            |  7 ++++
 lib/{eal => os}/windows/fnmatch.c             |  0
 .../windows/include => os/windows}/fnmatch.h  |  0
 lib/telemetry/meson.build                     |  3 +-
 lib/telemetry/telemetry.c                     | 12 +++---
 lib/telemetry/telemetry_internal.h            |  3 +-
 28 files changed, 100 insertions(+), 59 deletions(-)
 rename lib/{eal/common => log}/eal_common_log.c (99%)
 rename lib/{eal/common => log}/eal_log.h (80%)
 rename lib/{eal/linux/eal_log.c => log/eal_log_linux.c} (100%)
 rename lib/{eal/windows/eal_log.c => log/eal_log_windows.c} (100%)
 create mode 100644 lib/log/meson.build
 rename lib/{eal/include => log}/rte_log.h (100%)
 create mode 100644 lib/log/version.map
 create mode 100644 lib/os/freebsd/fnmatch.c
 create mode 100644 lib/os/linux/fnmatch.c
 create mode 100644 lib/os/meson.build
 create mode 100644 lib/os/os.c
 create mode 100644 lib/os/version.map
 rename lib/{eal => os}/windows/fnmatch.c (100%)
 rename lib/{eal/windows/include => os/windows}/fnmatch.h (100%)

--
2.34.1
  

On Wed, Aug 9, 2023 at 3:36 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> There is a general desire to reduce the size and scope of EAL. To this
> end, this patchset makes a (very) small step in that direction by taking
> the logging functionality out of EAL and putting it into its own library
> that can be built and maintained separately.
>
> As with the first RFC for this, the main obstacle is the "fnmatch"
> function which is needed by both EAL and the new log function when
> building on windows. While the function cannot stay in EAL - or we would
> have a circular dependency, moving it to a new library or just putting
> it in the log library have the disadvantages that it then "leaks" into
> the public namespace without an rte_prefix, which could cause issues.
> Since only a single function is involved, subsequent versions take a
> different approach to v1, and just moves the offending function to be a
> static function in a header file. This allows use by multiple libs
> without conflicting names or making it public.
>
> The other complication, as explained in v1 RFC was that of multiple
> implementations for different OS's. This is solved here in the same
> way as v1, by including the OS in the name and having meson pick the
> correct file for each build. Since only one file is involved, there
> seemed little need for replicating EAL's separate subdirectories
> per-OS.

Series applied, thanks Bruce for this first step.

As mentionned during the maintainers weekly call yesterday, this is
only a first "easy" step but, thinking of next steps, more splitting
may not be that easy.

At least, on the libabigail topic, as we need the ABI check to handle
libraries splits, a new feature has been cooked in (not yet released)
2.4 libabigail.
https://sourceware.org/git/?p=libabigail.git;a=commitdiff;h=0b338dfaf690993e123b6433201b3a8b8204d662
Hopefully, we will have a libabigail release available by the time we
start the v24.03 release (and re-enable ABI checks).
  

On Fri, Aug 11, 2023 at 02:46:13PM +0200, David Marchand wrote:
> On Wed, Aug 9, 2023 at 3:36 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > There is a general desire to reduce the size and scope of EAL. To this
> > end, this patchset makes a (very) small step in that direction by taking
> > the logging functionality out of EAL and putting it into its own library
> > that can be built and maintained separately.
> >
> > As with the first RFC for this, the main obstacle is the "fnmatch"
> > function which is needed by both EAL and the new log function when
> > building on windows. While the function cannot stay in EAL - or we would
> > have a circular dependency, moving it to a new library or just putting
> > it in the log library have the disadvantages that it then "leaks" into
> > the public namespace without an rte_prefix, which could cause issues.
> > Since only a single function is involved, subsequent versions take a
> > different approach to v1, and just moves the offending function to be a
> > static function in a header file. This allows use by multiple libs
> > without conflicting names or making it public.
> >
> > The other complication, as explained in v1 RFC was that of multiple
> > implementations for different OS's. This is solved here in the same
> > way as v1, by including the OS in the name and having meson pick the
> > correct file for each build. Since only one file is involved, there
> > seemed little need for replicating EAL's separate subdirectories
> > per-OS.
> 
> Series applied, thanks Bruce for this first step.
> 
> As mentionned during the maintainers weekly call yesterday, this is
> only a first "easy" step but, thinking of next steps, more splitting
> may not be that easy.
> 

I took a look after doing this patchset to try and find more easy to
extract parts, but did not find many. The EAL is pretty intertwined now.
As I look at it, there are really two ways to try and dis-entangle it - 
bottoms-up or top-down. This patchset is an example of the former, taking a
logical set of related APIs and pulling them out into a separate library
that EAL can depend upon. There may be some other API-sets like this we can
pull out, but in my search I didn't find any.

The other tops-down approach may be worth looking at too. We can try and
take the top-level EAL init function and separate it out into a separate
initialization library (which may be called "EAL" with the rest being
called something else). I have not tried this approach to see how it goes,
but pulling out the init may allow further dis-entangling of other parts of
EAL.

/Bruce
  

11/08/2023 14:59, Bruce Richardson:
> On Fri, Aug 11, 2023 at 02:46:13PM +0200, David Marchand wrote:
> > On Wed, Aug 9, 2023 at 3:36 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > There is a general desire to reduce the size and scope of EAL. To this
> > > end, this patchset makes a (very) small step in that direction by taking
> > > the logging functionality out of EAL and putting it into its own library
> > > that can be built and maintained separately.
> > >
> > > As with the first RFC for this, the main obstacle is the "fnmatch"
> > > function which is needed by both EAL and the new log function when
> > > building on windows. While the function cannot stay in EAL - or we would
> > > have a circular dependency, moving it to a new library or just putting
> > > it in the log library have the disadvantages that it then "leaks" into
> > > the public namespace without an rte_prefix, which could cause issues.
> > > Since only a single function is involved, subsequent versions take a
> > > different approach to v1, and just moves the offending function to be a
> > > static function in a header file. This allows use by multiple libs
> > > without conflicting names or making it public.
> > >
> > > The other complication, as explained in v1 RFC was that of multiple
> > > implementations for different OS's. This is solved here in the same
> > > way as v1, by including the OS in the name and having meson pick the
> > > correct file for each build. Since only one file is involved, there
> > > seemed little need for replicating EAL's separate subdirectories
> > > per-OS.
> > 
> > Series applied, thanks Bruce for this first step.
> > 
> > As mentionned during the maintainers weekly call yesterday, this is
> > only a first "easy" step but, thinking of next steps, more splitting
> > may not be that easy.
> 
> I took a look after doing this patchset to try and find more easy to
> extract parts, but did not find many. The EAL is pretty intertwined now.
> As I look at it, there are really two ways to try and dis-entangle it - 
> bottoms-up or top-down. This patchset is an example of the former, taking a
> logical set of related APIs and pulling them out into a separate library
> that EAL can depend upon. There may be some other API-sets like this we can
> pull out, but in my search I didn't find any.

A small thing to easily separate is rte_bitmap.

> The other tops-down approach may be worth looking at too. We can try and
> take the top-level EAL init function and separate it out into a separate
> initialization library (which may be called "EAL" with the rest being
> called something else). I have not tried this approach to see how it goes,
> but pulling out the init may allow further dis-entangling of other parts of
> EAL.

I agree we should start with separating the init logic.
I would call it rte_init. We may need rte_opts for command line parsing.
I think the name EAL should be kept for the low-level functions,
abstracting differences between OS, CPU and toolchains.

Next steps would be to try to separate memory and CPU management
in 2 different libraries.
We'll need to decide whether rte_service is part of the CPU management.
Same question for multiprocess communication rte_mp and rte_keepalive.
I suppose we can keep IRQ and threading in EAL.
VFIO may be managed in a separate library perhaps.

Once we do that, the low-level libraries like log, kvargs and telemetry
can depend on EAL, being the very low-level common denominator.
The trace subsystem can probably become a separate library as well.

In a later step, we can think about bus and device management.

And the ideal would be to extract tailq, once all logic is out of EAL.

How does it sound?
  

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, 1 September 2023 13.25
> 
> 11/08/2023 14:59, Bruce Richardson:
> > On Fri, Aug 11, 2023 at 02:46:13PM +0200, David Marchand wrote:
> > > On Wed, Aug 9, 2023 at 3:36 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > >
> > > > There is a general desire to reduce the size and scope of EAL. To this
> > > > end, this patchset makes a (very) small step in that direction by taking
> > > > the logging functionality out of EAL and putting it into its own library
> > > > that can be built and maintained separately.
> > > >
> > > > As with the first RFC for this, the main obstacle is the "fnmatch"
> > > > function which is needed by both EAL and the new log function when
> > > > building on windows. While the function cannot stay in EAL - or we would
> > > > have a circular dependency, moving it to a new library or just putting
> > > > it in the log library have the disadvantages that it then "leaks" into
> > > > the public namespace without an rte_prefix, which could cause issues.
> > > > Since only a single function is involved, subsequent versions take a
> > > > different approach to v1, and just moves the offending function to be a
> > > > static function in a header file. This allows use by multiple libs
> > > > without conflicting names or making it public.
> > > >
> > > > The other complication, as explained in v1 RFC was that of multiple
> > > > implementations for different OS's. This is solved here in the same
> > > > way as v1, by including the OS in the name and having meson pick the
> > > > correct file for each build. Since only one file is involved, there
> > > > seemed little need for replicating EAL's separate subdirectories
> > > > per-OS.
> > >
> > > Series applied, thanks Bruce for this first step.
> > >
> > > As mentionned during the maintainers weekly call yesterday, this is
> > > only a first "easy" step but, thinking of next steps, more splitting
> > > may not be that easy.
> >
> > I took a look after doing this patchset to try and find more easy to
> > extract parts, but did not find many. The EAL is pretty intertwined now.
> > As I look at it, there are really two ways to try and dis-entangle it -
> > bottoms-up or top-down. This patchset is an example of the former, taking a
> > logical set of related APIs and pulling them out into a separate library
> > that EAL can depend upon. There may be some other API-sets like this we can
> > pull out, but in my search I didn't find any.
> 
> A small thing to easily separate is rte_bitmap.
> 
> > The other tops-down approach may be worth looking at too. We can try and
> > take the top-level EAL init function and separate it out into a separate
> > initialization library (which may be called "EAL" with the rest being
> > called something else). I have not tried this approach to see how it goes,
> > but pulling out the init may allow further dis-entangling of other parts of
> > EAL.
> 
> I agree we should start with separating the init logic.
> I would call it rte_init. We may need rte_opts for command line parsing.
> I think the name EAL should be kept for the low-level functions,
> abstracting differences between OS, CPU and toolchains.
> 
> Next steps would be to try to separate memory and CPU management
> in 2 different libraries.
> We'll need to decide whether rte_service is part of the CPU management.
> Same question for multiprocess communication rte_mp and rte_keepalive.
> I suppose we can keep IRQ and threading in EAL.
> VFIO may be managed in a separate library perhaps.
> 
> Once we do that, the low-level libraries like log, kvargs and telemetry
> can depend on EAL, being the very low-level common denominator.
> The trace subsystem can probably become a separate library as well.
> 
> In a later step, we can think about bus and device management.
> 
> And the ideal would be to extract tailq, once all logic is out of EAL.

Yes, tailq is a good example of a utility library, which can stand on its own (i.e. it could be used for non-DPDK applications too).

> 
> How does it sound?
> 

Not really a reply to Thomas, just thinking out loud here...

From a high level perspective, thinking about two dimensions, layers and silos, (the boxes a marketing person would draw on a PowerPoint slide to describe what DPDK comprises of) might also help defining the borders.

For vertical separation:

There are the architecture specific definitions, e.g. header files created when selecting an architecture. There should be no functions in this box, only descriptions/definitions.

Then there are the utility libraries/functions, e.g. bit manipulation and log2 functions, the command line parser, etc.. Some of these depend on the architecture definitions. But none of these should depend on the DPDK runtime itself - i.e. it should be possible to use this for writing a non-dataplane application (in theory).

Then there is the DPDK core itself. The core of this should be kept at an absolute minimum, making the various features less cross-dependent. (This is where the "mandatory" libraries live.)

And then there is the DPDK runtime, with application init/start, timers, service cores, etc.

And finally, there are all the DPDK libraries.

Maybe they can be grouped into "bus" libraries (e.g. pci), "driver" libraries (e.g. compressdev, cryptodev, ethdev), "feature" libraries (e.g. reorder, bpf, gro, frag) and "utility" libraries (e.g. net, ring, rcu, mempool).

Some can be classified as "core" libraries (e.g. mbuf), because many other libs can depend on them. And some might be classified as "mandatory" (e.g. ring and mempool), because the DPDK runtime depends on them.
  

On Fri, Aug 11, 2023 at 2:46 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Wed, Aug 9, 2023 at 3:36 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > There is a general desire to reduce the size and scope of EAL. To this
> > end, this patchset makes a (very) small step in that direction by taking
> > the logging functionality out of EAL and putting it into its own library
> > that can be built and maintained separately.
> >
> > As with the first RFC for this, the main obstacle is the "fnmatch"
> > function which is needed by both EAL and the new log function when
> > building on windows. While the function cannot stay in EAL - or we would
> > have a circular dependency, moving it to a new library or just putting
> > it in the log library have the disadvantages that it then "leaks" into
> > the public namespace without an rte_prefix, which could cause issues.
> > Since only a single function is involved, subsequent versions take a
> > different approach to v1, and just moves the offending function to be a
> > static function in a header file. This allows use by multiple libs
> > without conflicting names or making it public.
> >
> > The other complication, as explained in v1 RFC was that of multiple
> > implementations for different OS's. This is solved here in the same
> > way as v1, by including the OS in the name and having meson pick the
> > correct file for each build. Since only one file is involved, there
> > seemed little need for replicating EAL's separate subdirectories
> > per-OS.
>
> Series applied, thanks Bruce for this first step.
>
> As mentionned during the maintainers weekly call yesterday, this is
> only a first "easy" step but, thinking of next steps, more splitting
> may not be that easy.
>
> At least, on the libabigail topic, as we need the ABI check to handle
> libraries splits, a new feature has been cooked in (not yet released)
> 2.4 libabigail.
> https://sourceware.org/git/?p=libabigail.git;a=commitdiff;h=0b338dfaf690993e123b6433201b3a8b8204d662
> Hopefully, we will have a libabigail release available by the time we
> start the v24.03 release (and re-enable ABI checks).

For the record, libabigail 2.4 got released last Friday.
I had already prepared a patch to use this new feature, I can send it
when needed.