[RFC,0/3] Split logging out of EAL
Message ID | 20220829151901.376754-1-bruce.richardson@intel.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E5ED2A0542; Mon, 29 Aug 2022 17:19:18 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D9A7440DF7; Mon, 29 Aug 2022 17:19:18 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mails.dpdk.org (Postfix) with ESMTP id A75EF4069F for <dev@dpdk.org>; Mon, 29 Aug 2022 17:19:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1661786356; x=1693322356; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=gX8ATEFB+BZQwOqlOe5kvQ0N0MF8CGW4IvzEvUJQxlk=; b=gYd3bLZ3rdtFeXOhecWdifQJYFrmWFuO66iCxRW37sSdmCzP2kdaejyj EyCeQ26Kt+tis/L3OJ2SCynypfjAj8+uuLyu7Du3n6wWacVOXiLuBT1ca aLjidWltkgpZ3v+twiijFUFYKZvqqqq34dvPslE/3gB1JIrsdMsBk/ebs uKbRDNnAEIp4Nqke7eIWDpsYLaIa0ucL0rmGypUFDGgRnrSTMNFtLLQ6L dluR5odRLJVc+MvpbTPmZc9pdtyKhlDAYsfWs2YMLpErO8WFAlX0gUv4p Tbptpbd5qBx6PbWQvKTjXUQOptBwm7uHfPL+ts5+nOwB6v7dgxd2dDE3w g==; X-IronPort-AV: E=McAfee;i="6500,9779,10454"; a="296196031" X-IronPort-AV: E=Sophos;i="5.93,272,1654585200"; d="scan'208";a="296196031" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Aug 2022 08:19:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.93,272,1654585200"; d="scan'208";a="588198065" Received: from silpixa00401385.ir.intel.com (HELO silpixa00401385.ger.corp.intel.com) ([10.237.222.145]) by orsmga006.jf.intel.com with ESMTP; 29 Aug 2022 08:19:06 -0700 From: Bruce Richardson <bruce.richardson@intel.com> To: dev@dpdk.org Cc: Bruce Richardson <bruce.richardson@intel.com> Subject: [RFC PATCH 0/3] Split logging out of EAL Date: Mon, 29 Aug 2022 16:18:58 +0100 Message-Id: <20220829151901.376754-1-bruce.richardson@intel.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org |
Message
Bruce Richardson
Aug. 29, 2022, 3:18 p.m. UTC
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
Comments
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.