dpdk: new package

RFC PATCH New Jan Viktorin 5 participants 16 messages Oct 30, 2015 – Dec 10, 2015

Share & tools

Jan Viktorin

author

2015-10-30 11:05 UTC #0 lore raw

RFC PATCH 1/1 dpdk: new package

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 <[email protected]>

---
 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

package/Config.in +1 -0

index 10ff94e..7ff7cc2 100644
@@ -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"

package/dpdk/Config.in new file +159 -0

159 lines not on this page · raw message

package/dpdk/dpdk.hash new file +6 -0

new file mode 100644
index 0000000..a5c4dbd
@@ -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

package/dpdk/dpdk.mk new file +119 -0

119 lines not on this page · raw message

Signature · 1 line

2.6.1

Arnout Vandecappelle

2015-10-30 17:17 UTC #1 lore raw

RFC PATCH 1/1 dpdk: new package

Hi Jan,

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

Quoted 5 lines · Jan Viktorin

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 :-)

Quoted 10 lines

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.

Quoted 9 lines

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.

Quoted 8 lines

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.

Quoted 4 lines

* 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.

Quoted 8 lines

* 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 <[email protected]>

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]

Signature · 6 lines

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

Jan Viktorin

author

2015-11-02 10:34 UTC #2 lore raw

RFC PATCH 1/1 dpdk: new package

Hello Arnout,

thanks for your quick review...

Quoted 4 lines · On Fri, 30 Oct 2015 18:17:09 +0100 Arnout Vandecappelle <[email protected]> wrote:

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

Quoted 8 lines

Quoted 5 lines · Jan Viktorin
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 ;).

Quoted 8 lines

Quoted 8 lines
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

Quoted 8 lines

Quoted 3 lines
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.

Quoted 2 lines

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.

Quoted 2 lines

Same for custom patches, we have BR2_GLOBAL_PATCH_DIR.

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

Quoted 15 lines

Quoted 15 lines
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:

Quoted 8 lines

Quoted 3 lines
* 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.

Quoted 2 lines

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))

Quoted 9 lines

Quoted 5 lines
* 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.

Quoted 7 lines

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.

Quoted 8 lines

Quoted 8 lines
* 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?

Quoted 6 lines

Quoted 1 line
Signed-off-by Jan Viktorin <[email protected]>
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.

Quoted 1 line

- 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.

Quoted 6 lines

- 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...).

Quoted 2 lines

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

OK, will fix.

Quoted 1 line

- Don't put empty assignments (DPDK_INSTALL_TARGET_LIBS)

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

Quoted 7 lines

Regards,
Arnout
[snip]

Signature · 4 lines

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

Jan Viktorin

author

2015-12-09 14:11 UTC #3 lore raw

v2 cover dpdk: new package

In reply to Jan Viktorin

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

Signature · 1 line

2.6.3

Jan Viktorin

author

2015-12-09 14:11 UTC #4 lore raw

PATCH v2 1/3 python-ptyprocess: new package

Signed-off-by Jan Viktorin <[email protected]>

---
 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

package/Config.in +1 -0

index 2bdad01..69bc347 100644
@@ -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"

package/python-ptyprocess/Config.in new file +7 -0

new file mode 100644
index 0000000..abf8045
@@ -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

package/python-ptyprocess/python-ptyprocess.mk new file +13 -0

new file mode 100644
index 0000000..9e6b762
@@ -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))

Signature · 1 line

2.6.3

Yegor Yefremov

2015-12-09 14:16 UTC #5 lore raw

PATCH v2 1/3 python-ptyprocess: new package

Hi Jan,

Quoted 27 lines · Jan Viktorin

Signed-off-by Jan Viktorin <[email protected]>

---
 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

package/Config.in +1 -0

index 2bdad01..69bc347 100644
@@ -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"

package/python-ptyprocess/Config.in new file +0 -0

