All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [RFC PATCH] dpdk: new package
@ 2015-10-30 11:05 Jan Viktorin
  2015-10-30 17:17 ` Arnout Vandecappelle
  2015-12-09 14:11 ` [Buildroot] [PATCH v2 0/3] " Jan Viktorin
  0 siblings, 2 replies; 16+ messages in thread
From: Jan Viktorin @ 2015-10-30 11:05 UTC (permalink / raw)
  To: buildroot

This patch introduces support of the DPDK library (www.dpdk.org) into
Buildroot. It compiles and installs DPDK libraries on the target and
staging and allows to compiler other applications depending on the DPDK
library. It can also install some basic tools the DPDK provides
(testpmd, python scripts, test suite).

The patch assumes DPDK 2.2 which is not out at the moment (scheduled
to the end of November). However, this version will contain support for
the ARM architecture that is currently being reviewed.

Testing of DPDK on ARM is possible by cloning

 https://github.com/RehiveTech/dpdk arm-support-v5

as the package allows customization of the download process.

The ARM ports can be tested by

 qemu_aarch64_virt_defconfig
 qemu_arm_vexpress_defconfig

I did not test, how it works with x86 as I do not use Buildroot
for this.

The included hashes are calculated locally by downloading the tar.gz
archives by hand.

There are unfortunately some pitfalls:

* it may require to enable PCI, MSIX, UIO in the Linux Kernel
  (some defconfigs does not include as default and it is platform
  dependent as ARMv7 almost does not use PCI)

* when building PCAP PMD driver, the libpcap is required (however,
  this dependency is difficult to express in Buildroot as this depends
  on the DPDK configuration file - similar to the issue above)

* some tools the DPDK provides depend on Python(2) so the user has
  to enable it to install those

I assume, the user knows what he is doing in those cases and configures
the Buildroot in the right way. Or any ideas? Yann?

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
---
 package/Config.in      |   1 +
 package/dpdk/Config.in | 159 +++++++++++++++++++++++++++++++++++++++++++++++++
 package/dpdk/dpdk.hash |   6 ++
 package/dpdk/dpdk.mk   | 119 ++++++++++++++++++++++++++++++++++++
 4 files changed, 285 insertions(+)
 create mode 100644 package/dpdk/Config.in
 create mode 100644 package/dpdk/dpdk.hash
 create mode 100644 package/dpdk/dpdk.mk

diff --git a/package/Config.in b/package/Config.in
index 10ff94e..7ff7cc2 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -988,6 +988,7 @@ menu "Networking"
 	source "package/cgic/Config.in"
 	source "package/cppzmq/Config.in"
 	source "package/czmq/Config.in"
+	source "package/dpdk/Config.in"
 	source "package/filemq/Config.in"
 	source "package/flickcurl/Config.in"
 	source "package/fmlib/Config.in"
