Message ID | 20240815212658.48933-1-vjardin@free.fr |
---|---|
State | Superseded |
Headers | show |
Series | [1/1] package/dpdk: add 24.03 | expand |
Hello Vincent, Thanks for your patch, nice to see a Buildroot patch from you! See some comments below. On Thu, 15 Aug 2024 23:26:58 +0200 Vincent Jardin <vjardin@free.fr> wrote: > Importantly, this commit does not enforce the use of UIO or VFIO > kernel frameworks, as DPDK's architecture supports userland memory > mappings that do not require these technologies. By maintaining this > flexibility, DPDK can operate with a broader range of hardware and > software configurations, making it suitable for diverse Buildroot's > deployment scenarios. Do you intend to follow-up with additional patches supporting the other use-cases? > Signed-off-by: Vincent Jardin <vjardin@free.fr> > --- > package/Config.in | 1 + > package/dpdk/Config.in | 14 ++++++++++++++ > package/dpdk/dpdk.hash | 1 + > package/dpdk/dpdk.mk | 39 +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 55 insertions(+) For all new packages, we require an entry in the DEVELOPERS file to be added. > diff --git a/package/dpdk/Config.in b/package/dpdk/Config.in > new file mode 100644 > index 0000000000..56dcb42a33 > --- /dev/null > +++ b/package/dpdk/Config.in > @@ -0,0 +1,14 @@ > +config BR2_PACKAGE_DPDK > + bool "dpdk" > + depends on BR2_TOOLCHAIN_HAS_THREADS # glibc or uClibc toolchain required The comment does not make sense: the only library that can be compiled without thread support is uClibc. glibc and musl both always have thread support. > + select BR2_PACKAGE_HOST_PYTHON_PYELFTOOLS Not needed, we typically don't select host packages. > + select BR2_PACKAGE_LIBBSD When you "select" an option, you need to replicate its "depends on", so in this case, you need to add: depends on BR2_PACKAGE_LIBBSD_ARCH_SUPPORTS # libbsd depends on !BR2_STATIC_LIBS # libbsd depends on BR2_TOOLCHAIN_HAS_THREADS # libbsd depends on BR2_USE_WCHAR # libbsd > + select BR2_PACKAGE_LIBEXECINFO Selecting libexecinfo only makes sense for non glibc toolchains, so this should be: select BR2_PACKAGE_LIBEXECINFO if !BR2_TOOLCHAIN_USES_GLIBC also, you need to "depends on BR2_USE_WCHAR", or rather because you already have it due to libbsd, you need to do: depends on BR2_USE_WCHAR # libbsd, libexecinfo > + select BR2_PACKAGE_JANSSON > + select BR2_PACKAGE_LIBPCAP > + select BR2_PACKAGE_ZLIB > + help > + DPDK (Data Plane Development Kit) is a set of libraries > + and drivers for fast packet processing. > + > + http://dpdk.org/ You need to add a Config.in comment about the dependencies: comment "dpdk needs a toolchain w/ dynamic library, threads, wchar" depends on BR2_PACKAGE_LIBBSD_ARCH_SUPPORTS depends on BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HAS_THREADS || !BR2_USE_WCHAR > diff --git a/package/dpdk/dpdk.hash b/package/dpdk/dpdk.hash > new file mode 100644 > index 0000000000..fd8ab5a6aa > --- /dev/null > +++ b/package/dpdk/dpdk.hash > @@ -0,0 +1 @@ Please indicate where the hash comes from, in a comment. Calculated locally? Verified with some upstream provided hash? Crypto signature? See other .hash files for example. > +sha256 33ed973b3945af4f5923096ddca250b401dc80be3b5c6393b4e4fe43a1a6c2ad dpdk-24.03.tar.xz Also, please add the hashes of the license files. > diff --git a/package/dpdk/dpdk.mk b/package/dpdk/dpdk.mk > new file mode 100644 > index 0000000000..56adcf1d00 > --- /dev/null > +++ b/package/dpdk/dpdk.mk > @@ -0,0 +1,39 @@ > +################################################################################ > +# > +# DPDK lower case > +# > +################################################################################ > + > +# DPDK_VERSION = main > +# DPDK_SITE = https://dpdk.org/git/dpdk > +# DPDK_SITE_METHOD = git Please drop those 3 lines. > +DPDK_VERSION ?= 24.03 Please turn ?= into just =. > +DPDK_SOURCE = dpdk-$(DPDK_VERSION).tar.xz > +DPDK_SITE = http://fast.dpdk.org/rel > +DPDK_LICENSE = BSD-3-Clause > +DPDK_LICENSE_FILES = license/bsd-3-clause.txt license/README license/bsd-2-clause.txt license/exceptions.txt license/gpl-2.0.txt license/isc.txt license/lgpl-2.1.txt license/mit.txt How can the license be just BSD-3-Clause when there are license files for BSD-2-Clause, GPL-2.0, ISC, LGPL-2.1, MIT, and some exceptions? Also, since the LICENSE_FILES variable is long, please format it as such: DPDK_LICENSE_FILES = \ license/bsd-3-clause.txt \ ... \ ... \ > +DPDK_DEPENDENCIES += host-pkgconf > +DPDK_DEPENDENCIES += host-python-pyelftools > +DPDK_DEPENDENCIES += libbsd > +DPDK_DEPENDENCIES += libexecinfo > +DPDK_DEPENDENCIES += jansson > +DPDK_DEPENDENCIES += libpcap > +DPDK_DEPENDENCIES += zlib Just one assignment, and use = instead of += since these are unconditional: DPDK_DEPENDENCIES = \ host-pkgconf \ host-python-pyelftools \ libbsd \ ... Please add libexecinfo in the dependencies only if BR2_TOOLCHAIN_USES_GLIBC is false: ifeq ($(BR2_TOOLCHAIN_USES_GLIBC),) DPDK_DEPENDENCIES += libexecinfo endif > +#not yet DPDK_DEPENDENCIES += openssl > +#not yet DPDK_DEPENDENCIES += libbpf Please drop commented code. > + > +DPDK_MARCH = $(BR2_ARCH) Are you sure all values of $(BR2_ARCH) are supported as -Dcpu_instruction_set values? What is -Dcpu_instruction_set actually doing? > +DPDK_MTUNE = $(BR2_ARCH) # not used yet Please drop if it's not used. > +GCC_TARGET_CPU=$(BR2_GCC_TARGET_ARCH) Please drop, GCC_TARGET_CPU does not belong to this package. > +# see meson_options.txt from DPDK Comment not needed. > +# > +DPDK_CONF_OPTS += -Ddeveloper_mode=enabled What does it do? Are we sure we want it enabled unconditionally? > +DPDK_CONF_OPTS += -Dcpu_instruction_set=$(DPDK_MARCH) > + > +# platform can be: native, all, cn9k, cn10k > +DPDK_CONF_OPTS += -Dplatform=generic Well, the comment says that platform can be "native", "all", "cn9k", "cn10k"... and you're setting it to "generic". Doesn't really make sense. Also, isn't libpcap only needed for the "generic" platform? Can one select only one platform, or a list of platforms? If one can only select one platform, then maybe we need to have a choice..endchoice in Config.in: choice prompt "Platform" default BR2_PACKAGE_DPDK_PLATFORM_GENERIC config BR2_PACKAGE_DPDK_PLATFORM_GENERIC bool "generic" select BR2_PACKAGE_LIBPCAP endchoice which of course can be extended later with additional platforms. Could you rework your patch according to those suggestions? Thanks a lot! Thanks! Thomas
Hi Thomas, Le 17 août 2024 10:29:25 Thomas Petazzoni <thomas.petazzoni@bootlin.com> a écrit : > Hello Vincent, > > Thanks for your patch, nice to see a Buildroot patch from you! See some > comments below. You welcome ;) You did help me to bootstrap, now I can follow up with some contributions. > > > On Thu, 15 Aug 2024 23:26:58 +0200 > Vincent Jardin <vjardin@free.fr> wrote: > >> Importantly, this commit does not enforce the use of UIO or VFIO >> kernel frameworks, as DPDK's architecture supports userland memory >> mappings that do not require these technologies. By maintaining this >> flexibility, DPDK can operate with a broader range of hardware and >> software configurations, making it suitable for diverse Buildroot's >> deployment scenarios. > > Do you intend to follow-up with additional patches supporting the other > use-cases? Yes I will but first I'd like to iterate with some early steps. See the v2. Once merge, I'll keep adding more cases. Meanwhile, there is a build CI on github for some early checks: https://github.com/vjardin/br_dpdk/actions Best regards, Vincent
diff --git a/package/Config.in b/package/Config.in index f2c63ffb6e..5b21805e9f 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1990,6 +1990,7 @@ menu "Networking" source "package/daq3/Config.in" source "package/davici/Config.in" source "package/dht/Config.in" + source "package/dpdk/Config.in" source "package/enet/Config.in" source "package/filemq/Config.in" source "package/fmlib/Config.in" diff --git a/package/dpdk/Config.in b/package/dpdk/Config.in new file mode 100644 index 0000000000..56dcb42a33 --- /dev/null +++ b/package/dpdk/Config.in @@ -0,0 +1,14 @@ +config BR2_PACKAGE_DPDK + bool "dpdk" + depends on BR2_TOOLCHAIN_HAS_THREADS # glibc or uClibc toolchain required + select BR2_PACKAGE_HOST_PYTHON_PYELFTOOLS + select BR2_PACKAGE_LIBBSD + select BR2_PACKAGE_LIBEXECINFO + select BR2_PACKAGE_JANSSON + select BR2_PACKAGE_LIBPCAP + select BR2_PACKAGE_ZLIB + help + DPDK (Data Plane Development Kit) is a set of libraries + and drivers for fast packet processing. + + http://dpdk.org/ diff --git a/package/dpdk/dpdk.hash b/package/dpdk/dpdk.hash new file mode 100644 index 0000000000..fd8ab5a6aa --- /dev/null +++ b/package/dpdk/dpdk.hash @@ -0,0 +1 @@ +sha256 33ed973b3945af4f5923096ddca250b401dc80be3b5c6393b4e4fe43a1a6c2ad dpdk-24.03.tar.xz diff --git a/package/dpdk/dpdk.mk b/package/dpdk/dpdk.mk new file mode 100644 index 0000000000..56adcf1d00 --- /dev/null +++ b/package/dpdk/dpdk.mk @@ -0,0 +1,39 @@ +################################################################################ +# +# DPDK +# +################################################################################ + +# DPDK_VERSION = main +# DPDK_SITE = https://dpdk.org/git/dpdk +# DPDK_SITE_METHOD = git +DPDK_VERSION ?= 24.03 +DPDK_SOURCE = dpdk-$(DPDK_VERSION).tar.xz +DPDK_SITE = http://fast.dpdk.org/rel +DPDK_LICENSE = BSD-3-Clause +DPDK_LICENSE_FILES = license/bsd-3-clause.txt license/README license/bsd-2-clause.txt license/exceptions.txt license/gpl-2.0.txt license/isc.txt license/lgpl-2.1.txt license/mit.txt + +DPDK_DEPENDENCIES += host-pkgconf +DPDK_DEPENDENCIES += host-python-pyelftools +DPDK_DEPENDENCIES += libbsd +DPDK_DEPENDENCIES += libexecinfo +DPDK_DEPENDENCIES += jansson +DPDK_DEPENDENCIES += libpcap +DPDK_DEPENDENCIES += zlib +#not yet DPDK_DEPENDENCIES += openssl +#not yet DPDK_DEPENDENCIES += libbpf + +DPDK_MARCH = $(BR2_ARCH) +DPDK_MTUNE = $(BR2_ARCH) # not used yet +GCC_TARGET_CPU=$(BR2_GCC_TARGET_ARCH) + +# see meson_options.txt from DPDK +# +DPDK_CONF_OPTS += -Ddeveloper_mode=enabled + +DPDK_CONF_OPTS += -Dcpu_instruction_set=$(DPDK_MARCH) + +# platform can be: native, all, cn9k, cn10k +DPDK_CONF_OPTS += -Dplatform=generic + +$(eval $(meson-package))