new file mode 100644
index 0000000..abf8045

package/python-ptyprocess/Config.in line 3

Quoted 3 lines

package/python-ptyprocess/Config.in new file +3 -0

@@ -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.

package/python-ptyprocess/Config.in line 5

Quoted 2 lines

package/python-ptyprocess/Config.in new file +2 -0

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

Line too long.

Quoted 28 lines

package/python-ptyprocess/Config.in new file +2 -0

+
+         https://github.com/pexpect/ptyprocess

package/python-ptyprocess/python-ptyprocess.mk new file +13 -1

new file mode 100644
index 0000000..9e6b762
@@ -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

Vicente Olivert Riera

2015-12-09 14:17 UTC #6 lore raw

PATCH v2 1/3 python-ptyprocess: new package

In reply to Jan Viktorin

Dear Jan Viktorin,

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

[snip]

package/python-ptyprocess/Config.in line 5

Quoted 11 lines

package/python-ptyprocess/Config.in new file +5 -0

new file mode 100644
index 0000000..abf8045
@@ -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.

Jan Viktorin

author

2015-12-09 14:11 UTC #7 lore raw

PATCH v2 2/3 python-pexpect: new package

In reply to Jan Viktorin

Signed-off-by Jan Viktorin <[email protected]>

---
 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

package/Config.in +1 -0

index 69bc347..77a92d2 100644
@@ -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"

package/python-pexpect/Config.in new file +10 -0

new file mode 100644
index 0000000..a42aec7
@@ -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

package/python-pexpect/python-pexpect.mk new file +14 -0

new file mode 100644
index 0000000..9de7849
@@ -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))

Signature · 1 line

2.6.3

Yegor Yefremov

2015-12-09 14:18 UTC #8 lore raw

PATCH v2 2/3 python-pexpect: new package

Quoted 27 lines · Jan Viktorin

Signed-off-by Jan Viktorin <[email protected]>

---
 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

package/Config.in +1 -0

index 69bc347..77a92d2 100644
@@ -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"

package/python-pexpect/Config.in new file +0 -0

new file mode 100644
index 0000000..a42aec7

package/python-pexpect/Config.in line 3

Quoted 3 lines

package/python-pexpect/Config.in new file +3 -0

@@ -0,0 +1,10 @@
+config BR2_PACKAGE_PYTHON_PEXPECT
+       bool "python-pexpect"
+       depends on BR2_PACKAGE_PYTHON

remove BR2_PACKAGE_PYTHON dependency

package/python-pexpect/Config.in line 8

Quoted 5 lines

package/python-pexpect/Config.in new file +5 -0

+       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

Quoted 10 lines

package/python-pexpect/Config.in new file +2 -0
+
+         https://pexpect.readthedocs.org
package/python-pexpect/python-pexpect.mk new file +2 -0
new file mode 100644
index 0000000..9de7849
@@ -0,0 +1,14 @@
+################################################################################
+#

package/python-pexpect/python-pexpect.mk line 12

Quoted 10 lines

package/python-pexpect/python-pexpect.mk new file +10 -0

+# 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?

Quoted 9 lines

package/python-pexpect/python-pexpect.mk new file +2 -1
+
+$(eval $(python-package))
--
2.6.3
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Vicente Olivert Riera

2015-12-09 14:21 UTC #9 lore raw

PATCH v2 2/3 python-pexpect: new package

Hello Yegor and Jan,

Quoted 12 lines · Yegor Yefremov

package/python-pexpect/Config.in line 3
Quoted 9 lines
package/python-pexpect/Config.in new file +3 -0
new file mode 100644
index 0000000..a42aec7
@@ -0,0 +1,10 @@
+config BR2_PACKAGE_PYTHON_PEXPECT
+       bool "python-pexpect"
+       depends on BR2_PACKAGE_PYTHON
remove BR2_PACKAGE_PYTHON dependency

Quoted 7 lines