diff --git a/package/dpdk/Config.in b/package/dpdk/Config.in
new file mode 100644
index 0000000..5353708
--- /dev/null
+++ b/package/dpdk/Config.in
@@ -0,0 +1,159 @@
+config BR2_PACKAGE_DPDK
+       bool "dpdk"
+       depends on BR2_x86_i686 || BR2_x86_64 || BR2_powerpc_power8 || BR2_ARM_CPU_ARMV7A || BR2_aarch64 || BR2_aarch64_be
+       depends on BR2_TOOLCHAIN_USES_GLIBC
+       help
+         DPDK is a set of libraries and drivers for fast packet processing. It was designed to run on
+         any processors knowing Intel x86 has been the first CPU to be supported. Ports for other CPUs
+         like IBM Power 8 and ARM are under progress. It runs mostly in Linux userland. A FreeBSD port
+         is now available for a subset of DPDK features.
+
+	 To build the included Linux Kernel drivers, it is necessary to enable CONFIG_PCI_MSI,
+	 CONFIG_UIO.
+
+         http://www.dpdk.org/
+
+if BR2_PACKAGE_DPDK
+
+choice
+	prompt "DPDK version"
+
+config BR2_PACKAGE_DPDK_VERSION_LATEST
+	bool "2.2.0"
+
+config BR2_PACKAGE_DPDK_VERSION_CUSTOM
+	bool "Custom"
+
+config BR2_PACKAGE_DPDK_CUSTOM_TARBALL
+	bool "Custom tarball"
+
+config BR2_PACKAGE_DPDK_CUSTOM_GIT
+	bool "Custom Git repository"
+
+config BR2_PACKAGE_DPDK_CUSTOM_LOCAL
+	bool "Local directory"
+
+endchoice
+
+config BR2_PACKAGE_DPDK_VERSION_CUSTOM_VALUE
+	string "DPDK version"
+	depends on BR2_PACKAGE_DPDK_VERSION_CUSTOM
+
+config BR2_PACKAGE_DPDK_CUSTOM_TARBALL_LOCATION
+	string "URL of custom DPDK tarball"
+	depends on BR2_PACKAGE_DPDK_CUSTOM_TARBALL
+
+config BR2_PACKAGE_DPDK_CUSTOM_REPO_URL
+	string "URL of custom repository"
+	depends on BR2_PACKAGE_DPDK_CUSTOM_GIT
+
+config BR2_PACKAGE_DPDK_CUSTOM_REPO_VERSION
+	string "Custom repository version"
+	depends on BR2_PACKAGE_DPDK_CUSTOM_GIT
+	help
+	  Revision to use in the typical format used by Git
+	  E.G. a sha id, a tag, branch, ..
+
+config BR2_PACKAGE_DPDK_CUSTOM_LOCAL_PATH
+	string "Path to the local directory"
+	depends on BR2_PACKAGE_DPDK_CUSTOM_LOCAL
+
+config BR2_PACKAGE_DPDK_VERSION
+	string
+	default "2.1.0" if BR2_PACKAGE_DPDK_VERSION_LATEST
+	default BR2_PACKAGE_DPDK_VERSION_CUSTOM_VALUE \
+		if BR2_PACKAGE_DPDK_VERSION_CUSTOM
+	default "custom" if BR2_PACKAGE_DPDK_CUSTOM_TARBALL
+	default BR2_PACKAGE_DPDK_CUSTOM_REPO_VERSION \
+		if BR2_PACKAGE_DPDK_CUSTOM_GIT
+	default "custom" if BR2_PACKAGE_DPDK_CUSTOM_LOCAL
+
+config BR2_PACKAGE_DPDK_PATCH
+	string "Custom DPDK patches"
+	depends on !BR2_PACKAGE_DPDK_CUSTOM_LOCAL
+	help
+	  A space-separated list of patches to apply to the
+	  kernel. Each patch can be described as an URL, a local file
+	  path, or a directory. In the case of a directory, all files
+	  matching *.patch in the directory will be applied.
+
+choice
+       prompt "DPDK configuration"
+
+config BR2_PACKAGE_DPDK_x86_i686_gcc
+       bool "i686-native-linuxapp-gcc"
+       depends on BR2_x86_i686
+
+config BR2_PACKAGE_DPDK_x86_64_gcc
+       bool "x86_64-native-linuxapp-gcc"
+       depends on BR2_x86_64
+
+config BR2_PACKAGE_DPDK_PPC8_gcc
+       bool "ppc_64-power8-native-linuxapp-gcc"
+       depends on BR2_powerpc_power8
+
+config BR2_PACKAGE_DPDK_ARMv7_gcc
+       bool "arm-armv7a-linuxapp-gcc"
+       depends on BR2_ARM_CPU_ARMV7A
+
+config BR2_PACKAGE_DPDK_ARMv8_gcc
+       bool "arm64-armv8a-linuxapp-gcc"
+       depends on BR2_aarch64 || BR2_aarch64_be
+
+config BR2_PACKAGE_DPDK_CONFIG_CUSTOM
+       bool "Custom"
+
+endchoice
+
+config BR2_PACKAGE_DPDK_CONFIG_CUSTOM_STRING
+       string "DPDK custom configuration"
+       depends on BR2_PACKAGE_DPDK_CONFIG_CUSTOM
+       help
+         See the config/ directory of DPDK for available configurations
+	 or patch it with your own one.
+
+config BR2_PACKAGE_DPDK_CONFIG
+	string
+	default BR2_PACKAGE_DPDK_CONFIG_CUSTOM_STRING \
+		if BR2_PACKAGE_DPDK_CONFIG_CUSTOM
+	default "i686-native-linuxapp-gcc" \
+		if BR2_PACKAGE_DPDK_x86_i686_gcc
+	default "x86_64-native-linuxapp-gcc" \
+		if BR2_PACKAGE_DPDK_x86_64_gcc
+	default "ppc_64-power8-native-linuxapp-gcc" \
+		if BR2_PACKAGE_DPDK_PPC8_gcc
+	default "arm-armv7a-linuxapp-gcc" \
+		if BR2_PACKAGE_DPDK_ARMv7_gcc
+	default "arm64-armv8a-linuxapp-gcc" \
+		if BR2_PACKAGE_DPDK_ARMv8_gcc
+
+config BR2_PACKAGE_DPDK_TOOLS_INSTALL
+	bool "install tools"
+	help
+	  Install applications coming with the DPDK project.
+
+config BR2_PACKAGE_DPDK_TOOLS_PYSCRIPTS
+	bool "helper scripts"
+	depends on BR2_PACKAGE_DPDK_TOOLS_INSTALL
+	depends on BR2_PACKAGE_PYTHON
+	help
+	  DPDK provides few Python scripts to determine configuration
+	  and status of the system and to configure drivers. These are:
+	  dpdk_nic_bind.py and cpu_layout.py. By enabling this option,
+	  the scripts are installed into the target system together with
+	  a Python 2 interpreter.
+
+config BR2_PACKAGE_DPDK_TOOLS_TESTPMD
+	bool "testpmd"
+	default y
+	depends on BR2_PACKAGE_DPDK_TOOLS_INSTALL
+	help
+	  Install application for general testing of DPDK and PMD drivers.
+
+config BR2_PACKAGE_DPDK_TOOLS_TEST
+	bool "tests suite"
+	depends on BR2_PACKAGE_DPDK_TOOLS_INSTALL
+	help
+	  Install all DPDK tests.
+
+endif
diff --git a/package/dpdk/dpdk.hash b/package/dpdk/dpdk.hash
new file mode 100644
index 0000000..a5c4dbd
--- /dev/null
+++ b/package/dpdk/dpdk.hash
@@ -0,0 +1,6 @@
+# Locally calculated
+sha256	f7b322867a45f99afd9c8fbacdc56e1621676f9ca0f046656ec85eb6a99a3440  dpdk-2.1.0.tar.gz
+sha256  643789a3be5ba44dd84d6b248cdf5471b260f8736dada177dadf076aebfbff3f  dpdk-2.0.0.tar.gz
+sha256  9f5386830bd999355182e20408f3fc2cfa0802a4497fdded8d43202feede1939  dpdk-1.8.0.tar.gz
+sha256  9deeb7467c404883946735a1fbcf126b98216ae8f8862ea86bc0c15e3807a679  dpdk-1.7.1.tar.gz
+sha256  aafc290260b5002d248ab8f8c8ffa76454d4b1382aa3c82ae2700ecce481397a  dpdk-1.7.0.tar.gz
diff --git a/package/dpdk/dpdk.mk b/package/dpdk/dpdk.mk
new file mode 100644
index 0000000..70ff4b8
--- /dev/null
+++ b/package/dpdk/dpdk.mk
@@ -0,0 +1,119 @@
+################################################################################
+#
+# dpdk
+#
+################################################################################
+
+DPDK_VERSION = $(call qstrip,$(BR2_PACKAGE_DPDK_VERSION))
+ifeq ($(BR2_PACKAGE_DPDK_CUSTOM_TARBALL),y)
+DPDK_TARBALL = $(call qstrip,$(BR2_PACKAGE_DPDK_CUSTOM_TARBALL_LOCATION))
+DPDK_SIZE = $(patsubst %/,%,$(dir $(DPDK_TARBALL)))
+DPDK_SOURCE = $(notdir $(DPDK_TARBALL))
+BR_NO_CHECK_HASH_FOR += $(DPDK_SOURCE)
+else ifeq ($(BR2_PACKAGE_DPDK_CUSTOM_LOCAL),y)
+DPDK_SITE = $(call qstrip,$(BR2_PACKAGE_DPDK_CUSTOM_LOCAL_PATH))
+DPDK_SITE_METHOD = local
+else ifeq ($(BR2_PACKAGE_DPDK_CUSTOM_GIT),y)
+DPDK_SITE = $(call qstrip,$(BR2_PACKAGE_DPDK_CUSTOM_REPO_URL))
+DPDK_SITE_METHOD = git
+else
+DPDK_SITE = http://dpdk.org/browse/dpdk/snapshot
+DPDK_SOURCE = dpdk-$(DPDK_VERSION).tar.gz
+ifeq ($(BR2_PACKAGE_DPDK_VERSION_CUSTOM),y)
+BR_NO_CHECK_HASH_FOR += $(DPDK_SOURCE)
+endif
+endif
+
+DPDK_LICENSE = BSD
+DPDK_LICENSE_FILES = LICENSE.LGPL LICENSE.GPL
+DPDK_INSTALL_STAGING = YES
+
+DPDK_DEPENDENCIES += linux
+#DPDK_DEPENDENCIES += $(if $(BR2_PACKAGE_DPDK_PMD_PCAP),libpcap,)
+
+ifeq ($(BR2_SHARED_LIBS),y)
+define DPDK_ENABLE_SHARED_LIBS
+	@echo CONFIG_RTE_BUILD_COMBINE_LIBS=y >> $(@D)/build/.config
+	@echo CONFIG_RTE_BUILD_SHARED_LIB=y >> $(@D)/build/.config
+endef
+
+DPDK_POST_CONFIGURE_HOOKS += DPDK_ENABLE_SHARED_LIBS
+endif
+
+# We're building a kernel module without using the kernel-module infra,
+# so we need to tell we want module support in the kernel
+ifeq ($(BR2_PACKAGE_DPDK),y)
+LINUX_NEEDS_MODULES = y
+endif
+
+
+DPDK_CONFIG = $(call qstrip,$(BR2_PACKAGE_DPDK_CONFIG))
+
+define DPDK_CONFIGURE_CMDS
+	$(MAKE) -C $(@D) T=$(DPDK_CONFIG) RTE_KERNELDIR=$(LINUX_DIR) CROSS=$(TARGET_CROSS) config
+	@ln -sv build $(@D)/$(DPDK_CONFIG) # avoid calling install
+endef
+
+define DPDK_BUILD_CMDS
+	$(MAKE1) -C $(@D) T=$(DPDK_CONFIG) RTE_KERNELDIR=$(LINUX_DIR) CROSS=$(TARGET_CROSS) install
+endef
+
+ifeq ($(BR2_SHARED_LIBS),n)
+# Install static libs only (DPDK compiles either *.so or *.a)
+define DPDK_INSTALL_STAGING_LIBS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
+	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.a $(STAGING_DIR)/usr/lib
+endef
+
+DPDK_INSTALL_TARGET_LIBS =
+else
+# Install shared libs only (DPDK compiles either *.so or *.a)
+define DPDK_INSTALL_STAGING_LIBS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
+	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.so* $(STAGING_DIR)/usr/lib
+endef
+
+define DPDK_INSTALL_TARGET_LIBS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
+	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.so* $(TARGET_DIR)/usr/lib
+endef
+endif
+
+ifeq ($(BR2_PACKAGE_DPDK_TOOLS_PYSCRIPTS),y)
+define DPDK_INSTALL_TARGET_PYSCRIPTS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/bin
+	$(INSTALL) -m 0755 -D $(@D)/tools/dpdk_nic_bind.py $(TARGET_DIR)/usr/bin
+	$(INSTALL) -m 0755 -D $(@D)/tools/cpu_layout.py $(TARGET_DIR)/usr/bin
+endef
+
+DPDK_DEPENDENCIES += python
+endif
+
+ifeq ($(BR2_PACKAGE_DPDK_TOOLS_TESTPMD),y)
+define DPDK_INSTALL_TARGET_TESTPMD
+	$(INSTALL) -m 0755 -D -d $(TARGET_DIR)/usr/bin
+	$(INSTALL) -m 0755 -D $(@D)/build/app/testpmd $(TARGET_DIR)/usr/bin
+endef
+endif
+
+ifeq ($(BR2_PACKAGE_DPDK_TOOLS_TEST),y)
+define DPDK_INSTALL_TARGET_TEST
+	$(INSTALL) -m 0755 -D -d $(TARGET_DIR)/usr/bin
+	$(INSTALL) -m 0755 -D $(@D)/build/app/test $(TARGET_DIR)/usr/bin
+endef
+endif
+
+define DPDK_INSTALL_STAGING_CMDS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/include
+	$(INSTALL) -m 0644 -D $(@D)/build/include/*.h $(STAGING_DIR)/usr/include
+	$(DPDK_INSTALL_STAGING_LIBS)
+endef
+
+define DPDK_INSTALL_TARGET_CMDS
+	$(DPDK_INSTALL_TARGET_LIBS)
+	$(DPDK_INSTALL_TARGET_PYSCRIPTS)
+	$(DPDK_INSTALL_TARGET_TESTPMD)
+	$(DPDK_INSTALL_TARGET_TEST)
+endef
+
+$(eval $(generic-package))
-- 
2.6.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Buildroot] [RFC PATCH] dpdk: new package
  2015-10-30 11:05 [Buildroot] [RFC PATCH] dpdk: new package Jan Viktorin
@ 2015-10-30 17:17 ` Arnout Vandecappelle
  2015-11-02 10:34   ` Jan Viktorin
  2015-12-09 14:11 ` [Buildroot] [PATCH v2 0/3] " Jan Viktorin
  1 sibling, 1 reply; 16+ messages in thread
From: Arnout Vandecappelle @ 2015-10-30 17:17 UTC (permalink / raw)
  To: buildroot

 Hi Jan,

 I didn't do a detailed review, just some general remarks.

On 30-10-15 12:05, Jan Viktorin wrote:
> This patch introduces support of the DPDK library (www.dpdk.org) into
> Buildroot. It compiles and installs DPDK libraries on the target and
> staging and allows to compiler other applications depending on the DPDK
> library. It can also install some basic tools the DPDK provides
> (testpmd, python scripts, test suite).

 Explanation of what DPDK stands for would be nice :-)

> 
> The patch assumes DPDK 2.2 which is not out at the moment (scheduled
> to the end of November). However, this version will contain support for
> the ARM architecture that is currently being reviewed.
> 
> Testing of DPDK on ARM is possible by cloning
> 
>  https://github.com/RehiveTech/dpdk arm-support-v5
> 
> as the package allows customization of the download process.

 We try to avoid adding options to select a version or different source. For
instance, we recently removed the version selection from busybox. If you need
for instance this arm-support branch, you can still use the _OVERRIDE_SRCDIR
approach. Is there any good reason why you need multi-version support for this
package?

 Same for custom patches, we have BR2_GLOBAL_PATCH_DIR.

> 
> The ARM ports can be tested by
> 
>  qemu_aarch64_virt_defconfig
>  qemu_arm_vexpress_defconfig
> 
> I did not test, how it works with x86 as I do not use Buildroot
> for this.
> 
> The included hashes are calculated locally by downloading the tar.gz
> archives by hand.
> 
> There are unfortunately some pitfalls:
> 
> * it may require to enable PCI, MSIX, UIO in the Linux Kernel
>   (some defconfigs does not include as default and it is platform
>   dependent as ARMv7 almost does not use PCI)

 We had a similar situation recently but I don't remember which package it was
and what the conclusion was. But basically, if you don't have PCI, then the
build will end with an error saying that you don't have PCI, so it's not rocket
science to discover that PCI should be enabled.

 UIO I think you could enable explicitly in linux.mk however.

> 
> * when building PCAP PMD driver, the libpcap is required (however,
>   this dependency is difficult to express in Buildroot as this depends
>   on the DPDK configuration file - similar to the issue above)

 Yeah, we have a similar situation with swupdate, and no resolution so far. We
also had a similar situation with the crosstool-NG support before that was removed.

 What you definitely can do is

ifeq ($(BR2_PACKAGE_LIBPCAP),y)
DPDK_DEPENDENCIES += libpcap
endif

and add in the helptext that you need to enable libpcap manually if required.

> 
> * some tools the DPDK provides depend on Python(2) so the user has
>   to enable it to install those
> 
> I assume, the user knows what he is doing in those cases and configures
> the Buildroot in the right way. Or any ideas? Yann?
> 
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>


 Generic other remarks:

- Why not use the kernel-module infra?
- Shouldn't it be a kconfig-package as well?
- Help text line wrapping is not OK.
- Instead of the configuration choice which always has only two options, just
allow to leave the config string empty and default to the arch-appropriate one then.
- pyscripts should probably not require an option; just install automatically if
python is enabled.
- License is probably not correct, if the files are called LICENSE.LGPL LICENSE.GPL
- For enabling kconfig opts, there are helper functions.
- Never put comments in command blocks; put them before the define.
- Don't put empty assignments (DPDK_INSTALL_TARGET_LIBS)



 Regards,
 Arnout

[snip]
-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [RFC PATCH] dpdk: new package
  2015-10-30 17:17 ` Arnout Vandecappelle
@ 2015-11-02 10:34   ` Jan Viktorin
  0 siblings, 0 replies; 16+ messages in thread
From: Jan Viktorin @ 2015-11-02 10:34 UTC (permalink / raw)
  To: buildroot

Hello Arnout,

thanks for your quick review...

On Fri, 30 Oct 2015 18:17:09 +0100
Arnout Vandecappelle <arnout@mind.be> wrote:

>  Hi Jan,
> 
>  I didn't do a detailed review, just some general remarks.
> 
> On 30-10-15 12:05, Jan Viktorin wrote:
> > This patch introduces support of the DPDK library (www.dpdk.org) into
> > Buildroot. It compiles and installs DPDK libraries on the target and
> > staging and allows to compiler other applications depending on the DPDK
> > library. It can also install some basic tools the DPDK provides
> > (testpmd, python scripts, test suite).  
> 
>  Explanation of what DPDK stands for would be nice :-)

OK, will improve ;).

> 
> > 
> > The patch assumes DPDK 2.2 which is not out at the moment (scheduled
> > to the end of November). However, this version will contain support for
> > the ARM architecture that is currently being reviewed.
> > 
> > Testing of DPDK on ARM is possible by cloning
> > 
> >  https://github.com/RehiveTech/dpdk arm-support-v5
> > 
> > as the package allows customization of the download process.  
> 
>  We try to avoid adding options to select a version or different source. For
> instance, we recently removed the version selection from busybox. If you need
> for instance this arm-support branch, you can still use the _OVERRIDE_SRCDIR
> approach.

Thanks for this hint. It is a little complication (I like when
everything is in the defconfig - a single file) for some internal
things. But I'll go this way in the next version.

> Is there any good reason why you need multi-version support for this
> package?

Probably not. all these were just for my comfort during develpoment,
patch management, etc.

> 
>  Same for custom patches, we have BR2_GLOBAL_PATCH_DIR.

Of course, I don't need this because of git...

> 
> > 
> > The ARM ports can be tested by
> > 
> >  qemu_aarch64_virt_defconfig
> >  qemu_arm_vexpress_defconfig
> > 
> > I did not test, how it works with x86 as I do not use Buildroot
> > for this.
> > 
> > The included hashes are calculated locally by downloading the tar.gz
> > archives by hand.
> > 
> > There are unfortunately some pitfalls:
> > 
> > * it may require to enable PCI, MSIX, UIO in the Linux Kernel
> >   (some defconfigs does not include as default and it is platform
> >   dependent as ARMv7 almost does not use PCI)  
> 
>  We had a similar situation recently but I don't remember which package it was
> and what the conclusion was. But basically, if you don't have PCI, then the
> build will end with an error saying that you don't have PCI, so it's not rocket
> science to discover that PCI should be enabled.

Yes, it's just annoying.

> 
>  UIO I think you could enable explicitly in linux.mk however.

You probably mean something like(?):

linux.mk:
$(if $(BR_PACKAGE_DPDK),
    $(call KCONFIG_ENABLE_OPT,CONFIG_XXX_UIO,$(@D)/.config))

> 
> > 
> > * when building PCAP PMD driver, the libpcap is required (however,
> >   this dependency is difficult to express in Buildroot as this depends
> >   on the DPDK configuration file - similar to the issue above)  
> 
>  Yeah, we have a similar situation with swupdate, and no resolution so far. We
> also had a similar situation with the crosstool-NG support before that was removed.
> 
>  What you definitely can do is
> 
> ifeq ($(BR2_PACKAGE_LIBPCAP),y)
> DPDK_DEPENDENCIES += libpcap
> endif
> 
> and add in the helptext that you need to enable libpcap manually if required.

OK.

> 
> > 
> > * some tools the DPDK provides depend on Python(2) so the user has
> >   to enable it to install those
> > 
> > I assume, the user knows what he is doing in those cases and configures
> > the Buildroot in the right way. Or any ideas? Yann?
> > 
> > Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>  
> 
> 
>  Generic other remarks:
> 
> - Why not use the kernel-module infra?

The DPDK project has a specific build system which is better to not
touch :). I tried several approaches and finally, I finished with the
proposed patch as the one that is clean and working.

> - Shouldn't it be a kconfig-package as well?

Unfortunately, the DPDK project does not use kconfig at the moment (it
probably will in the future but...). So the kconfig-package cannot be
used here.

> - Help text line wrapping is not OK.
> - Instead of the configuration choice which always has only two options, just
> allow to leave the config string empty and default to the arch-appropriate one then.
> - pyscripts should probably not require an option; just install automatically if
> python is enabled.
> - License is probably not correct, if the files are called LICENSE.LGPL LICENSE.GPL

The DPDK itself is under BSD (however, I did not find the license
file there). The included kernel drivers are GPL. I have no idea about
the purpose of LGPL at the moment (probably dual-licensing, will
check...).

> - For enabling kconfig opts, there are helper functions.
> - Never put comments in command blocks; put them before the define.

OK, will fix.

> - Don't put empty assignments (DPDK_INSTALL_TARGET_LIBS)

OK. I wanted to make obvious that it is empty. I will remove it.

> 
> 
> 
>  Regards,
>  Arnout
> 
> [snip]



-- 
  Jan Viktorin                E-mail: Viktorin at RehiveTech.com
  System Architect            Web:    www.RehiveTech.com
  RehiveTech
  Brno, Czech Republic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 0/3] dpdk: new package
  2015-10-30 11:05 [Buildroot] [RFC PATCH] dpdk: new package Jan Viktorin
  2015-10-30 17:17 ` Arnout Vandecappelle
@ 2015-12-09 14:11 ` Jan Viktorin
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 1/3] python-ptyprocess: " Jan Viktorin
                     ` (2 more replies)
  1 sibling, 3 replies; 16+ messages in thread
From: Jan Viktorin @ 2015-12-09 14:11 UTC (permalink / raw)
  To: buildroot

This patch series introduces the DPDK package into Buildroot. The DPDK
library (www.dpdk.org) allows high-speed packet sending/receiving
while bypassing the Linux Kernel. It allows to reach a high throughput
for 10-100 Gbps networks on the x86 platform.

The ARM ports can be tested by

 qemu_aarch64_virt_defconfig
 qemu_arm_vexpress_defconfig

I did not test, how it works with x86 as I do not use Buildroot
for this.

There are unfortunately some pitfalls:

* There is a new install infra in DPDK that is not reflected in
  this patch set. I will send v3 of this patch set that uses the
  new install infra after I make the intergration.

* The DPDK may require to enable PCI, MSIX, UIO in the Linux Kernel
  (some defconfigs does not include as default and it is platform
  dependent as ARMv7 almost does not use PCI).

* When building PCAP PMD driver, the libpcap is required (partially
  fixed as suggested by Arnout).

* Some tools the DPDK provides depend on Python(2) so the user has
  to enable it to install those.

The DPDK tests require pexpect and ptyprocess python packages to run
so they are included.

Jan Viktorin (3):
  python-ptyprocess: new package
  python-pexpect: new package
  dpdk: new package

 package/Config.in                              |   3 +
 package/dpdk/Config.in                         |  47 +++++++++++
 package/dpdk/dpdk.hash                         |   4 +
 package/dpdk/dpdk.mk                           | 112 +++++++++++++++++++++++++
 package/python-pexpect/Config.in               |  10 +++
 package/python-pexpect/python-pexpect.mk       |  14 ++++
 package/python-ptyprocess/Config.in            |   7 ++
 package/python-ptyprocess/python-ptyprocess.mk |  13 +++
 8 files changed, 210 insertions(+)
 create mode 100644 package/dpdk/Config.in
 create mode 100644 package/dpdk/dpdk.hash
 create mode 100644 package/dpdk/dpdk.mk
 create mode 100644 package/python-pexpect/Config.in
 create mode 100644 package/python-pexpect/python-pexpect.mk
 create mode 100644 package/python-ptyprocess/Config.in
 create mode 100644 package/python-ptyprocess/python-ptyprocess.mk

-- 
2.6.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 1/3] python-ptyprocess: new package
  2015-12-09 14:11 ` [Buildroot] [PATCH v2 0/3] " Jan Viktorin
@ 2015-12-09 14:11   ` Jan Viktorin
  2015-12-09 14:16     ` Yegor Yefremov
  2015-12-09 14:17     ` Vicente Olivert Riera
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 2/3] python-pexpect: " Jan Viktorin
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 3/3] dpdk: " Jan Viktorin
  2 siblings, 2 replies; 16+ messages in thread
From: Jan Viktorin @ 2015-12-09 14:11 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
---
 package/Config.in                              |  1 +
 package/python-ptyprocess/Config.in            |  7 +++++++
 package/python-ptyprocess/python-ptyprocess.mk | 13 +++++++++++++
 3 files changed, 21 insertions(+)
 create mode 100644 package/python-ptyprocess/Config.in
 create mode 100644 package/python-ptyprocess/python-ptyprocess.mk

diff --git a/package/Config.in b/package/Config.in
index 2bdad01..69bc347 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -642,6 +642,7 @@ menu "External python modules"
 	source "package/python-posix-ipc/Config.in"
 	source "package/python-protobuf/Config.in"
 	source "package/python-psutil/Config.in"
+	source "package/python-ptyprocess/Config.in"
 	source "package/python-pyasn/Config.in"
 	source "package/python-pycli/Config.in"
 	source "package/python-pycrypto/Config.in"
diff --git a/package/python-ptyprocess/Config.in b/package/python-ptyprocess/Config.in
new file mode 100644
index 0000000..abf8045
--- /dev/null
+++ b/package/python-ptyprocess/Config.in
@@ -0,0 +1,7 @@
+config BR2_PACKAGE_PYTHON_PTYPROCESS
+	bool "python-ptyprocess"
+	depends on BR2_PACKAGE_PYTHON
+	help
+	  Launch a subprocess in a pseudo terminal (pty), and interact with both the process and its pty.
+
+	  https://github.com/pexpect/ptyprocess
diff --git a/package/python-ptyprocess/python-ptyprocess.mk b/package/python-ptyprocess/python-ptyprocess.mk
new file mode 100644
index 0000000..9e6b762
--- /dev/null
+++ b/package/python-ptyprocess/python-ptyprocess.mk
@@ -0,0 +1,13 @@
+################################################################################
+#
+# python-ptyprocess
+#
+################################################################################
+
+PYTHON_PTYPROCESS_VERSION = 0.5
+PYTHON_PTYPROCESS_SITE = $(call github,pexpect,ptyprocess,$(PYTHON_PTYPROCESS_VERSION))
+PYTHON_PTYPROCESS_LICENSE = ISC
+PYTHON_PTYPROCESS_LICENSE_FILES = LICENSE
+PYTHON_PTYPROCESS_SETUP_TYPE = distutils
+
+$(eval $(python-package))
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 2/3] python-pexpect: new package
  2015-12-09 14:11 ` [Buildroot] [PATCH v2 0/3] " Jan Viktorin
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 1/3] python-ptyprocess: " Jan Viktorin
@ 2015-12-09 14:11   ` Jan Viktorin
  2015-12-09 14:18     ` Yegor Yefremov
  2015-12-09 14:19     ` Vicente Olivert Riera
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 3/3] dpdk: " Jan Viktorin
  2 siblings, 2 replies; 16+ messages in thread
From: Jan Viktorin @ 2015-12-09 14:11 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
---
 package/Config.in                        |  1 +
 package/python-pexpect/Config.in         | 10 ++++++++++
 package/python-pexpect/python-pexpect.mk | 14 ++++++++++++++
 3 files changed, 25 insertions(+)
 create mode 100644 package/python-pexpect/Config.in
 create mode 100644 package/python-pexpect/python-pexpect.mk

diff --git a/package/Config.in b/package/Config.in
index 69bc347..77a92d2 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -638,6 +638,7 @@ menu "External python modules"
 	source "package/python-networkmanager/Config.in"
 	source "package/python-nfc/Config.in"
 	source "package/python-numpy/Config.in"
+	source "package/python-pexpect/Config.in"
 	source "package/python-pam/Config.in"
 	source "package/python-posix-ipc/Config.in"
 	source "package/python-protobuf/Config.in"
diff --git a/package/python-pexpect/Config.in b/package/python-pexpect/Config.in
new file mode 100644
index 0000000..a42aec7
--- /dev/null
+++ b/package/python-pexpect/Config.in
@@ -0,0 +1,10 @@
+config BR2_PACKAGE_PYTHON_PEXPECT
+	bool "python-pexpect"
+	depends on BR2_PACKAGE_PYTHON
+	select BR2_PACKAGE_PYTHON_PTYPROCESS
+	help
+	  Pexpect is a pure Python module for spawning child applications; controlling them; and responding to
+	  expected patterns in their output. Pexpect works like Don Libes? Expect. Pexpect allows your script
+	  to spawn a child application and control it as if a human were typing commands.
+
+	  https://pexpect.readthedocs.org
diff --git a/package/python-pexpect/python-pexpect.mk b/package/python-pexpect/python-pexpect.mk
new file mode 100644
index 0000000..9de7849
--- /dev/null
+++ b/package/python-pexpect/python-pexpect.mk
@@ -0,0 +1,14 @@
+################################################################################
+#
+# python-pexpect
+#
+################################################################################
+
+PYTHON_PEXPECT_VERSION = 4.0.1
+PYTHON_PEXPECT_SITE = $(call github,pexpect,pexpect,$(PYTHON_PEXPECT_VERSION))
+PYTHON_PEXPECT_LICENSE = ISC
+PYTHON_PEXPECT_LICENSE_FILES = LICENSE
+PYTHON_PEXPECT_SETUP_TYPE = distutils
+PYTHON_PEXPECT_DEPENDENCIES = python-ptyprocess
+
+$(eval $(python-package))
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 3/3] dpdk: new package
  2015-12-09 14:11 ` [Buildroot] [PATCH v2 0/3] " Jan Viktorin
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 1/3] python-ptyprocess: " Jan Viktorin
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 2/3] python-pexpect: " Jan Viktorin
@ 2015-12-09 14:11   ` Jan Viktorin
  2015-12-09 22:36     ` Thomas Petazzoni
  2 siblings, 1 reply; 16+ messages in thread
From: Jan Viktorin @ 2015-12-09 14:11 UTC (permalink / raw)
  To: buildroot

This patch introduces support of the DPDK library (www.dpdk.org) into
Buildroot. DPDK is a library for high-speed packet sending/receiving
while bypassing the Linux Kernel. It allows to reach a high throughput
for 10-100 Gbps networks on the x86 platform.

The package compiles and installs DPDK libraries on the target and
staging and allows to compiler other applications depending on the DPDK
library. It can also install some basic tools the DPDK provides
(testpmd, python scripts, test suite). The patch assumes DPDK 2.2.0 (rc3)
which introduces the ARM architecture support.

The included hashes are calculated locally by downloading the tar.gz
archives by hand.

The DPDK project is licensed under BSD license (see http://dpdk.org/),
however, there is no license file in the root of the project. Moreover,
there are parts (Linux Kernel modules) which are GPLv2. So the license
specification might look strange.

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
---
v2: (mostly suggestions by Arnout)
* simplified Config.in - avoid version, source and config selection
* improved dependency on libpcap
* user python scripts are included if python package is enabled
* installing of tests includes autotest*.py scripts (we do not care
  about the python here, assuming the user will install it manually
  when testing)
* minor coding style fixes
* depends on python-pexpect package
* using version 2.2.0-rc3
---
 package/Config.in      |   1 +
 package/dpdk/Config.in |  47 +++++++++++++++++++++
 package/dpdk/dpdk.hash |   4 ++
 package/dpdk/dpdk.mk   | 112 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 164 insertions(+)
 create mode 100644 package/dpdk/Config.in
 create mode 100644 package/dpdk/dpdk.hash
 create mode 100644 package/dpdk/dpdk.mk

diff --git a/package/Config.in b/package/Config.in
index 77a92d2..fd07d5d 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -999,6 +999,7 @@ menu "Networking"
 	source "package/cgic/Config.in"
 	source "package/cppzmq/Config.in"
 	source "package/czmq/Config.in"
+	source "package/dpdk/Config.in"
 	source "package/filemq/Config.in"
 	source "package/flickcurl/Config.in"
 	source "package/fmlib/Config.in"
diff --git a/package/dpdk/Config.in b/package/dpdk/Config.in
new file mode 100644
index 0000000..a4935ae
--- /dev/null
+++ b/package/dpdk/Config.in
@@ -0,0 +1,47 @@
+config BR2_PACKAGE_DPDK
+       bool "dpdk"
+       depends on BR2_x86_i686 || BR2_x86_64 || BR2_powerpc_power8 || BR2_ARM_CPU_ARMV7A || BR2_aarch64 || BR2_aarch64_be
+       depends on BR2_TOOLCHAIN_USES_GLIBC
+       help
+         DPDK is a set of libraries and drivers for fast packet processing. It was designed to run on
+         any processors knowing Intel x86 has been the first CPU to be supported. Ports for other CPUs
+         like IBM Power 8 and ARM are under progress. It runs mostly in Linux userland. A FreeBSD port
+         is now available for a subset of DPDK features.
+
+	 Notes:
+	 * To build the included Linux Kernel drivers, it is necessary to enable CONFIG_PCI_MSI,
+	   CONFIG_UIO.
+	 * To build the PCAP PMD properly, you need to enable the libpcap manually.
+	 * To install the python scripts (dpdk_nic_bind.py, cpu_layout.py), enable
+	   the python2 interpreter for the target.
+
+         http://www.dpdk.org/
+
+if BR2_PACKAGE_DPDK
+
+config BR2_PACKAGE_DPDK_CONFIG
+	string "Configuration"
+	default "i686-native-linuxapp-gcc" \
+		if BR2_x86_i686
+	default "x86_64-native-linuxapp-gcc" \
+		if BR2_x86_64
+	default "ppc_64-power8-native-linuxapp-gcc" \
+		if BR2_powerpc_power8
+	default "arm-armv7a-linuxapp-gcc" \
+		if BR2_ARM_CPU_ARMV7A
+	default "arm64-armv8a-linuxapp-gcc" \
+		if BR2_aarch64 || BR2_aarch64_be
+
+config BR2_PACKAGE_DPDK_TOOLS_TESTPMD
+	bool "Install testpmd"
+	default y
+	help
+	  Install application for general testing of DPDK and PMD drivers.
+
+config BR2_PACKAGE_DPDK_TOOLS_TEST
+	bool "Install tests suite"
+	help
+	  Install all DPDK tests. If you want to run the tests by the included
+	  autotest.py script you need to enable python manually.
+
+endif
diff --git a/package/dpdk/dpdk.hash b/package/dpdk/dpdk.hash
new file mode 100644
index 0000000..c78ec85
--- /dev/null
+++ b/package/dpdk/dpdk.hash
@@ -0,0 +1,4 @@
+# Locally calculated
+sha256	f7b322867a45f99afd9c8fbacdc56e1621676f9ca0f046656ec85eb6a99a3440  dpdk-2.1.0.tar.gz
+sha256	530074d4eaefe1f717e7411e6a74e4ba0fa619af304c5e74e1097e51d33cc19e  dpdk-2.2.0-rc2.tar.gz
+sha256	7caf52554c0f724a09e9342ee6670b324a77dade5cd0b96ff5b66957ed1bc1f9  dpdk-2.2.0-rc3.tar.gz
diff --git a/package/dpdk/dpdk.mk b/package/dpdk/dpdk.mk
new file mode 100644
index 0000000..2d8a5a3
--- /dev/null
+++ b/package/dpdk/dpdk.mk
@@ -0,0 +1,112 @@
+################################################################################
+#
+# dpdk
+#
+################################################################################
+
+DPDK_VERSION = 2.2.0-rc3
+DPDK_SITE = http://dpdk.org/browse/dpdk/snapshot
+DPDK_SOURCE = dpdk-$(DPDK_VERSION).tar.gz
+
+DPDK_LICENSE = BSD
+DPDK_LICENSE_FILES = LICENSE.LGPL LICENSE.GPL
+DPDK_INSTALL_STAGING = YES
+
+DPDK_DEPENDENCIES += linux
+
+ifeq ($(BR2_PACKAGE_LIBPCAP),y)
+DPDK_DEPENDENCIES += libpcap
+endif
+
+ifeq ($(BR2_SHARED_LIBS),y)
+define DPDK_ENABLE_SHARED_LIBS
+	@echo CONFIG_RTE_BUILD_COMBINE_LIBS=y >> $(@D)/build/.config
+	@echo CONFIG_RTE_BUILD_SHARED_LIB=y >> $(@D)/build/.config
+endef
+
+DPDK_POST_CONFIGURE_HOOKS += DPDK_ENABLE_SHARED_LIBS
+endif
+
+# We're building a kernel module without using the kernel-module infra,
+# so we need to tell we want module support in the kernel
+ifeq ($(BR2_PACKAGE_DPDK),y)
+LINUX_NEEDS_MODULES = y
+endif
+
+
+DPDK_CONFIG = $(call qstrip,$(BR2_PACKAGE_DPDK_CONFIG))
+
+# We create symlink named $(DPDK_CONFIG) to the build directory
+# to avoid calling install which behaves strange in DPDK build system.
+define DPDK_CONFIGURE_CMDS
+	$(MAKE) -C $(@D) T=$(DPDK_CONFIG) RTE_KERNELDIR=$(LINUX_DIR) CROSS=$(TARGET_CROSS) config
+	@ln -sv build $(@D)/$(DPDK_CONFIG)
+endef
+
+define DPDK_BUILD_CMDS
+	$(MAKE1) -C $(@D) T=$(DPDK_CONFIG) RTE_KERNELDIR=$(LINUX_DIR) CROSS=$(TARGET_CROSS) install
+endef
+
+ifeq ($(BR2_SHARED_LIBS),n)
+# Install static libs only (DPDK compiles either *.so or *.a)
+define DPDK_INSTALL_STAGING_LIBS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
+	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.a $(STAGING_DIR)/usr/lib
+endef
+
+else
+# Install shared libs only (DPDK compiles either *.so or *.a)
+define DPDK_INSTALL_STAGING_LIBS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
+	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.so* $(STAGING_DIR)/usr/lib
+endef
+
+define DPDK_INSTALL_TARGET_LIBS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
+	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.so* $(TARGET_DIR)/usr/lib
+endef
+endif
+
+ifeq ($(BR2_PACKAGE_PYTHON),y)
+define DPDK_INSTALL_TARGET_PYSCRIPTS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/bin
+	$(INSTALL) -m 0755 -D $(@D)/tools/dpdk_nic_bind.py $(TARGET_DIR)/usr/bin
+	$(INSTALL) -m 0755 -D $(@D)/tools/cpu_layout.py $(TARGET_DIR)/usr/bin
+endef
+
+DPDK_DEPENDENCIES += python
+endif
+
+ifeq ($(BR2_PACKAGE_DPDK_TOOLS_TESTPMD),y)
+define DPDK_INSTALL_TARGET_TESTPMD
+	$(INSTALL) -m 0755 -D -d $(TARGET_DIR)/usr/bin
+	$(INSTALL) -m 0755 -D $(@D)/build/app/testpmd $(TARGET_DIR)/usr/bin
+endef
+endif
+
+ifeq ($(BR2_PACKAGE_DPDK_TOOLS_TEST),y)
+define DPDK_INSTALL_TARGET_TEST
+	$(INSTALL) -m 0755 -D -d $(TARGET_DIR)/usr/dpdk
+	$(INSTALL) -m 0755 -D $(@D)/build/app/test $(TARGET_DIR)/usr/dpdk
+	$(INSTALL) -m 0755 -D $(@D)/app/test/*.py $(TARGET_DIR)/usr/dpdk
+endef
+
+ifeq ($(BR2_PACKAGE_PYTHON),y)
+DPDK_DEPENDENCIES += python-pexpect
+endif
+endif
+
+define DPDK_INSTALL_STAGING_CMDS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/include
+	$(INSTALL) -m 0644 -D $(@D)/build/include/*.h $(STAGING_DIR)/usr/include
+	$(DPDK_INSTALL_STAGING_LIBS)
+endef
+
+define DPDK_INSTALL_TARGET_CMDS
+	$(DPDK_INSTALL_TARGET_LIBS)
+	$(DPDK_INSTALL_TARGET_PYSCRIPTS)
+	$(DPDK_INSTALL_TARGET_TESTPMD)
+	$(DPDK_INSTALL_TARGET_TEST)
+endef
+
+$(eval $(generic-package))
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 1/3] python-ptyprocess: new package
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 1/3] python-ptyprocess: " Jan Viktorin
@ 2015-12-09 14:16     ` Yegor Yefremov
  2015-12-09 14:17     ` Vicente Olivert Riera
  1 sibling, 0 replies; 16+ messages in thread
From: Yegor Yefremov @ 2015-12-09 14:16 UTC (permalink / raw)
  To: buildroot

Hi Jan,

On Wed, Dec 9, 2015 at 3:11 PM, Jan Viktorin <viktorin@rehivetech.com> wrote:
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> ---
>  package/Config.in                              |  1 +
>  package/python-ptyprocess/Config.in            |  7 +++++++
>  package/python-ptyprocess/python-ptyprocess.mk | 13 +++++++++++++
>  3 files changed, 21 insertions(+)
>  create mode 100644 package/python-ptyprocess/Config.in
>  create mode 100644 package/python-ptyprocess/python-ptyprocess.mk
>
> diff --git a/package/Config.in b/package/Config.in
> index 2bdad01..69bc347 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -642,6 +642,7 @@ menu "External python modules"
>         source "package/python-posix-ipc/Config.in"
>         source "package/python-protobuf/Config.in"
>         source "package/python-psutil/Config.in"
> +       source "package/python-ptyprocess/Config.in"
>         source "package/python-pyasn/Config.in"
>         source "package/python-pycli/Config.in"
>         source "package/python-pycrypto/Config.in"
> diff --git a/package/python-ptyprocess/Config.in b/package/python-ptyprocess/Config.in
> new file mode 100644
> index 0000000..abf8045
> --- /dev/null
> +++ b/package/python-ptyprocess/Config.in
> @@ -0,0 +1,7 @@
> +config BR2_PACKAGE_PYTHON_PTYPROCESS
> +       bool "python-ptyprocess"
> +       depends on BR2_PACKAGE_PYTHON

setup.py says, that this package can run under Python 3 too. So remove
this dependency.

> +       help
> +         Launch a subprocess in a pseudo terminal (pty), and interact with both the process and its pty.

Line too long.

> +
> +         https://github.com/pexpect/ptyprocess
> diff --git a/package/python-ptyprocess/python-ptyprocess.mk b/package/python-ptyprocess/python-ptyprocess.mk
> new file mode 100644
> index 0000000..9e6b762
> --- /dev/null
> +++ b/package/python-ptyprocess/python-ptyprocess.mk
> @@ -0,0 +1,13 @@
> +################################################################################
> +#
> +# python-ptyprocess
> +#
> +################################################################################
> +
> +PYTHON_PTYPROCESS_VERSION = 0.5
> +PYTHON_PTYPROCESS_SITE = $(call github,pexpect,ptyprocess,$(PYTHON_PTYPROCESS_VERSION))
> +PYTHON_PTYPROCESS_LICENSE = ISC
> +PYTHON_PTYPROCESS_LICENSE_FILES = LICENSE
> +PYTHON_PTYPROCESS_SETUP_TYPE = distutils
> +
> +$(eval $(python-package))
> --
> 2.6.3
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 1/3] python-ptyprocess: new package
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 1/3] python-ptyprocess: " Jan Viktorin
  2015-12-09 14:16     ` Yegor Yefremov
@ 2015-12-09 14:17     ` Vicente Olivert Riera
  1 sibling, 0 replies; 16+ messages in thread
From: Vicente Olivert Riera @ 2015-12-09 14:17 UTC (permalink / raw)
  To: buildroot

Dear Jan Viktorin,

On 09/12/15 14:11, Jan Viktorin wrote:

[snip]

> diff --git a/package/python-ptyprocess/Config.in b/package/python-ptyprocess/Config.in
> new file mode 100644
> index 0000000..abf8045
> --- /dev/null
> +++ b/package/python-ptyprocess/Config.in
> @@ -0,0 +1,7 @@
> +config BR2_PACKAGE_PYTHON_PTYPROCESS
> +	bool "python-ptyprocess"
> +	depends on BR2_PACKAGE_PYTHON
> +	help
> +	  Launch a subprocess in a pseudo terminal (pty), and interact with both the process and its pty.

Wrap that line at 72 characters length, taking into account that we
consider a tab as 8 characters wide.

Regards,

Vincent.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 2/3] python-pexpect: new package
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 2/3] python-pexpect: " Jan Viktorin
@ 2015-12-09 14:18     ` Yegor Yefremov
  2015-12-09 14:21       ` Vicente Olivert Riera
  2015-12-09 14:34       ` Jan Viktorin
  2015-12-09 14:19     ` Vicente Olivert Riera
  1 sibling, 2 replies; 16+ messages in thread
From: Yegor Yefremov @ 2015-12-09 14:18 UTC (permalink / raw)
  To: buildroot

On Wed, Dec 9, 2015 at 3:11 PM, Jan Viktorin <viktorin@rehivetech.com> wrote:
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> ---
>  package/Config.in                        |  1 +
>  package/python-pexpect/Config.in         | 10 ++++++++++
>  package/python-pexpect/python-pexpect.mk | 14 ++++++++++++++
>  3 files changed, 25 insertions(+)
>  create mode 100644 package/python-pexpect/Config.in
>  create mode 100644 package/python-pexpect/python-pexpect.mk
>
> diff --git a/package/Config.in b/package/Config.in
> index 69bc347..77a92d2 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -638,6 +638,7 @@ menu "External python modules"
>         source "package/python-networkmanager/Config.in"
>         source "package/python-nfc/Config.in"
>         source "package/python-numpy/Config.in"
> +       source "package/python-pexpect/Config.in"
>         source "package/python-pam/Config.in"
>         source "package/python-posix-ipc/Config.in"
>         source "package/python-protobuf/Config.in"
> diff --git a/package/python-pexpect/Config.in b/package/python-pexpect/Config.in
> new file mode 100644
> index 0000000..a42aec7
> --- /dev/null
> +++ b/package/python-pexpect/Config.in
> @@ -0,0 +1,10 @@
> +config BR2_PACKAGE_PYTHON_PEXPECT
> +       bool "python-pexpect"
> +       depends on BR2_PACKAGE_PYTHON

remove BR2_PACKAGE_PYTHON dependency

> +       select BR2_PACKAGE_PYTHON_PTYPROCESS
> +       help
> +         Pexpect is a pure Python module for spawning child applications; controlling them; and responding to
> +         expected patterns in their output. Pexpect works like Don Libes? Expect. Pexpect allows your script
> +         to spawn a child application and control it as if a human were typing commands.

Lines must be max. 80 characters long

> +
> +         https://pexpect.readthedocs.org
> diff --git a/package/python-pexpect/python-pexpect.mk b/package/python-pexpect/python-pexpect.mk
> new file mode 100644
> index 0000000..9de7849
> --- /dev/null
> +++ b/package/python-pexpect/python-pexpect.mk
> @@ -0,0 +1,14 @@
> +################################################################################
> +#
> +# python-pexpect
> +#
> +################################################################################
> +
> +PYTHON_PEXPECT_VERSION = 4.0.1
> +PYTHON_PEXPECT_SITE = $(call github,pexpect,pexpect,$(PYTHON_PEXPECT_VERSION))
> +PYTHON_PEXPECT_LICENSE = ISC
> +PYTHON_PEXPECT_LICENSE_FILES = LICENSE
> +PYTHON_PEXPECT_SETUP_TYPE = distutils
> +PYTHON_PEXPECT_DEPENDENCIES = python-ptyprocess

is this a run-time or build dependency?

> +
> +$(eval $(python-package))
> --
> 2.6.3
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 2/3] python-pexpect: new package
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 2/3] python-pexpect: " Jan Viktorin
  2015-12-09 14:18     ` Yegor Yefremov
@ 2015-12-09 14:19     ` Vicente Olivert Riera
  1 sibling, 0 replies; 16+ messages in thread
From: Vicente Olivert Riera @ 2015-12-09 14:19 UTC (permalink / raw)
  To: buildroot

Dear Jan Viktorin,

On 09/12/15 14:11, Jan Viktorin wrote:
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> ---
>  package/Config.in                        |  1 +
>  package/python-pexpect/Config.in         | 10 ++++++++++
>  package/python-pexpect/python-pexpect.mk | 14 ++++++++++++++
>  3 files changed, 25 insertions(+)
>  create mode 100644 package/python-pexpect/Config.in
>  create mode 100644 package/python-pexpect/python-pexpect.mk
> 
> diff --git a/package/Config.in b/package/Config.in
> index 69bc347..77a92d2 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -638,6 +638,7 @@ menu "External python modules"
>  	source "package/python-networkmanager/Config.in"
>  	source "package/python-nfc/Config.in"
>  	source "package/python-numpy/Config.in"
> +	source "package/python-pexpect/Config.in"

This should be alphabetically ordered.

>  	source "package/python-pam/Config.in"
>  	source "package/python-posix-ipc/Config.in"
>  	source "package/python-protobuf/Config.in"
> diff --git a/package/python-pexpect/Config.in b/package/python-pexpect/Config.in
> new file mode 100644
> index 0000000..a42aec7
> --- /dev/null
> +++ b/package/python-pexpect/Config.in
> @@ -0,0 +1,10 @@
> +config BR2_PACKAGE_PYTHON_PEXPECT
> +	bool "python-pexpect"
> +	depends on BR2_PACKAGE_PYTHON
> +	select BR2_PACKAGE_PYTHON_PTYPROCESS
> +	help
> +	  Pexpect is a pure Python module for spawning child applications; controlling them; and responding to
> +	  expected patterns in their output. Pexpect works like Don Libes? Expect. Pexpect allows your script
> +	  to spawn a child application and control it as if a human were typing commands.

Wrap that line to 72 characters length, taking into account that we
consider a tab as 8 characters wide.

Regards,

Vincent.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 2/3] python-pexpect: new package
  2015-12-09 14:18     ` Yegor Yefremov
@ 2015-12-09 14:21       ` Vicente Olivert Riera
  2015-12-09 14:34       ` Jan Viktorin
  1 sibling, 0 replies; 16+ messages in thread
From: Vicente Olivert Riera @ 2015-12-09 14:21 UTC (permalink / raw)
  To: buildroot

Hello Yegor and Jan,

On 09/12/15 14:18, Yegor Yefremov wrote:
>> diff --git a/package/python-pexpect/Config.in b/package/python-pexpect/Config.in
>> new file mode 100644
>> index 0000000..a42aec7
>> --- /dev/null
>> +++ b/package/python-pexpect/Config.in
>> @@ -0,0 +1,10 @@
>> +config BR2_PACKAGE_PYTHON_PEXPECT
>> +       bool "python-pexpect"
>> +       depends on BR2_PACKAGE_PYTHON
> 
> remove BR2_PACKAGE_PYTHON dependency
> 
>> +       select BR2_PACKAGE_PYTHON_PTYPROCESS
>> +       help
>> +         Pexpect is a pure Python module for spawning child applications; controlling them; and responding to
>> +         expected patterns in their output. Pexpect works like Don Libes? Expect. Pexpect allows your script
>> +         to spawn a child application and control it as if a human were typing commands.
> 
> Lines must be max. 80 characters long

72.

Regards,

Vincent.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 2/3] python-pexpect: new package
  2015-12-09 14:18     ` Yegor Yefremov
  2015-12-09 14:21       ` Vicente Olivert Riera
@ 2015-12-09 14:34       ` Jan Viktorin
  1 sibling, 0 replies; 16+ messages in thread
From: Jan Viktorin @ 2015-12-09 14:34 UTC (permalink / raw)
  To: buildroot

On Wed, 9 Dec 2015 15:18:40 +0100
Yegor Yefremov <yegorslists@googlemail.com> wrote:

> On Wed, Dec 9, 2015 at 3:11 PM, Jan Viktorin <viktorin@rehivetech.com> wrote:
> > Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> > ---
> >  package/Config.in                        |  1 +
> >  package/python-pexpect/Config.in         | 10 ++++++++++
> >  package/python-pexpect/python-pexpect.mk | 14 ++++++++++++++
> >  3 files changed, 25 insertions(+)
> >  create mode 100644 package/python-pexpect/Config.in
> >  create mode 100644 package/python-pexpect/python-pexpect.mk
> >
> > diff --git a/package/Config.in b/package/Config.in
> > index 69bc347..77a92d2 100644
> > --- a/package/Config.in
> > +++ b/package/Config.in
> > @@ -638,6 +638,7 @@ menu "External python modules"
> >         source "package/python-networkmanager/Config.in"
> >         source "package/python-nfc/Config.in"
> >         source "package/python-numpy/Config.in"
> > +       source "package/python-pexpect/Config.in"
> >         source "package/python-pam/Config.in"
> >         source "package/python-posix-ipc/Config.in"
> >         source "package/python-protobuf/Config.in"
> > diff --git a/package/python-pexpect/Config.in b/package/python-pexpect/Config.in
> > new file mode 100644
> > index 0000000..a42aec7
> > --- /dev/null
> > +++ b/package/python-pexpect/Config.in
> > @@ -0,0 +1,10 @@
> > +config BR2_PACKAGE_PYTHON_PEXPECT
> > +       bool "python-pexpect"
> > +       depends on BR2_PACKAGE_PYTHON  
> 
> remove BR2_PACKAGE_PYTHON dependency

OK, fixed for v3.

> 
> > +       select BR2_PACKAGE_PYTHON_PTYPROCESS
> > +       help
> > +         Pexpect is a pure Python module for spawning child applications; controlling them; and responding to
> > +         expected patterns in their output. Pexpect works like Don Libes? Expect. Pexpect allows your script
> > +         to spawn a child application and control it as if a human were typing commands.  
> 
> Lines must be max. 80 characters long

OK, fixed everywhere... sorry, thanks ;).

> 
> > +
> > +         https://pexpect.readthedocs.org
> > diff --git a/package/python-pexpect/python-pexpect.mk b/package/python-pexpect/python-pexpect.mk
> > new file mode 100644
> > index 0000000..9de7849
> > --- /dev/null
> > +++ b/package/python-pexpect/python-pexpect.mk
> > @@ -0,0 +1,14 @@
> > +################################################################################
> > +#
> > +# python-pexpect
> > +#
> > +################################################################################
> > +
> > +PYTHON_PEXPECT_VERSION = 4.0.1
> > +PYTHON_PEXPECT_SITE = $(call github,pexpect,pexpect,$(PYTHON_PEXPECT_VERSION))
> > +PYTHON_PEXPECT_LICENSE = ISC
> > +PYTHON_PEXPECT_LICENSE_FILES = LICENSE
> > +PYTHON_PEXPECT_SETUP_TYPE = distutils
> > +PYTHON_PEXPECT_DEPENDENCIES = python-ptyprocess  
> 
> is this a run-time or build dependency?

This is run-time so I guess to remove this line...

> 
> > +
> > +$(eval $(python-package))
> > --
> > 2.6.3
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot at busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot  



-- 
   Jan Viktorin                  E-mail: Viktorin at RehiveTech.com
   System Architect              Web:    www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 3/3] dpdk: new package
  2015-12-09 14:11   ` [Buildroot] [PATCH v2 3/3] dpdk: " Jan Viktorin
@ 2015-12-09 22:36     ` Thomas Petazzoni
  2015-12-09 23:27       ` Jan Viktorin
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Petazzoni @ 2015-12-09 22:36 UTC (permalink / raw)
  To: buildroot

Jan,

On Wed,  9 Dec 2015 15:11:53 +0100, Jan Viktorin wrote:
> This patch introduces support of the DPDK library (www.dpdk.org) into
> Buildroot. DPDK is a library for high-speed packet sending/receiving
> while bypassing the Linux Kernel. It allows to reach a high throughput
> for 10-100 Gbps networks on the x86 platform.

Great! I'm not using DPDK myself, but I believe it's good to have this
supported in Buildroot.

> diff --git a/package/dpdk/Config.in b/package/dpdk/Config.in
> new file mode 100644
> index 0000000..a4935ae
> --- /dev/null
> +++ b/package/dpdk/Config.in
> @@ -0,0 +1,47 @@
> +config BR2_PACKAGE_DPDK
> +       bool "dpdk"
> +       depends on BR2_x86_i686 || BR2_x86_64 || BR2_powerpc_power8 || BR2_ARM_CPU_ARMV7A || BR2_aarch64 || BR2_aarch64_be

Why only BR2_x86_i686 ? I'm pretty sure it can work on many other 32
bits x86 CPUs. Did you want to say "at least i686" ?

Please wrap lines at 72 characters.

> +       depends on BR2_TOOLCHAIN_USES_GLIBC

You need a dependency on BR2_LINUX_KERNEL here, since your .mk file is
building some kernel modules.

> +       help
> +         DPDK is a set of libraries and drivers for fast packet processing. It was designed to run on
> +         any processors knowing Intel x86 has been the first CPU to be supported. Ports for other CPUs
> +         like IBM Power 8 and ARM are under progress. It runs mostly in Linux userland. A FreeBSD port
> +         is now available for a subset of DPDK features.

Wrapping please :)

> +
> +	 Notes:
> +	 * To build the included Linux Kernel drivers, it is necessary to enable CONFIG_PCI_MSI,
> +	   CONFIG_UIO.
> +	 * To build the PCAP PMD properly, you need to enable the libpcap manually.
> +	 * To install the python scripts (dpdk_nic_bind.py, cpu_layout.py), enable
> +	   the python2 interpreter for the target.
> +
> +         http://www.dpdk.org/
> +
> +if BR2_PACKAGE_DPDK
> +
> +config BR2_PACKAGE_DPDK_CONFIG
> +	string "Configuration"

Why do you make this a visible option? Is there any reason for the user
to be able to chnage this value ?

> +	default "i686-native-linuxapp-gcc" \
> +		if BR2_x86_i686
> +	default "x86_64-native-linuxapp-gcc" \
> +		if BR2_x86_64
> +	default "ppc_64-power8-native-linuxapp-gcc" \

So it's a PowerPC 64 configuration? BR2_powerpc_power8 by itself
doesn't say if it's 32 bits or 64 bits, so maybe you need
BR2_powerpc_power8 && BR2_ARCH_IS_64.

> +		if BR2_powerpc_power8

> diff --git a/package/dpdk/dpdk.hash b/package/dpdk/dpdk.hash
> new file mode 100644
> index 0000000..c78ec85
> --- /dev/null
> +++ b/package/dpdk/dpdk.hash
> @@ -0,0 +1,4 @@
> +# Locally calculated
> +sha256	f7b322867a45f99afd9c8fbacdc56e1621676f9ca0f046656ec85eb6a99a3440  dpdk-2.1.0.tar.gz
> +sha256	530074d4eaefe1f717e7411e6a74e4ba0fa619af304c5e74e1097e51d33cc19e  dpdk-2.2.0-rc2.tar.gz
> +sha256	7caf52554c0f724a09e9342ee6670b324a77dade5cd0b96ff5b66957ed1bc1f9  dpdk-2.2.0-rc3.tar.gz

You no longer offer a version choice, so there's no reason to have all
those hashes. Keep just the one matching the version used in dpdk.mk.

> +DPDK_VERSION = 2.2.0-rc3
> +DPDK_SITE = http://dpdk.org/browse/dpdk/snapshot
> +DPDK_SOURCE = dpdk-$(DPDK_VERSION).tar.gz
> +
> +DPDK_LICENSE = BSD

This is not sufficient, since as you said previously, it's not only
under BSD. Also BSD is not specific enough, it should be BSD-2c or
BSD-3c.

> +DPDK_LICENSE_FILES = LICENSE.LGPL LICENSE.GPL

So those contain the GPL and LGPL texts, but DPDK_LICENSE does not
reference the GPL and LGPL licenses. And the BSD license text is not
there. I believe this licensing information needs to be improved a bit.

> +DPDK_INSTALL_STAGING = YES
> +
> +DPDK_DEPENDENCIES += linux
> +
> +ifeq ($(BR2_PACKAGE_LIBPCAP),y)
> +DPDK_DEPENDENCIES += libpcap
> +endif
> +
> +ifeq ($(BR2_SHARED_LIBS),y)
> +define DPDK_ENABLE_SHARED_LIBS
> +	@echo CONFIG_RTE_BUILD_COMBINE_LIBS=y >> $(@D)/build/.config
> +	@echo CONFIG_RTE_BUILD_SHARED_LIB=y >> $(@D)/build/.config
> +endef
> +
> +DPDK_POST_CONFIGURE_HOOKS += DPDK_ENABLE_SHARED_LIBS
> +endif

I think this piece of code does not take into account the fact that
BR2_SHARED_STATIC_LIBS=y means that both shared and static libraries
must be enabled. Remember that there is a three state choice:

 - BR2_STATIC_LIBS -> static libraries only (no shared libs)
 - BR2_SHARED_LIBS -> shared libraries only (no static libs)
 - BR2_SHARED_STATIC_LIBS -> both static and shared libraries

What does BUILD_COMBINE_LIBS does ?

Also, these >> means that if you do "make dpdk-reconfigure", those
lines will keep being added and added and added at the end of
the .config file.

> +# We create symlink named $(DPDK_CONFIG) to the build directory

a symlink

> +# to avoid calling install which behaves strange in DPDK build system.
> +define DPDK_CONFIGURE_CMDS
> +	$(MAKE) -C $(@D) T=$(DPDK_CONFIG) RTE_KERNELDIR=$(LINUX_DIR) CROSS=$(TARGET_CROSS) config
> +	@ln -sv build $(@D)/$(DPDK_CONFIG)

Don't use @ here.

> +endef
> +
> +define DPDK_BUILD_CMDS
> +	$(MAKE1) -C $(@D) T=$(DPDK_CONFIG) RTE_KERNELDIR=$(LINUX_DIR) CROSS=$(TARGET_CROSS) install

So you're doing the installation during the build step? This is not
good. But I see that later you're doing the installation manually. What
is being installed here then? The kernel modules?

Since the package is not "standard", it would probably be good to
include a few more comments in the .mk file to explain what is going on.

Also, is the MAKE1 instead of MAKE intentional?

> +endef
> +
> +ifeq ($(BR2_SHARED_LIBS),n)

A BR2_<foo> boolean variable can never be 'n'. It can be 'y' or empty.

> +# Install static libs only (DPDK compiles either *.so or *.a)
> +define DPDK_INSTALL_STAGING_LIBS
> +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
> +	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.a $(STAGING_DIR)/usr/lib

No -D in this case.

> +endef
> +
> +else
> +# Install shared libs only (DPDK compiles either *.so or *.a)
> +define DPDK_INSTALL_STAGING_LIBS
> +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
> +	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.so* $(STAGING_DIR)/usr/lib
> +endef

This logic again assumes that we install either shared *or* static
libraries. But if BR2_SHARED_STATIC_LIBS=y, we need to install both.


> +
> +define DPDK_INSTALL_TARGET_LIBS
> +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib

Should be $(TARGET_DIR).

> +	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.so* $(TARGET_DIR)/usr/lib
> +endef
> +endif
> +
> +ifeq ($(BR2_PACKAGE_PYTHON),y)
> +define DPDK_INSTALL_TARGET_PYSCRIPTS
> +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/bin

Not needed (and wrong since you're using STAGING_DIR).

> +	$(INSTALL) -m 0755 -D $(@D)/tools/dpdk_nic_bind.py $(TARGET_DIR)/usr/bin
> +	$(INSTALL) -m 0755 -D $(@D)/tools/cpu_layout.py $(TARGET_DIR)/usr/bin

Use full paths for the destination.

> +endef
> +
> +DPDK_DEPENDENCIES += python
> +endif
> +
> +ifeq ($(BR2_PACKAGE_DPDK_TOOLS_TESTPMD),y)
> +define DPDK_INSTALL_TARGET_TESTPMD
> +	$(INSTALL) -m 0755 -D -d $(TARGET_DIR)/usr/bin

Not needed.

> +	$(INSTALL) -m 0755 -D $(@D)/build/app/testpmd $(TARGET_DIR)/usr/bin

Use a complete path for the destination.

> +endef
> +endif
> +
> +ifeq ($(BR2_PACKAGE_DPDK_TOOLS_TEST),y)
> +define DPDK_INSTALL_TARGET_TEST
> +	$(INSTALL) -m 0755 -D -d $(TARGET_DIR)/usr/dpdk
> +	$(INSTALL) -m 0755 -D $(@D)/build/app/test $(TARGET_DIR)/usr/dpdk
> +	$(INSTALL) -m 0755 -D $(@D)/app/test/*.py $(TARGET_DIR)/usr/dpdk
> +endef
> +
> +ifeq ($(BR2_PACKAGE_PYTHON),y)
> +DPDK_DEPENDENCIES += python-pexpect

Not good, you cannot add a package to the dependencies if it is not
enabled in the configuration. Is there anyway any point in installing
the .py files if you don't have a Python interpreter?

If you really want to handle it this way, you can do:

	select BR2_PACKAGE_PYTHON_PEXPECT if BR2_PACKAGE_PYTHON

under the BR2_PACKAGE_DPDK_TOOLS_TEST option.

> +define DPDK_INSTALL_STAGING_CMDS
> +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/include
> +	$(INSTALL) -m 0644 -D $(@D)/build/include/*.h $(STAGING_DIR)/usr/include
> +	$(DPDK_INSTALL_STAGING_LIBS)
> +endef
> +
> +define DPDK_INSTALL_TARGET_CMDS
> +	$(DPDK_INSTALL_TARGET_LIBS)
> +	$(DPDK_INSTALL_TARGET_PYSCRIPTS)
> +	$(DPDK_INSTALL_TARGET_TESTPMD)
> +	$(DPDK_INSTALL_TARGET_TEST)
> +endef
> +
> +$(eval $(generic-package))

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 3/3] dpdk: new package
  2015-12-09 22:36     ` Thomas Petazzoni
@ 2015-12-09 23:27       ` Jan Viktorin
  2015-12-10  8:54         ` Thomas Petazzoni
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Viktorin @ 2015-12-09 23:27 UTC (permalink / raw)
  To: buildroot

On Wed, 9 Dec 2015 23:36:03 +0100
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Jan,
> 
> On Wed,  9 Dec 2015 15:11:53 +0100, Jan Viktorin wrote:
> > This patch introduces support of the DPDK library (www.dpdk.org) into
> > Buildroot. DPDK is a library for high-speed packet sending/receiving
> > while bypassing the Linux Kernel. It allows to reach a high throughput
> > for 10-100 Gbps networks on the x86 platform.  
> 
> Great! I'm not using DPDK myself, but I believe it's good to have this
> supported in Buildroot.

Hello Thomas, thanks for your review...

> 
> > diff --git a/package/dpdk/Config.in b/package/dpdk/Config.in
> > new file mode 100644
> > index 0000000..a4935ae
> > --- /dev/null
> > +++ b/package/dpdk/Config.in
> > @@ -0,0 +1,47 @@
> > +config BR2_PACKAGE_DPDK
> > +       bool "dpdk"
> > +       depends on BR2_x86_i686 || BR2_x86_64 || BR2_powerpc_power8 || BR2_ARM_CPU_ARMV7A || BR2_aarch64 || BR2_aarch64_be  
> 
> Why only BR2_x86_i686 ? I'm pretty sure it can work on many other 32
> bits x86 CPUs. Did you want to say "at least i686" ?

Well, I'd say "at least i686", however, the general reason was the list
of default configurations available in DPDK (only gcc-based
configurations are listed):

defconfig_arm64-armv8a-linuxapp-gcc
defconfig_arm64-thunderx-linuxapp-gcc
defconfig_arm64-xgene1-linuxapp-gcc
defconfig_arm-armv7a-linuxapp-gcc
defconfig_i686-native-linuxapp-gcc
defconfig_ppc_64-power8-linuxapp-gcc
defconfig_tile-tilegx-linuxapp-gcc
defconfig_x86_64-ivshmem-linuxapp-gcc
defconfig_x86_64-native-bsdapp-gcc
defconfig_x86_64-native-linuxapp-gcc
defconfig_x86_x32-native-linuxapp-gcc

> 
> Please wrap lines at 72 characters.

I know... ;) I've discovered a great helper today ":set colorcolumn=80".

> 
> > +       depends on BR2_TOOLCHAIN_USES_GLIBC  
> 
> You need a dependency on BR2_LINUX_KERNEL here, since your .mk file is
> building some kernel modules.

OK.

> 
> > +       help
> > +         DPDK is a set of libraries and drivers for fast packet processing. It was designed to run on
> > +         any processors knowing Intel x86 has been the first CPU to be supported. Ports for other CPUs
> > +         like IBM Power 8 and ARM are under progress. It runs mostly in Linux userland. A FreeBSD port
> > +         is now available for a subset of DPDK features.  
> 
> Wrapping please :)

Already fixed internally, thanks.

> 
> > +
> > +	 Notes:
> > +	 * To build the included Linux Kernel drivers, it is necessary to enable CONFIG_PCI_MSI,
> > +	   CONFIG_UIO.
> > +	 * To build the PCAP PMD properly, you need to enable the libpcap manually.
> > +	 * To install the python scripts (dpdk_nic_bind.py, cpu_layout.py), enable
> > +	   the python2 interpreter for the target.
> > +
> > +         http://www.dpdk.org/
> > +
> > +if BR2_PACKAGE_DPDK
> > +
> > +config BR2_PACKAGE_DPDK_CONFIG
> > +	string "Configuration"  
> 
> Why do you make this a visible option? Is there any reason for the user
> to be able to chnage this value ?

DPDK provides a list of default configurations. I can imagine somebody
would like to use a custom one for this (added probably by a custom
patch to Buildroot). Also, DPDK will evolve overtime and this enables to
specify a configuration we didn't have in the list. Moreover, as you can
see from the list of default configurations above, for ARMv8, there are
3 possible configurations...

> 
> > +	default "i686-native-linuxapp-gcc" \
> > +		if BR2_x86_i686
> > +	default "x86_64-native-linuxapp-gcc" \
> > +		if BR2_x86_64
> > +	default "ppc_64-power8-native-linuxapp-gcc" \  
> 
> So it's a PowerPC 64 configuration? BR2_powerpc_power8 by itself
> doesn't say if it's 32 bits or 64 bits, so maybe you need
> BR2_powerpc_power8 && BR2_ARCH_IS_64.

Yes, 64 bits. Anyway, I cannot test this, so it is just a blind shoot.

> 
> > +		if BR2_powerpc_power8  
> 
> > diff --git a/package/dpdk/dpdk.hash b/package/dpdk/dpdk.hash
> > new file mode 100644
> > index 0000000..c78ec85
> > --- /dev/null
> > +++ b/package/dpdk/dpdk.hash
> > @@ -0,0 +1,4 @@
> > +# Locally calculated
> > +sha256	f7b322867a45f99afd9c8fbacdc56e1621676f9ca0f046656ec85eb6a99a3440  dpdk-2.1.0.tar.gz
> > +sha256	530074d4eaefe1f717e7411e6a74e4ba0fa619af304c5e74e1097e51d33cc19e  dpdk-2.2.0-rc2.tar.gz
> > +sha256	7caf52554c0f724a09e9342ee6670b324a77dade5cd0b96ff5b66957ed1bc1f9  dpdk-2.2.0-rc3.tar.gz  
> 
> You no longer offer a version choice, so there's no reason to have all
> those hashes. Keep just the one matching the version used in dpdk.mk.

OK. So you don't expect somebody sticks on a certain version of a
package? This happens... It does not make sense for -rc versions, of
course.

> 
> > +DPDK_VERSION = 2.2.0-rc3
> > +DPDK_SITE = http://dpdk.org/browse/dpdk/snapshot
> > +DPDK_SOURCE = dpdk-$(DPDK_VERSION).tar.gz
> > +
> > +DPDK_LICENSE = BSD  
> 
> This is not sufficient, since as you said previously, it's not only
> under BSD. Also BSD is not specific enough, it should be BSD-2c or
> BSD-3c.

BSD-3c

Can the DPDK_LICENSE contain more licenses? (RTFM..., I am lazy, maybe tomorrow :).) 

> 
> > +DPDK_LICENSE_FILES = LICENSE.LGPL LICENSE.GPL  
> 
> So those contain the GPL and LGPL texts, but DPDK_LICENSE does not
> reference the GPL and LGPL licenses. And the BSD license text is not
> there. I believe this licensing information needs to be improved a bit.

How? The BSD license text is not in the DPDK upstream, no idea why.

> 
> > +DPDK_INSTALL_STAGING = YES
> > +
> > +DPDK_DEPENDENCIES += linux
> > +
> > +ifeq ($(BR2_PACKAGE_LIBPCAP),y)
> > +DPDK_DEPENDENCIES += libpcap
> > +endif
> > +
> > +ifeq ($(BR2_SHARED_LIBS),y)
> > +define DPDK_ENABLE_SHARED_LIBS
> > +	@echo CONFIG_RTE_BUILD_COMBINE_LIBS=y >> $(@D)/build/.config
> > +	@echo CONFIG_RTE_BUILD_SHARED_LIB=y >> $(@D)/build/.config
> > +endef
> > +
> > +DPDK_POST_CONFIGURE_HOOKS += DPDK_ENABLE_SHARED_LIBS
> > +endif  
> 
> I think this piece of code does not take into account the fact that
> BR2_SHARED_STATIC_LIBS=y means that both shared and static libraries
> must be enabled. Remember that there is a three state choice:
> 
>  - BR2_STATIC_LIBS -> static libraries only (no shared libs)
>  - BR2_SHARED_LIBS -> shared libraries only (no static libs)
>  - BR2_SHARED_STATIC_LIBS -> both static and shared libraries

Does BR2_SHARED_STATIC_LIBSBR2_SHARED_STATIC_LIBS mean, that I MUST
provide both shared and static version? In that case, you are right and
I will have hard time of figure out, how to persuade DPDK to give me
those... But, as I've mentioned in the cover letter, the install system
has changed recently in DPDK, so it's possible I can do this better...

> 
> What does BUILD_COMBINE_LIBS does ?

Good question... My answer is that it magically builds and works
with this option :). I've done this quite a long time ago, I have to
check it again, my bad.

> 
> Also, these >> means that if you do "make dpdk-reconfigure", those
> lines will keep being added and added and added at the end of
> the .config file.

Wow :). Is there a better way to do this? BTW, the fact there is
a .config file does not mean that it is the same system as in the
kconfig. It is not...

> 
> > +# We create symlink named $(DPDK_CONFIG) to the build directory  
> 
> a symlink

OK.

> 
> > +# to avoid calling install which behaves strange in DPDK build system.
> > +define DPDK_CONFIGURE_CMDS
> > +	$(MAKE) -C $(@D) T=$(DPDK_CONFIG) RTE_KERNELDIR=$(LINUX_DIR) CROSS=$(TARGET_CROSS) config
> > +	@ln -sv build $(@D)/$(DPDK_CONFIG)  
> 
> Don't use @ here.

OK.

> 
> > +endef
> > +
> > +define DPDK_BUILD_CMDS
> > +	$(MAKE1) -C $(@D) T=$(DPDK_CONFIG) RTE_KERNELDIR=$(LINUX_DIR) CROSS=$(TARGET_CROSS) install  
> 
> So you're doing the installation during the build step? This is not
> good. But I see that later you're doing the installation manually. What
> is being installed here then? The kernel modules?

Nothing is installed here. Actually, it installs things for itself (not
into standard paths or anything...). This is the only magic sequence
that builds the DPDK in the way I need for Buildroot. Think of it in a
way like s/install/build/. Yes, it is strange. I hope that it is no
more an issue with the recent changes in DPDK.

> 
> Since the package is not "standard", it would probably be good to
> include a few more comments in the .mk file to explain what is going on.

True.

> 
> Also, is the MAKE1 instead of MAKE intentional?

Yes, that is an intention. I have issues with parallel builds.

> 
> > +endef
> > +
> > +ifeq ($(BR2_SHARED_LIBS),n)  
> 
> A BR2_<foo> boolean variable can never be 'n'. It can be 'y' or empty.

OK. This is better (however, less readable):

ifneq ($(BR2_SHARED_LIBS),y)

isn't it?

> 
> > +# Install static libs only (DPDK compiles either *.so or *.a)
> > +define DPDK_INSTALL_STAGING_LIBS
> > +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
> > +	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.a $(STAGING_DIR)/usr/lib  
> 
> No -D in this case.

Do you mean only for the second one?

> 
> > +endef
> > +
> > +else
> > +# Install shared libs only (DPDK compiles either *.so or *.a)
> > +define DPDK_INSTALL_STAGING_LIBS
> > +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
> > +	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.so* $(STAGING_DIR)/usr/lib
> > +endef  
> 
> This logic again assumes that we install either shared *or* static
> libraries. But if BR2_SHARED_STATIC_LIBS=y, we need to install both.

So my assumption (MUST) on the top of this e-mail is right. But, DPDK
provides either *.so or *.a. Never both. Or, I don't know, how to
configure it for this. In fact, the shared builds are sometimes claimed
(I've seen that written somewhere but it was not in the docs) to be
broken (they work for my cases) and DPDK applications are usually build
statically for x86.

> 
> 
> > +
> > +define DPDK_INSTALL_TARGET_LIBS
> > +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib  
> 
> Should be $(TARGET_DIR).

OK.

> 
> > +	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.so* $(TARGET_DIR)/usr/lib
> > +endef
> > +endif
> > +
> > +ifeq ($(BR2_PACKAGE_PYTHON),y)
> > +define DPDK_INSTALL_TARGET_PYSCRIPTS
> > +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/bin  
> 
> Not needed (and wrong since you're using STAGING_DIR).

This looks like a copy & paste mistake. Same as above, it should be
TARGET_DIR.

> 
> > +	$(INSTALL) -m 0755 -D $(@D)/tools/dpdk_nic_bind.py $(TARGET_DIR)/usr/bin
> > +	$(INSTALL) -m 0755 -D $(@D)/tools/cpu_layout.py $(TARGET_DIR)/usr/bin  
> 
> Use full paths for the destination.

Do you mean: $(TARGET_DIR)/usr/bin/dpdk_nic_bind.py?

> 
> > +endef
> > +
> > +DPDK_DEPENDENCIES += python
> > +endif
> > +
> > +ifeq ($(BR2_PACKAGE_DPDK_TOOLS_TESTPMD),y)
> > +define DPDK_INSTALL_TARGET_TESTPMD
> > +	$(INSTALL) -m 0755 -D -d $(TARGET_DIR)/usr/bin  
> 
> Not needed.

OK.

> 
> > +	$(INSTALL) -m 0755 -D $(@D)/build/app/testpmd $(TARGET_DIR)/usr/bin  
> 
> Use a complete path for the destination.

Same as above.

> 
> > +endef
> > +endif
> > +
> > +ifeq ($(BR2_PACKAGE_DPDK_TOOLS_TEST),y)
> > +define DPDK_INSTALL_TARGET_TEST
> > +	$(INSTALL) -m 0755 -D -d $(TARGET_DIR)/usr/dpdk
> > +	$(INSTALL) -m 0755 -D $(@D)/build/app/test $(TARGET_DIR)/usr/dpdk
> > +	$(INSTALL) -m 0755 -D $(@D)/app/test/*.py $(TARGET_DIR)/usr/dpdk
> > +endef
> > +
> > +ifeq ($(BR2_PACKAGE_PYTHON),y)
> > +DPDK_DEPENDENCIES += python-pexpect  
> 
> Not good, you cannot add a package to the dependencies if it is not
> enabled in the configuration. Is there anyway any point in installing
> the .py files if you don't have a Python interpreter?

No. I think the *.py files can be ommited when there is no Python
interpreter.

> 
> If you really want to handle it this way, you can do:
> 
> 	select BR2_PACKAGE_PYTHON_PEXPECT if BR2_PACKAGE_PYTHON
> 
> under the BR2_PACKAGE_DPDK_TOOLS_TEST option.

So, I'd install the binaries only.

And if there is a python interpreter I add the *.py files. I also
enforce the PEXPECT package in Config.in in that case. This sounds
promising.

> 
> > +define DPDK_INSTALL_STAGING_CMDS
> > +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/include
> > +	$(INSTALL) -m 0644 -D $(@D)/build/include/*.h $(STAGING_DIR)/usr/include
> > +	$(DPDK_INSTALL_STAGING_LIBS)
> > +endef
> > +
> > +define DPDK_INSTALL_TARGET_CMDS
> > +	$(DPDK_INSTALL_TARGET_LIBS)
> > +	$(DPDK_INSTALL_TARGET_PYSCRIPTS)
> > +	$(DPDK_INSTALL_TARGET_TESTPMD)
> > +	$(DPDK_INSTALL_TARGET_TEST)
> > +endef
> > +
> > +$(eval $(generic-package))  
> 
> Thanks!
> 
> Thomas

Regards
Jan

-- 
  Jan Viktorin                E-mail: Viktorin at RehiveTech.com
  System Architect            Web:    www.RehiveTech.com
  RehiveTech
  Brno, Czech Republic

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 3/3] dpdk: new package
  2015-12-09 23:27       ` Jan Viktorin
@ 2015-12-10  8:54         ` Thomas Petazzoni
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Petazzoni @ 2015-12-10  8:54 UTC (permalink / raw)
  To: buildroot

Hello Jan,

On Thu, 10 Dec 2015 00:27:28 +0100, Jan Viktorin wrote:

> > > diff --git a/package/dpdk/Config.in b/package/dpdk/Config.in
> > > new file mode 100644
> > > index 0000000..a4935ae
> > > --- /dev/null
> > > +++ b/package/dpdk/Config.in
> > > @@ -0,0 +1,47 @@
> > > +config BR2_PACKAGE_DPDK
> > > +       bool "dpdk"
> > > +       depends on BR2_x86_i686 || BR2_x86_64 || BR2_powerpc_power8 || BR2_ARM_CPU_ARMV7A || BR2_aarch64 || BR2_aarch64_be  
> > 
> > Why only BR2_x86_i686 ? I'm pretty sure it can work on many other 32
> > bits x86 CPUs. Did you want to say "at least i686" ?
> 
> Well, I'd say "at least i686", however, the general reason was the list
> of default configurations available in DPDK (only gcc-based
> configurations are listed):
> 
> defconfig_arm64-armv8a-linuxapp-gcc
> defconfig_arm64-thunderx-linuxapp-gcc
> defconfig_arm64-xgene1-linuxapp-gcc
> defconfig_arm-armv7a-linuxapp-gcc
> defconfig_i686-native-linuxapp-gcc
> defconfig_ppc_64-power8-linuxapp-gcc
> defconfig_tile-tilegx-linuxapp-gcc
> defconfig_x86_64-ivshmem-linuxapp-gcc
> defconfig_x86_64-native-bsdapp-gcc
> defconfig_x86_64-native-linuxapp-gcc
> defconfig_x86_x32-native-linuxapp-gcc

For x86, I think you should just exclude the CPUs < i686, like:

	depends on (BR2_i386 && !BR2_x86_i386 && !BR2_x86_i486 && !BR2_x86_i586 && !BR2_x86_x1000) || ...

> > > +if BR2_PACKAGE_DPDK
> > > +
> > > +config BR2_PACKAGE_DPDK_CONFIG
> > > +	string "Configuration"  
> > 
> > Why do you make this a visible option? Is there any reason for the user
> > to be able to chnage this value ?
> 
> DPDK provides a list of default configurations. I can imagine somebody
> would like to use a custom one for this (added probably by a custom
> patch to Buildroot). Also, DPDK will evolve overtime and this enables to
> specify a configuration we didn't have in the list. Moreover, as you can
> see from the list of default configurations above, for ARMv8, there are
> 3 possible configurations...

Ok, makes sense.

> > > +	default "i686-native-linuxapp-gcc" \
> > > +		if BR2_x86_i686
> > > +	default "x86_64-native-linuxapp-gcc" \
> > > +		if BR2_x86_64
> > > +	default "ppc_64-power8-native-linuxapp-gcc" \  
> > 
> > So it's a PowerPC 64 configuration? BR2_powerpc_power8 by itself
> > doesn't say if it's 32 bits or 64 bits, so maybe you need
> > BR2_powerpc_power8 && BR2_ARCH_IS_64.
> 
> Yes, 64 bits. Anyway, I cannot test this, so it is just a blind shoot.

You can decide to not support it.

> > > +		if BR2_powerpc_power8  
> > 
> > > diff --git a/package/dpdk/dpdk.hash b/package/dpdk/dpdk.hash
> > > new file mode 100644
> > > index 0000000..c78ec85
> > > --- /dev/null
> > > +++ b/package/dpdk/dpdk.hash
> > > @@ -0,0 +1,4 @@
> > > +# Locally calculated
> > > +sha256	f7b322867a45f99afd9c8fbacdc56e1621676f9ca0f046656ec85eb6a99a3440  dpdk-2.1.0.tar.gz
> > > +sha256	530074d4eaefe1f717e7411e6a74e4ba0fa619af304c5e74e1097e51d33cc19e  dpdk-2.2.0-rc2.tar.gz
> > > +sha256	7caf52554c0f724a09e9342ee6670b324a77dade5cd0b96ff5b66957ed1bc1f9  dpdk-2.2.0-rc3.tar.gz  
> > 
> > You no longer offer a version choice, so there's no reason to have all
> > those hashes. Keep just the one matching the version used in dpdk.mk.
> 
> OK. So you don't expect somebody sticks on a certain version of a
> package? This happens... It does not make sense for -rc versions, of
> course.

I don't understand what you mean here. dpdk.mk references 2.2.0-rc3
only, so only the hash for 2.2.0-rc3 should be in the .hash file. There
is no reason to have hashes for other versions.

> > > +DPDK_VERSION = 2.2.0-rc3
> > > +DPDK_SITE = http://dpdk.org/browse/dpdk/snapshot
> > > +DPDK_SOURCE = dpdk-$(DPDK_VERSION).tar.gz
> > > +
> > > +DPDK_LICENSE = BSD  
> > 
> > This is not sufficient, since as you said previously, it's not only
> > under BSD. Also BSD is not specific enough, it should be BSD-2c or
> > BSD-3c.
> 
> BSD-3c
> 
> Can the DPDK_LICENSE contain more licenses? (RTFM..., I am lazy, maybe tomorrow :).) 

Yes, it can contain more licenses. Here is an example from Buildroot:

	ATTR_LICENSE = GPLv2+ (programs), LGPLv2.1+ (libraries)

> > > +DPDK_LICENSE_FILES = LICENSE.LGPL LICENSE.GPL  
> > 
> > So those contain the GPL and LGPL texts, but DPDK_LICENSE does not
> > reference the GPL and LGPL licenses. And the BSD license text is not
> > there. I believe this licensing information needs to be improved a bit.
> 
> How? The BSD license text is not in the DPDK upstream, no idea why.

You can put a source file which carries a BSD license header in
DPDK_LICENSE_FILES. Usually, we try to choose a fairly small source
file.

See for example:

	E2FSPROGS_LICENSE_FILES = COPYING lib/uuid/COPYING lib/ss/mit-sipb-copyright.h lib/et/internal.h

> > I think this piece of code does not take into account the fact that
> > BR2_SHARED_STATIC_LIBS=y means that both shared and static libraries
> > must be enabled. Remember that there is a three state choice:
> > 
> >  - BR2_STATIC_LIBS -> static libraries only (no shared libs)
> >  - BR2_SHARED_LIBS -> shared libraries only (no static libs)
> >  - BR2_SHARED_STATIC_LIBS -> both static and shared libraries
> 
> Does BR2_SHARED_STATIC_LIBSBR2_SHARED_STATIC_LIBS mean, that I MUST
> provide both shared and static version?

Normally yes. However in practice, not all packages fully comply with
this. BR2_SHARED_LIBS sometimes lead to packages building/installing
both the shared and static variants. What is however non-negotiable is
that BR2_STATIC_LIBS should not build any shared library.

> In that case, you are right and
> I will have hard time of figure out, how to persuade DPDK to give me
> those... But, as I've mentioned in the cover letter, the install system
> has changed recently in DPDK, so it's possible I can do this better...

I think you can do BR2_STATIC_LIBS builds/installs static libs only,
and BR2_STATIC_SHARED_LIBS or BR2_SHARED_LIBS builds/installs shared
libs only. It's probably good enough for now.

> > Also, these >> means that if you do "make dpdk-reconfigure", those
> > lines will keep being added and added and added at the end of
> > the .config file.
> 
> Wow :). Is there a better way to do this? BTW, the fact there is
> a .config file does not mean that it is the same system as in the
> kconfig. It is not...

Even if it is not kconfig based, you can still use the sort of logic as
the KCONFIG_ENABLE_OPT, KCONFIG_SET_OPT and KCONFIG_DISABLE_OPT (see
pkg-utils.mk).

> > > +endef
> > > +
> > > +define DPDK_BUILD_CMDS
> > > +	$(MAKE1) -C $(@D) T=$(DPDK_CONFIG) RTE_KERNELDIR=$(LINUX_DIR) CROSS=$(TARGET_CROSS) install  
> > 
> > So you're doing the installation during the build step? This is not
> > good. But I see that later you're doing the installation manually. What
> > is being installed here then? The kernel modules?
> 
> Nothing is installed here. Actually, it installs things for itself (not
> into standard paths or anything...). This is the only magic sequence
> that builds the DPDK in the way I need for Buildroot. Think of it in a
> way like s/install/build/. Yes, it is strange. I hope that it is no
> more an issue with the recent changes in DPDK.

Just add a comment above to explain this. Thanks :)

> > Also, is the MAKE1 instead of MAKE intentional?
> 
> Yes, that is an intention. I have issues with parallel builds.

Ok.

> > > +ifeq ($(BR2_SHARED_LIBS),n)  
> > 
> > A BR2_<foo> boolean variable can never be 'n'. It can be 'y' or empty.
> 
> OK. This is better (however, less readable):
> 
> ifneq ($(BR2_SHARED_LIBS),y)
> 
> isn't it?

No:

ifeq ($(BR2_SHARED_LIBS),)

or alternatively:

ifeq ($(BR2_STATIC_LIBS),y)

depending on the behavior you want for BR2_SHARED_STATIC_LIBS.

> 
> > 
> > > +# Install static libs only (DPDK compiles either *.so or *.a)
> > > +define DPDK_INSTALL_STAGING_LIBS
> > > +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
> > > +	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.a $(STAGING_DIR)/usr/lib  
> > 
> > No -D in this case.
> 
> Do you mean only for the second one?

For both.

> > > +endef
> > > +
> > > +else
> > > +# Install shared libs only (DPDK compiles either *.so or *.a)
> > > +define DPDK_INSTALL_STAGING_LIBS
> > > +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib
> > > +	$(INSTALL) -m 0644 -D $(@D)/build/lib/*.so* $(STAGING_DIR)/usr/lib
> > > +endef  
> > 
> > This logic again assumes that we install either shared *or* static
> > libraries. But if BR2_SHARED_STATIC_LIBS=y, we need to install both.
> 
> So my assumption (MUST) on the top of this e-mail is right. But, DPDK
> provides either *.so or *.a. Never both. Or, I don't know, how to
> configure it for this. In fact, the shared builds are sometimes claimed
> (I've seen that written somewhere but it was not in the docs) to be
> broken (they work for my cases) and DPDK applications are usually build
> statically for x86.

I think it's OK if BR2_SHARED_STATIC_LIBS is not implemented perfectly
for dpdk. Just assume BR2_SHARED_STATIC_LIBS is like BR2_SHARED_LIBS.

> > > +define DPDK_INSTALL_TARGET_LIBS
> > > +	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib  
> > 
> > Should be $(TARGET_DIR).
> 
> OK.

And no -D here.

> > > +	$(INSTALL) -m 0755 -D $(@D)/tools/dpdk_nic_bind.py $(TARGET_DIR)/usr/bin
> > > +	$(INSTALL) -m 0755 -D $(@D)/tools/cpu_layout.py $(TARGET_DIR)/usr/bin  
> > 
> > Use full paths for the destination.
> 
> Do you mean: $(TARGET_DIR)/usr/bin/dpdk_nic_bind.py?

Yes.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-12-10  8:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-30 11:05 [Buildroot] [RFC PATCH] dpdk: new package Jan Viktorin
2015-10-30 17:17 ` Arnout Vandecappelle
2015-11-02 10:34   ` Jan Viktorin
2015-12-09 14:11 ` [Buildroot] [PATCH v2 0/3] " Jan Viktorin
2015-12-09 14:11   ` [Buildroot] [PATCH v2 1/3] python-ptyprocess: " Jan Viktorin
2015-12-09 14:16     ` Yegor Yefremov
2015-12-09 14:17     ` Vicente Olivert Riera
2015-12-09 14:11   ` [Buildroot] [PATCH v2 2/3] python-pexpect: " Jan Viktorin
2015-12-09 14:18     ` Yegor Yefremov
2015-12-09 14:21       ` Vicente Olivert Riera
2015-12-09 14:34       ` Jan Viktorin
2015-12-09 14:19     ` Vicente Olivert Riera
2015-12-09 14:11   ` [Buildroot] [PATCH v2 3/3] dpdk: " Jan Viktorin
2015-12-09 22:36     ` Thomas Petazzoni
2015-12-09 23:27       ` Jan Viktorin
2015-12-10  8:54         ` Thomas Petazzoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.