package/python-pexpect/Config.in line 8

Quoted 5 lines

package/python-pexpect/Config.in new file +5 -0

+       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.

Jan Viktorin

author

2015-12-09 14:34 UTC #10 lore raw

PATCH v2 2/3 python-pexpect: new package

In reply to Yegor Yefremov

Quoted 25 lines · On Wed, 9 Dec 2015 15:18:40 +0100 Yegor Yefremov <[email protected]>…

Quoted 24 lines · Jan Viktorin

Signed-off-by Jan Viktorin <[email protected]>

---
 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

package/Config.in +1 -0

index 69bc347..77a92d2 100644
@@ -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"

package/python-pexpect/Config.in new file +0 -0

new file mode 100644
index 0000000..a42aec7

Quoted 8 lines

package/python-pexpect/Config.in line 3
Quoted 6 lines
package/python-pexpect/Config.in new file +3 -0
@@ -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.

Quoted 8 lines

package/python-pexpect/Config.in line 8

Quoted 6 lines

package/python-pexpect/Config.in new file +5 -0

+       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 ;).

Quoted 15 lines

package/python-pexpect/Config.in new file +2 -0

+
+         https://pexpect.readthedocs.org

package/python-pexpect/python-pexpect.mk new file +6 -0

new file mode 100644
index 0000000..9de7849
@@ -0,0 +1,14 @@
+################################################################################
+#
+# python-pexpect
+#
+################################################################################
+

Quoted 8 lines

package/python-pexpect/python-pexpect.mk line 12
Quoted 6 lines
package/python-pexpect/python-pexpect.mk new file +6 -0
+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...

Quoted 10 lines

Quoted 10 lines
package/python-pexpect/python-pexpect.mk new file +2 -1
+
+$(eval $(python-package))
--
2.6.3
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 4 lines

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

Vicente Olivert Riera

2015-12-09 14:19 UTC #11 lore raw

PATCH v2 2/3 python-pexpect: new package

In reply to Jan Viktorin

Dear Jan Viktorin,

Quoted 14 lines · Jan Viktorin

Signed-off-by Jan Viktorin <[email protected]>

---
 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

package/Config.in +0 -0

index 69bc347..77a92d2 100644

package/Config.in menu "External python modules" · line 641

Quoted 4 lines

package/Config.in +1 -0

@@ -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.

Quoted 9 lines

package/Config.in +0 -0

 	source "package/python-pam/Config.in"
 	source "package/python-posix-ipc/Config.in"
 	source "package/python-protobuf/Config.in"

package/python-pexpect/Config.in new file +0 -0

new file mode 100644
index 0000000..a42aec7

package/python-pexpect/Config.in line 8

Quoted 8 lines

package/python-pexpect/Config.in new file +8 -0

@@ -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.

Jan Viktorin

author

2015-12-09 14:11 UTC #12 lore raw

PATCH v2 3/3 dpdk: new package

In reply to Jan Viktorin

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 <[email protected]>

---
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

package/Config.in +1 -0

index 77a92d2..fd07d5d 100644
@@ -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"

package/dpdk/Config.in new file +47 -0

47 lines not on this page · raw message

package/dpdk/dpdk.hash new file +4 -0

new file mode 100644
index 0000000..c78ec85
@@ -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

package/dpdk/dpdk.mk new file +112 -0

112 lines not on this page · raw message

Signature · 1 line

2.6.3

Thomas Petazzoni

2015-12-09 22:36 UTC #13 lore raw

PATCH v2 3/3 dpdk: new package

Jan,

Quoted 4 lines · Jan Viktorin

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.

package/dpdk/Config.in line 3

Quoted 9 lines

package/dpdk/Config.in new file +3 -0

new file mode 100644
index 0000000..a4935ae
@@ -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.

package/dpdk/Config.in line 4

Quoted 1 line

package/dpdk/Config.in new file +1 -0
+       depends on BR2_TOOLCHAIN_USES_GLIBC

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

package/dpdk/Config.in line 9

Quoted 5 lines

package/dpdk/Config.in new file +5 -0

+       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 :)

Quoted 4 lines

package/dpdk/Config.in new file +4 -0

+
+	 Notes:
+	 * To build the included Linux Kernel drivers, it is necessary to enable CONFIG_PCI_MSI,
+	   CONFIG_UIO.

package/dpdk/Config.in line 23

Quoted 10 lines

package/dpdk/Config.in new file +10 -0

+	 * 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 ?

package/dpdk/Config.in line 28

Quoted 5 lines

package/dpdk/Config.in new file +5 -0

+	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.

package/dpdk/dpdk.hash line 4

Quoted 12 lines

package/dpdk/Config.in new file +1 -0

+		if BR2_powerpc_power8

package/dpdk/dpdk.hash new file +4 -0

new file mode 100644
index 0000000..c78ec85
@@ -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.

package/dpdk/dpdk.hash

Quoted 5 lines

package/dpdk/dpdk.hash new file +5 -0

+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.

Quoted 1 line

+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.

Quoted 6 lines

package/dpdk/dpdk.hash new file +6 -0

+DPDK_INSTALL_STAGING = YES
+
+DPDK_DEPENDENCIES += linux
+
+ifeq ($(BR2_PACKAGE_LIBPCAP),y)
+DPDK_DEPENDENCIES += libpcap

package/dpdk/dpdk.hash

Quoted 10 lines

package/dpdk/dpdk.hash new file +10 -0

+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.

Quoted 1 line

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

a symlink

package/dpdk/dpdk.hash

Quoted 4 lines

package/dpdk/dpdk.hash new file +4 -0

+# 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.

package/dpdk/dpdk.hash

Quoted 4 lines

package/dpdk/dpdk.hash new file +4 -0

+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?

package/dpdk/dpdk.hash

Quoted 3 lines

package/dpdk/dpdk.hash new file +3 -0
+endef
+
+ifeq ($(BR2_SHARED_LIBS),n)

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

package/dpdk/dpdk.hash

Quoted 4 lines

package/dpdk/dpdk.hash new file +4 -0

+# 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.

package/dpdk/dpdk.hash

Quoted 8 lines

package/dpdk/dpdk.hash new file +8 -0

+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.

package/dpdk/dpdk.hash

Quoted 3 lines

package/dpdk/dpdk.hash new file +3 -0

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

Should be $(TARGET_DIR).

package/dpdk/dpdk.hash

Quoted 7 lines

package/dpdk/dpdk.hash new file +7 -0

+	$(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).

package/dpdk/dpdk.hash

Quoted 2 lines

package/dpdk/dpdk.hash new file +2 -0

+	$(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.

package/dpdk/dpdk.hash

Quoted 8 lines

package/dpdk/dpdk.hash new file +8 -0

+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.

package/dpdk/dpdk.hash

Quoted 1 line

package/dpdk/dpdk.hash new file +1 -0
+	$(INSTALL) -m 0755 -D $(@D)/build/app/testpmd $(TARGET_DIR)/usr/bin

Use a complete path for the destination.

package/dpdk/dpdk.hash

Quoted 12 lines

package/dpdk/dpdk.hash new file +12 -0

+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.

Quoted 4 lines

package/dpdk/dpdk.hash new file +4 -0

+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)

package/dpdk/dpdk.hash

Quoted 10 lines

package/dpdk/dpdk.hash new file +10 -0

+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

Signature · 3 lines

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

Jan Viktorin

author

2015-12-09 23:27 UTC #14 lore raw

PATCH v2 3/3 dpdk: new package

Quoted 10 lines · On Wed, 9 Dec 2015 23:36:03 +0100 Thomas Petazzoni…

Jan,
Quoted 4 lines · Jan Viktorin
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...

Quoted 13 lines

package/dpdk/Config.in line 3
Quoted 10 lines
package/dpdk/Config.in new file +3 -0
new file mode 100644
index 0000000..a4935ae
@@ -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

Quoted 2 lines

Please wrap lines at 72 characters.

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

Quoted 5 lines

package/dpdk/Config.in line 4
Quoted 2 lines
package/dpdk/Config.in new file +1 -0
+       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.

Quoted 8 lines

package/dpdk/Config.in line 9

Quoted 6 lines

package/dpdk/Config.in new file +5 -0

+       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.

Quoted 10 lines

package/dpdk/Config.in new file +9 -0

+
+	 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/

Quoted 8 lines

package/dpdk/Config.in line 23
Quoted 5 lines
package/dpdk/Config.in new file +5 -0
+
+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...

Quoted 10 lines

package/dpdk/Config.in line 28
Quoted 6 lines
package/dpdk/Config.in new file +5 -0
+	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.

Quoted 16 lines

package/dpdk/dpdk.hash line 4
Quoted 13 lines
package/dpdk/Config.in new file +1 -0
+		if BR2_powerpc_power8  
package/dpdk/dpdk.hash new file +4 -0
new file mode 100644
index 0000000..c78ec85
@@ -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.

Quoted 10 lines

package/dpdk/dpdk.hash
Quoted 6 lines
package/dpdk/dpdk.hash new file +5 -0
+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 :).)

Quoted 6 lines

Quoted 2 lines
+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.

Quoted 22 lines

Quoted 7 lines
package/dpdk/dpdk.hash new file +6 -0
+DPDK_INSTALL_STAGING = YES
+
+DPDK_DEPENDENCIES += linux
+
+ifeq ($(BR2_PACKAGE_LIBPCAP),y)
+DPDK_DEPENDENCIES += libpcap
package/dpdk/dpdk.hash
Quoted 10 lines
package/dpdk/dpdk.hash new file +10 -0
+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:

Quoted 3 lines

- 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...

Quoted 2 lines

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.

Quoted 4 lines

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...

Quoted 4 lines

Quoted 2 lines
+# We create symlink named $(DPDK_CONFIG) to the build directory
a symlink

OK.

Quoted 7 lines

package/dpdk/dpdk.hash

Quoted 5 lines

package/dpdk/dpdk.hash new file +4 -0

+# 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.

Quoted 9 lines

package/dpdk/dpdk.hash
Quoted 5 lines
package/dpdk/dpdk.hash new file +4 -0
+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.

Quoted 3 lines

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.

Quoted 2 lines

Also, is the MAKE1 instead of MAKE intentional?

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

Quoted 6 lines

package/dpdk/dpdk.hash
Quoted 4 lines
package/dpdk/dpdk.hash new file +3 -0
+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?

Quoted 7 lines

package/dpdk/dpdk.hash

Quoted 5 lines

package/dpdk/dpdk.hash new file +4 -0

+# 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?

Quoted 12 lines

package/dpdk/dpdk.hash
Quoted 9 lines
package/dpdk/dpdk.hash new file +8 -0
+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.

Quoted 7 lines

package/dpdk/dpdk.hash
Quoted 5 lines
package/dpdk/dpdk.hash new file +3 -0
+
+define DPDK_INSTALL_TARGET_LIBS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib  
Should be $(TARGET_DIR).

OK.

Quoted 10 lines

package/dpdk/dpdk.hash
Quoted 8 lines
package/dpdk/dpdk.hash new file +7 -0
+	$(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.

Quoted 5 lines

package/dpdk/dpdk.hash
Quoted 3 lines
package/dpdk/dpdk.hash new file +2 -0
+	$(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?

Quoted 11 lines

package/dpdk/dpdk.hash

Quoted 9 lines

package/dpdk/dpdk.hash new file +8 -0

+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.

Quoted 4 lines

package/dpdk/dpdk.hash
Quoted 2 lines
package/dpdk/dpdk.hash new file +1 -0
+	$(INSTALL) -m 0755 -D $(@D)/build/app/testpmd $(TARGET_DIR)/usr/bin  
Use a complete path for the destination.

Same as above.

Quoted 17 lines

package/dpdk/dpdk.hash
Quoted 13 lines
package/dpdk/dpdk.hash new file +12 -0
+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.

Quoted 6 lines

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.

Quoted 18 lines

Quoted 5 lines

package/dpdk/dpdk.hash new file +4 -0

+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)

package/dpdk/dpdk.hash

Quoted 10 lines

package/dpdk/dpdk.hash new file +10 -0

+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!

Quoted 1 line

Thomas

Regards
Jan

Signature · 4 lines

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

Thomas Petazzoni

2015-12-10 08:54 UTC #15 lore raw

PATCH v2 3/3 dpdk: new package

Hello Jan,

Quoted 17 lines · Jan Viktorin

Quoted 4 lines
Quoted 4 lines
package/dpdk/Config.in new file +0 -1
new file mode 100644
index 0000000..a4935ae
--- /dev/null
Quoted 8 lines
package/dpdk/Config.in line 3
Quoted 5 lines
+++ b/package/dpdk/Config.in
package/dpdk/Config.in new file +3 -0
@@ -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):

Quoted 11 lines

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) || ...

Quoted 8 lines

Quoted 7 lines
package/dpdk/Config.in line 23
Quoted 4 lines
package/dpdk/Config.in new file +4 -0
+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 ?

Quoted 6 lines

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.

Quoted 11 lines

Quoted 9 lines
package/dpdk/Config.in line 28
Quoted 5 lines
package/dpdk/Config.in new file +5 -0
+	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.

Quoted 16 lines

Quoted 15 lines
package/dpdk/dpdk.hash line 4
Quoted 12 lines
package/dpdk/Config.in new file +1 -0
+		if BR2_powerpc_power8  
package/dpdk/dpdk.hash new file +4 -0
new file mode 100644
index 0000000..c78ec85
@@ -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.

Quoted 3 lines

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.

Quoted 10 lines

Quoted 9 lines
package/dpdk/dpdk.hash
Quoted 5 lines
package/dpdk/dpdk.hash new file +5 -0
+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.

Quoted 3 lines

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)

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
+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

Quoted 10 lines

Quoted 7 lines
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.

Quoted 4 lines

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.

Quoted 7 lines

Quoted 3 lines
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).

Quoted 9 lines

Quoted 8 lines
package/dpdk/dpdk.hash
Quoted 4 lines
package/dpdk/dpdk.hash new file +4 -0
+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?

Quoted 5 lines

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 :)

Quoted 3 lines

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

Ok.

Quoted 9 lines

Quoted 3 lines
Quoted 1 line
+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.

Quoted 10 lines

Quoted 8 lines
package/dpdk/dpdk.hash
Quoted 6 lines
package/dpdk/dpdk.hash new file +4 -0
+# 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.

Quoted 12 lines

Quoted 11 lines
package/dpdk/dpdk.hash
Quoted 8 lines
package/dpdk/dpdk.hash new file +8 -0
+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.

Quoted 6 lines

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.

Quoted 6 lines

Quoted 4 lines
package/dpdk/dpdk.hash
Quoted 2 lines
package/dpdk/dpdk.hash new file +2 -0
+define DPDK_INSTALL_TARGET_LIBS
+	$(INSTALL) -m 0755 -D -d $(STAGING_DIR)/usr/lib  
Should be $(TARGET_DIR).
OK.

And no -D here.

Quoted 6 lines

Quoted 4 lines
package/dpdk/dpdk.hash
Quoted 2 lines
package/dpdk/dpdk.hash new file +2 -0
+	$(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

Signature · 3 lines

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