linux-tools: add usbip (USB/IP userspace tools)
PATCH 1/1 linux-tools: add usbip (USB/IP userspace tools)
From: Julien BOIBESSOT <[email protected]>
I did this work before knowing the existence of previous attempts:
* 12/2012: http://lists.busybox.net/pipermail/buildroot/2012-December/064070.html
* 12/2016: http://buildroot-busybox.2317881.n4.nabble.com/PATCH-usbip-add-a-new-package-td152317.html
I choosed to duplicate some autostuff infra commands in linux-tools as it was more logical to me to include usbip inside linux-tools instead of trying to make it a "normal" BR package. Indeed, previous 2016 attempt finally never reached BR mainline.
Since 2012 usbip have moved from an external tool to an inside kernel sources one.
2 linux patches are added to fix issues encountered while build testing with utils/test-pkg. These patches have been sent upstream (linux-usb).
Config used for test-pkg is:
BR2_LINUX_KERNEL=y
BR2_LINUX_KERNEL_DEFCONFIG="mxs"
BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y
BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_17=y
BR2_PACKAGE_LINUX_TOOLS_USBIP=y
BR2_LINUX_KERNEL_DTS_SUPPORT=y
BR2_LINUX_KERNEL_USE_INTREE_DTS=y
BR2_LINUX_KERNEL_INTREE_DTS_NAME="imx28evk"
(some toolchains needs to select a device tree explicitly)
Results are the following:
./utils/test-pkg -c usbip.config -p linux-tool
armv5-ctng-linux-gnueabi [ 1/47]: SKIPPED
armv7-ctng-linux-gnueabihf [ 2/47]: SKIPPED
br-aarch64-glibc [ 3/47]: OK
br-arcle-hs38 [ 4/47]: OK
br-arm-basic [ 5/47]: SKIPPED
br-arm-cortex-a9-glibc [ 6/47]: OK
br-arm-cortex-a9-musl [ 7/47]: OK
br-arm-cortex-m4-full [ 8/47]: SKIPPED
br-arm-full [ 9/47]: SKIPPED
br-arm-full-nothread [10/47]: OK
br-arm-full-static [11/47]: SKIPPED
br-bfin-full [12/47]: SKIPPED
br-i386-pentium4-full [13/47]: SKIPPED
br-i386-pentium-mmx-musl [14/47]: SKIPPED
br-m68k-5208-full [15/47]: SKIPPED
br-m68k-68040-full [16/47]: OK
br-microblazeel-full [17/47]: OK
br-mips32r6-el-hf-glibc [18/47]: OK
br-mips64-n64-full [19/47]: OK
br-mips64r6-el-hf-glibc [20/47]: OK
br-mipsel-o32-full [21/47]: OK
br-nios2-glibc [22/47]: OK
br-openrisc-uclibc [23/47]: OK
br-powerpc-603e-basic-cpp [24/47]: SKIPPED
br-powerpc64le-power8-glibc [25/47]: OK
br-powerpc64-power7-glibc [26/47]: OK
br-powerpc-e500mc-full [27/47]: OK
br-sh4-full [28/47]: OK
br-sparc64-glibc [29/47]: OK
br-sparc-uclibc [30/47]: OK
br-x86-64-core2-full [31/47]: OK
br-x86-64-musl [32/47]: OK
br-xtensa-full [33/47]: OK
i686-ctng-linux-gnu [34/47]: SKIPPED
linaro-aarch64 [35/47]: OK
linaro-arm [36/47]: OK
mips64el-ctng_n32-linux-gnu [37/47]: SKIPPED
mips64el-ctng_n64-linux-gnu [38/47]: SKIPPED
powerpc-ctng_e500v2-linux-gnuspe [39/47]: SKIPPED
sourcery-arm-armv4t [40/47]: SKIPPED
sourcery-arm [41/47]: SKIPPED
sourcery-arm-thumb2 [42/47]: SKIPPED
sourcery-mips64 [43/47]: OK
sourcery-mips [44/47]: OK
sourcery-nios2 [45/47]: OK
sourcery-x86-64 [46/47]: OK
x86_64-ctng_locales-linux-gnu [47/47]: SKIPPED47 builds, 19 skipped, 0 build failed, 0 legal-info failed
--- ...tial-minor-buffer-overflow-de.patch.conditional | 39 ++++++++++++++++ ...uild-with-musl-libc-toolchain.patch.conditional | 44 ++++++++++++++++++ linux/linux.mk | 18 ++++++++ package/linux-tools/Config.in | 21 +++++++++ package/linux-tools/linux-tool-usbip.mk.in | 54 ++++++++++++++++++++++ 5 files changed, 176 insertions(+) create mode 100644 linux/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional create mode 100644 linux/0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional create mode 100644 package/linux-tools/linux-tool-usbip.mk.in
linux/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional new file +39 -0
@@ -0,0 +1,39 @@ +From 51d880bf162e88db19eb26829021078c92ad3260 Mon Sep 17 00:00:00 2001 +From: Julien BOIBESSOT <[email protected]> +Date: Tue, 5 Dec 2017 14:49:36 +0100 +Subject: [PATCH] tools/usbip: fixes potential (minor) "buffer overflow" + (detected on recent gcc with -Werror) + +Fixes following build error: +vhci_driver.c: In function 'refresh_imported_device_list': +vhci_driver.c:118:37: error: 'snprintf' output may be truncated before + the last format character [-Werror=format-truncation=] + snprintf(status, sizeof(status), "status.%d", i); + ^~~~~~~~~~~ +vhci_driver.c:118:4: note: 'snprintf' output between 9 and 18 bytes into + a destination of size 17 + snprintf(status, sizeof(status), "status.%d", i); + ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +cc1: all warnings being treated as errors + +Signed-off-by: Julien BOIBESSOT <[email protected]> +--- + tools/usb/usbip/libsrc/vhci_driver.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c +index 5727dfb..a9ce431 100644 +--- a/tools/usb/usbip/libsrc/vhci_driver.c ++++ b/tools/usb/usbip/libsrc/vhci_driver.c +@@ -106,7 +106,7 @@ static int parse_status(const char *value) + return 0; + } + +-#define MAX_STATUS_NAME 16 ++#define MAX_STATUS_NAME 18 + + static int refresh_imported_device_list(void) + { +-- +2.1.4 +
linux/0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional new file +44 -0
44 lines not on this page · raw message
linux/linux.mk +18 -0
@@ -221,6 +221,24 @@ define LINUX_TRY_PATCH_TIMECONST endef LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_TIMECONST +# Fixes tools/usbip build with recent gcc when -Werror is used +# Try a dry-run patch to see if this applies, if it does go ahead +define LINUX_TRY_PATCH_USBIP_GCC + @if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional >/dev/null ; then \ + $(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional ; \ + fi +endef +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_USBIP_GCC + +# Fixes tools/usbip build with musl toolchains +# Try a dry-run patch to see if this applies, if it does go ahead +define LINUX_TRY_PATCH_USBIP_MUSL + @if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional >/dev/null ; then \ + $(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional ; \ + fi +endef +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_USBIP_MUSL + ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y) LINUX_KCONFIG_DEFCONFIG = $(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig else ifeq ($(BR2_LINUX_KERNEL_USE_ARCH_DEFAULT_CONFIG),y)
package/linux-tools/Config.in +21 -0
@@ -85,4 +85,25 @@ config BR2_PACKAGE_LINUX_TOOLS_TMON tmon is a terminal-based tool (using curses) that allows the user to access thermal information about the system. +config BR2_PACKAGE_LINUX_TOOLS_USBIP + bool "usbip" + depends on BR2_PACKAGE_HAS_UDEV + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_17 # moved out of staging/ dir in kernel 3.17 + select BR2_PACKAGE_LINUX_TOOLS + help + USB/IP protocol allows to pass USB device from server to client over + the network. + You need to activate support for it in your kernel configuration. + + This (usbip) is the set of userspace tools used to handle connection + and management. + + You can optionally add hwdata package to your BR config to have + better runtime experience. + + https://github.com/torvalds/linux/blob/master/tools/usb/usbip/README + +comment "usbip needs udev /dev management and a toolchain w/ headers >= 3.17" + depends on !BR2_PACKAGE_HAS_UDEV || !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_17 + endmenu
package/linux-tools/linux-tool-usbip.mk.in new file +54 -0
54 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH 1/1 linux-tools: add usbip (USB/IP userspace tools)
Hello,
+Yann in Cc, as I'd really like to have his feedback on this patch.
Title should be just:
"linux-tools: add support for usbip"
Quoted 3 lines · julien.boibessot at free.fr
From: Julien BOIBESSOT <[email protected]>
I did this work before knowing the existence of previous attempts:
Please don't use first person sentences in commit logs.
Quoted 76 lines
* 12/2012: http://lists.busybox.net/pipermail/buildroot/2012-December/064070.html
* 12/2016: http://buildroot-busybox.2317881.n4.nabble.com/PATCH-usbip-add-a-new-package-td152317.htmlI choosed to duplicate some autostuff infra commands in linux-tools as it was more logical to me to include usbip inside linux-tools instead of trying to make it a "normal" BR package. Indeed, previous 2016 attempt finally never reached BR mainline.
Since 2012 usbip have moved from an external tool to an inside kernel sources one.2 linux patches are added to fix issues encountered while build testing with utils/test-pkg. These patches have been sent upstream (linux-usb).
Config used for test-pkg is:
BR2_LINUX_KERNEL=y
BR2_LINUX_KERNEL_DEFCONFIG="mxs"
BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_EUDEV=y
BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_17=y
BR2_PACKAGE_LINUX_TOOLS_USBIP=y
BR2_LINUX_KERNEL_DTS_SUPPORT=y
BR2_LINUX_KERNEL_USE_INTREE_DTS=y
BR2_LINUX_KERNEL_INTREE_DTS_NAME="imx28evk"(some toolchains needs to select a device tree explicitly)
Results are the following:
./utils/test-pkg -c usbip.config -p linux-tool
armv5-ctng-linux-gnueabi [ 1/47]: SKIPPED armv7-ctng-linux-gnueabihf [ 2/47]: SKIPPED br-aarch64-glibc [ 3/47]: OK br-arcle-hs38 [ 4/47]: OK br-arm-basic [ 5/47]: SKIPPED br-arm-cortex-a9-glibc [ 6/47]: OK br-arm-cortex-a9-musl [ 7/47]: OK br-arm-cortex-m4-full [ 8/47]: SKIPPED br-arm-full [ 9/47]: SKIPPED br-arm-full-nothread [10/47]: OK br-arm-full-static [11/47]: SKIPPED br-bfin-full [12/47]: SKIPPED br-i386-pentium4-full [13/47]: SKIPPED br-i386-pentium-mmx-musl [14/47]: SKIPPED br-m68k-5208-full [15/47]: SKIPPED br-m68k-68040-full [16/47]: OK br-microblazeel-full [17/47]: OK br-mips32r6-el-hf-glibc [18/47]: OK br-mips64-n64-full [19/47]: OK br-mips64r6-el-hf-glibc [20/47]: OK br-mipsel-o32-full [21/47]: OK br-nios2-glibc [22/47]: OK br-openrisc-uclibc [23/47]: OK br-powerpc-603e-basic-cpp [24/47]: SKIPPED br-powerpc64le-power8-glibc [25/47]: OK br-powerpc64-power7-glibc [26/47]: OK br-powerpc-e500mc-full [27/47]: OK br-sh4-full [28/47]: OK br-sparc64-glibc [29/47]: OK br-sparc-uclibc [30/47]: OK br-x86-64-core2-full [31/47]: OK br-x86-64-musl [32/47]: OK br-xtensa-full [33/47]: OK i686-ctng-linux-gnu [34/47]: SKIPPED linaro-aarch64 [35/47]: OK linaro-arm [36/47]: OK mips64el-ctng_n32-linux-gnu [37/47]: SKIPPED mips64el-ctng_n64-linux-gnu [38/47]: SKIPPED powerpc-ctng_e500v2-linux-gnuspe [39/47]: SKIPPED sourcery-arm-armv4t [40/47]: SKIPPED sourcery-arm [41/47]: SKIPPED sourcery-arm-thumb2 [42/47]: SKIPPED sourcery-mips64 [43/47]: OK sourcery-mips [44/47]: OK sourcery-nios2 [45/47]: OK sourcery-x86-64 [46/47]: OK x86_64-ctng_locales-linux-gnu [47/47]: SKIPPED
Quoted 1 line
47 builds, 19 skipped, 0 build failed, 0 legal-info failed
Does it makes sense to include the test-pkg details in the commit log? This is a real open question. We don't do it today, but I might see some value in having it. What do others think ?
linux/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional
Quoted 1 line
linux/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional +0
-0
I'm wondering if those two patches should be in linux/ or in package/linux-tools/. Not sure.
Quoted 15 lines
linux/linux.mk +7
-0@@ -221,6 +221,24 @@ define LINUX_TRY_PATCH_TIMECONST endef LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_TIMECONST +# Fixes tools/usbip build with recent gcc when -Werror is used +# Try a dry-run patch to see if this applies, if it does go ahead +define LINUX_TRY_PATCH_USBIP_GCC + @if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional >/dev/null ; then \ + $(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional ; \ + fi +endef
linux/linux.mk define LINUX_TRY_PATCH_TIMECONST · line 240
Quoted 10 lines
linux/linux.mk +10
-0+LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_USBIP_GCC + +# Fixes tools/usbip build with musl toolchains +# Try a dry-run patch to see if this applies, if it does go ahead +define LINUX_TRY_PATCH_USBIP_MUSL + @if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional >/dev/null ; then \ + $(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional ; \ + fi +endef +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_USBIP_MUSL
Should we have this logic in linux/linux.mk, or in package/linux-tools/linux-tool-usbip.mk.in, possibly via some additional extension of the linux-tools infrastructure to register some patches to be applied ?
Again, not sure, I'm just thinking out loud.
Since this "let's try to apply a patch, but it fails don't error out" logic is now duplicated three times, perhaps it calls for a TRY_APPLY_PATCHES function, or something like that ?
linux/linux.mk define LINUX_TRY_PATCH_TIMECONST
Quoted 9 lines
linux/linux.mk +9
-0+config BR2_PACKAGE_LINUX_TOOLS_USBIP + bool "usbip" + depends on BR2_PACKAGE_HAS_UDEV + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_17 # moved out of staging/ dir in kernel 3.17 + select BR2_PACKAGE_LINUX_TOOLS + help + USB/IP protocol allows to pass USB device from server to client over + the network. + You need to activate support for it in your kernel configuration.
Listing the actual kernel config options that are needed would be more useful.
linux/linux.mk
Quoted 5 lines
linux/linux.mk +5
-0+ This (usbip) is the set of userspace tools used to handle connection + and management. + + You can optionally add hwdata package to your BR config to have + better runtime experience.
BR -> Buildroot
config -> configuration
"better runtime experience" is somewhat fuzzy. Can we have a better description ?
linux/linux.mk
Quoted 13 lines
linux/linux.mk +13
-0+LINUX_TOOLS += usbip + +USBIP_DEPENDENCIES = udev +USBIP_CONF_OPTS = --with-tcp-wrappers=no + +define USBIP_BUILD_CMDS + $(Q)if test ! -f $(LINUX_DIR)/tools/usb/usbip/Makefile.am ; then \ + echo "Your kernel version is too old and does not have the usbip tool." ; \ + echo "At least kernel 3.17 must be used." ; \ + exit 1 ; \ + fi + + cd $(LINUX_DIR)/tools/usb/usbip && PATH=$(BR_PATH) ./autogen.sh
If you autogen, you need to add at least host-autoconf and host-automake as dependencies.
Quoted 5 lines
linux/linux.mk +5
-0+ cd $(LINUX_DIR)/tools/usb/usbip && rm -rf config.cache && \ + $(TARGET_CONFIGURE_OPTS) \ + $(TARGET_CONFIGURE_ARGS) \ + $(USBIP_CONF_ENV) \ + CONFIG_SITE=/dev/null \
linux/linux.mk
Quoted 10 lines
linux/linux.mk +10
-0+ ./configure \ + --target=$(GNU_TARGET_NAME) \ + --host=$(GNU_TARGET_NAME) \ + --build=$(GNU_HOST_NAME) \ + --prefix=/usr \ + --exec-prefix=/usr \ + --sysconfdir=/etc \ + --localstatedir=/var \ + --program-prefix="" \ + $(QUIET) $(USBIP_CONF_OPTS)
Shouldn't we add a configure step for linux-tools ?
Quoted 9 lines
linux/linux.mk +9
-0+ + $(TARGET_MAKE_ENV) $(MAKE) $(USBIP_MAKE_OPTS) \ + -C $(LINUX_DIR)/tools/usb/usbip/ +endef + +# for libusbip +define USBIP_INSTALL_STAGING_CMDS + $(TARGET_MAKE_ENV) $(MAKE) -C $(LINUX_DIR)/tools/usb/usbip \ + $(USBIP_MAKE_OPTS) \
linux/linux.mk
Quoted 10 lines
linux/linux.mk +10
-0+ DESTDIR=$(STAGING_DIR) \ + install +endef + +define USBIP_INSTALL_TARGET_CMDS + $(TARGET_MAKE_ENV) $(MAKE) -C $(LINUX_DIR)/tools/usb/usbip \ + $(USBIP_MAKE_OPTS) \ + DESTDIR=$(TARGET_DIR) \ + install +endef
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 linux-tools: add usbip (USB/IP userspace tools)
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas Petazzoni <[email protected]> writes:
Hi,
Quoted 10 lines
Quoted 9 lines
sourcery-arm-armv4t [40/47]: SKIPPED
sourcery-arm [41/47]: SKIPPED
sourcery-arm-thumb2 [42/47]: SKIPPED sourcery-mips64 [43/47]: OK
sourcery-mips [44/47]: OK
sourcery-nios2 [45/47]: OK
sourcery-x86-64 [46/47]: OK x86_64-ctng_locales-linux-gnu [47/47]: SKIPPED
47 builds, 19 skipped, 0 build failed, 0 legal-info failed
Quoted 3 lines
Does it makes sense to include the test-pkg details in the commit log? This is a real open question. We don't do it today, but I might see some value in having it. What do others think ?
I think it does. It is useful information and if it is in the commit message then it is easy to find back.
Signature · 1 line
Bye, Peter Korsgaard
PATCH 1/1 linux-tools: add usbip (USB/IP userspace tools)
In reply to Thomas PetazzoniJulien, All,
On 2017-12-14 09:56 +0100, Thomas Petazzoni spake thusly:
Quoted 1 line
+Yann in Cc, as I'd really like to have his feedback on this patch.
Ah, yep, I'd have expected to be in Cc: for the patch. ;-)
Quoted 10 lines
Title should be just:
"linux-tools: add support for usbip"
Quoted 3 lines · julien.boibessot at free.fr
From: Julien BOIBESSOT <[email protected]>
I did this work before knowing the existence of previous attempts:
Please don't use first person sentences in commit logs.
Usually, use 'we' instead, like;
We use the git version becasue blabla... Autoreconf does not work, so we trick it into believing it has been autoreconf by blabla...
However, if you have a "personal" message (like what you write), you can keep speaking with 'I' but put it below the --- line (which marks the end of the commit message, and that git-am will ignore when the patch is applied). See for example:
https://patchwork.ozlabs.org/patch/843728/
Quoted 2 lines
I find it interesting that your referenced the prior art on this, and it is defeintely something that I'd like to see below the ---.
[--SNIP--]
Quoted 5 lines
Quoted 1 line
47 builds, 19 skipped, 0 build failed, 0 legal-info failed
Does it makes sense to include the test-pkg details in the commit log? This is a real open question. We don't do it today, but I might see some value in having it. What do others think ?
If everything is workih as expected, then no, except maybe a note like "the defconfig below builds with test-pkg with no error".
Quoted 4 lines
linux/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional
Quoted 1 line
linux/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional +0
-0I'm wondering if those two patches should be in linux/ or in package/linux-tools/. Not sure.
If they go into linux-tools, they won;t be applied because linux-tools does not have a source. Rather, they piggyback onto hte kernel sources and build directly in $(LINUX_DIR).
Yes, that's is ugly and a special case I'm investigating for OOT. And maybe something you'll also want to keep an eye on for TLPB as well...
Quoted 22 lines
Quoted 22 lines
linux/linux.mk +14
-0@@ -221,6 +221,24 @@ define LINUX_TRY_PATCH_TIMECONST endef LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_TIMECONST +# Fixes tools/usbip build with recent gcc when -Werror is used +# Try a dry-run patch to see if this applies, if it does go ahead +define LINUX_TRY_PATCH_USBIP_GCC + @if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional >/dev/null ; then \ + $(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional ; \ + fi +endef +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_USBIP_GCC + +# Fixes tools/usbip build with musl toolchains +# Try a dry-run patch to see if this applies, if it does go ahead +define LINUX_TRY_PATCH_USBIP_MUSL + @if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional >/dev/null ; then \ + $(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional ; \
Quoted 8 lines
linux/linux.mk define LINUX_TRY_PATCH_TIMECONST · line 240
Quoted 3 lines
linux/linux.mk +3
-0+ fi +endef +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_USBIP_MUSLShould we have this logic in
linux/linux.mk, or in package/linux-tools/linux-tool-usbip.mk.in, possibly via some additional extension of the linux-tools infrastructure to register some patches to be applied ?
One would argue that those are the source of the kernel tree, so the tweaks belong to linux.mk.
OTOH, we are fixing usbip, which is built by the linux-tools package.
Quoted 1 line
Again, not sure, I'm just thinking out loud.
I'm not covinced either way, but to be pragmatic, I'd keep those in linux.mk.
Quoted 3 lines
Since this "let's try to apply a patch, but it fails don't error out" logic is now duplicated three times, perhaps it calls for a
TRY_APPLY_PATCHESfunction, or something like that ?
Yep. Julien, would you volunteer? ;-)
Quoted 11 lines
linux/linux.mk define LINUX_TRY_PATCH_TIMECONST
Quoted 5 lines
linux/linux.mk +5
-0+ This (usbip) is the set of userspace tools used to handle connection + and management. + + You can optionally add hwdata package to your BR config to have + better runtime experience.BR -> Buildroot
config -> configuration"better runtime experience" is somewhat fuzzy. Can we have a better description ?
If the exprience is "so much better", why don't we forcibly select it? Is it only because hwdata is big?
[--SNIP--]
Quoted 9 lines
Quoted 9 lines
linux/linux.mk +9
-0+ cd $(LINUX_DIR)/tools/usb/usbip && rm -rf config.cache && \ + $(TARGET_CONFIGURE_OPTS) \ + $(TARGET_CONFIGURE_ARGS) \ + $(USBIP_CONF_ENV) \ + CONFIG_SITE=/dev/null \ + ./configure \ + --target=$(GNU_TARGET_NAME) \ + --host=$(GNU_TARGET_NAME) \ + --build=$(GNU_HOST_NAME) \
Quoted 8 lines
linux/linux.mk
Quoted 6 lines
linux/linux.mk +6
-0+ --prefix=/usr \ + --exec-prefix=/usr \ + --sysconfdir=/etc \ + --localstatedir=/var \ + --program-prefix="" \ + $(QUIET) $(USBIP_CONF_OPTS)Shouldn't we add a configure step for linux-tools ?
Yes. That does not need to be a separate patch, but that'd be better if it were.
Thanks! :-)
Regards,
Yann E. MORIN.
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 linux-tools: add usbip (USB/IP userspace tools)
Hello,
Yann, Thomas, Peter,
thanks for review !
Quoted 5 lines · Yann E. MORIN
Julien, All,
On 2017-12-14 09:56 +0100, Thomas Petazzoni spake thusly:
Quoted 1 line
+Yann in Cc, as I'd really like to have his feedback on this patch.
Ah, yep, I'd have expected to be in Cc: for the patch. ;-)
Yeap sorry, next version, I promise.
Quoted 74 lines
Quoted 10 lines
Title should be just:
"linux-tools: add support for usbip"
Quoted 3 lines · julien.boibessot at free.fr
From: Julien BOIBESSOT <[email protected]>
I did this work before knowing the existence of previous attempts:
Please don't use first person sentences in commit logs.
Usually, use 'we' instead, like;
We use the git version becasue blabla... Autoreconf does not work, so we trick it into believing it has been autoreconf by blabla...
However, if you have a "personal" message (like what you write), you can keep speaking with 'I' but put it below the --- line (which marks the end of the commit message, and that git-am will ignore when the patch is applied). See for example:
https://patchwork.ozlabs.org/patch/843728/Quoted 2 lines
I find it interesting that your referenced the prior art on this, and it is defeintely something that I'd like to see below the ---.
[--SNIP--]
Quoted 4 lines
Quoted 1 line
47 builds, 19 skipped, 0 build failed, 0 legal-info failed
Does it makes sense to include the test-pkg details in the commit log? This is a real open question. We don't do it today, but I might see some value in having it. What do others think ?
If everything is workih as expected, then no, except maybe a note like "the defconfig below builds with test-pkg with no error".
Quoted 3 lines
linux/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional
Quoted 1 line
linux/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional +0
-0I'm wondering if those two patches should be in linux/ or in package/linux-tools/. Not sure.
If they go into linux-tools, they won;t be applied because linux-tools does not have a source. Rather, they piggyback onto hte kernel sources and build directly in $(LINUX_DIR).
Yes, that's is ugly and a special case I'm investigating for OOT. And maybe something you'll also want to keep an eye on for TLPB as well...
Quoted 21 lines
Quoted 21 lines
linux/linux.mk +13
-0@@ -221,6 +221,24 @@ define LINUX_TRY_PATCH_TIMECONST endef LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_TIMECONST +# Fixes tools/usbip build with recent gcc when -Werror is used +# Try a dry-run patch to see if this applies, if it does go ahead +define LINUX_TRY_PATCH_USBIP_GCC + @if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional >/dev/null ; then \ + $(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional ; \ + fi +endef +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_USBIP_GCC + +# Fixes tools/usbip build with musl toolchains +# Try a dry-run patch to see if this applies, if it does go ahead +define LINUX_TRY_PATCH_USBIP_MUSL + @if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional >/dev/null ; then \Quoted 8 lines
linux/linux.mk define LINUX_TRY_PATCH_TIMECONST · line 240
Quoted 4 lines
linux/linux.mk +4
-0+ $(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional ; \ + fi +endef +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_USBIP_MUSLShould we have this logic in
linux/linux.mk, or in package/linux-tools/linux-tool-usbip.mk.in, possibly via some additional extension of the linux-tools infrastructure to register some patches to be applied ?
Quoted 4 lines
One would argue that those are the source of the kernel tree, so the tweaks belong to linux.mk.
OTOH, we are fixing usbip, which is built by the linux-tools package.
For your information, my patches have been accepted by Greg KH for next Linux version.
Quoted 9 lines
Quoted 2 lines
Again, not sure, I'm just thinking out loud.
I'm not covinced either way, but to be pragmatic, I'd keep those in linux.mk.
Quoted 3 lines
Since this "let's try to apply a patch, but it fails don't error out" logic is now duplicated three times, perhaps it calls for a
TRY_APPLY_PATCHESfunction, or something like that ?Yep. Julien, would you volunteer? ;-)
I will have a look at it between turkey and foie gras ;-)
Quoted 9 lines
Quoted 9 lines
linux/linux.mk define LINUX_TRY_PATCH_TIMECONST
Quoted 6 lines
linux/linux.mk +5
-0+ This (usbip) is the set of userspace tools used to handle connection + and management. + + You can optionally add hwdata package to your BR config to have + better runtime experience.BR -> Buildroot
config -> configuration
Quoted 4 lines
Quoted 2 lines
"better runtime experience" is somewhat fuzzy. Can we have a better description ?
If the exprience is "so much better", why don't we forcibly select it? Is it only because hwdata is big?
yes, hwdata is big. "better experience" means you see vendors/devices real names and not USB VID/PID. Moreover hwdata install PCI and USB infos and PCI is not needed in that case.
Quoted 18 lines
[--SNIP--]
Quoted 8 lines
Quoted 8 lines
linux/linux.mk +8
-0+ cd $(LINUX_DIR)/tools/usb/usbip && rm -rf config.cache && \ + $(TARGET_CONFIGURE_OPTS) \ + $(TARGET_CONFIGURE_ARGS) \ + $(USBIP_CONF_ENV) \ + CONFIG_SITE=/dev/null \ + ./configure \ + --target=$(GNU_TARGET_NAME) \ + --host=$(GNU_TARGET_NAME) \Quoted 8 lines
linux/linux.mk
Quoted 7 lines
linux/linux.mk +7
-0+ --build=$(GNU_HOST_NAME) \ + --prefix=/usr \ + --exec-prefix=/usr \ + --sysconfdir=/etc \ + --localstatedir=/var \ + --program-prefix="" \ + $(QUIET) $(USBIP_CONF_OPTS)Shouldn't we add a configure step for linux-tools ?
Quoted 2 lines
Yes. That does not need to be a separate patch, but that'd be better if it were.
Understood.
Thanks and regards, and merry Christmas ;-)
Julien
PATCH 1/1 linux-tools: add usbip (USB/IP userspace tools)
Hi Julien;
Le 22/12/2017 ? 10:44, Julien Boibessot a ?crit?:
Quoted 89 lines
Hello,
Yann, Thomas, Peter,
thanks for review !
Quoted 5 lines · Yann E. MORIN
Julien, All,
On 2017-12-14 09:56 +0100, Thomas Petazzoni spake thusly:
Quoted 1 line
+Yann in Cc, as I'd really like to have his feedback on this patch.
Ah, yep, I'd have expected to be in Cc: for the patch. ;-)
Yeap sorry, next version, I promise.
Quoted 74 lines
Quoted 10 lines
Title should be just:
"linux-tools: add support for usbip"
Quoted 3 lines · julien.boibessot at free.fr
From: Julien BOIBESSOT <[email protected]>
I did this work before knowing the existence of previous attempts:
Please don't use first person sentences in commit logs.
Usually, use 'we' instead, like;
We use the git version becasue blabla... Autoreconf does not work, so we trick it into believing it has been autoreconf by blabla...
However, if you have a "personal" message (like what you write), you can keep speaking with 'I' but put it below the --- line (which marks the end of the commit message, and that git-am will ignore when the patch is applied). See for example:
https://patchwork.ozlabs.org/patch/843728/Quoted 2 lines
I find it interesting that your referenced the prior art on this, and it is defeintely something that I'd like to see below the ---.
[--SNIP--]
Quoted 4 lines
Quoted 1 line
47 builds, 19 skipped, 0 build failed, 0 legal-info failed
Does it makes sense to include the test-pkg details in the commit log? This is a real open question. We don't do it today, but I might see some value in having it. What do others think ?
If everything is workih as expected, then no, except maybe a note like "the defconfig below builds with test-pkg with no error".
Quoted 3 lines
linux/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional
Quoted 1 line
linux/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional +0
-0I'm wondering if those two patches should be in linux/ or in package/linux-tools/. Not sure.
If they go into linux-tools, they won;t be applied because linux-tools does not have a source. Rather, they piggyback onto hte kernel sources and build directly in $(LINUX_DIR).
Yes, that's is ugly and a special case I'm investigating for OOT. And maybe something you'll also want to keep an eye on for TLPB as well...
Quoted 29 lines
Quoted 15 lines
linux/linux.mk +7
-0@@ -221,6 +221,24 @@ define LINUX_TRY_PATCH_TIMECONST endef LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_TIMECONST +# Fixes tools/usbip build with recent gcc when -Werror is used +# Try a dry-run patch to see if this applies, if it does go ahead +define LINUX_TRY_PATCH_USBIP_GCC + @if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional >/dev/null ; then \ + $(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0002-tools-usbip-fixes-potential-minor-buffer-overflow-de.patch.conditional ; \ + fi +endeflinux/linux.mk define LINUX_TRY_PATCH_TIMECONST · line 240
Quoted 10 lines
linux/linux.mk +10
-0+LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_USBIP_GCC + +# Fixes tools/usbip build with musl toolchains +# Try a dry-run patch to see if this applies, if it does go ahead +define LINUX_TRY_PATCH_USBIP_MUSL + @if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional >/dev/null ; then \ + $(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0003-tools-usbip-fixes-build-with-musl-libc-toolchain.patch.conditional ; \ + fi +endef +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_USBIP_MUSLShould we have this logic in
linux/linux.mk, or in package/linux-tools/linux-tool-usbip.mk.in, possibly via some additional extension of the linux-tools infrastructure to register some patches to be applied ?
Quoted 7 lines
Quoted 4 lines
One would argue that those are the source of the kernel tree, so the tweaks belong to linux.mk.
OTOH, we are fixing usbip, which is built by the linux-tools package.
For your information, my patches have been accepted by Greg KH for next Linux version.
Ok great :)
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Again, not sure, I'm just thinking out loud.
I'm not covinced either way, but to be pragmatic, I'd keep those in linux.mk.
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
Since this "let's try to apply a patch, but it fails don't error out" logic is now duplicated three times, perhaps it calls for a
TRY_APPLY_PATCHESfunction, or something like that ?Yep. Julien, would you volunteer? ;-)
I will have a look at it between turkey and foie gras ;-)
So, in the mean time I'll mark this patch "Changes requested" in patchwork.
Best regards,
Romain
Quoted 50 lines
Quoted 10 lines
Quoted 10 lines
linux/linux.mk define LINUX_TRY_PATCH_TIMECONST
Quoted 7 lines
linux/linux.mk +5
-0+ This (usbip) is the set of userspace tools used to handle connection + and management. + + You can optionally add hwdata package to your BR config to have + better runtime experience.BR -> Buildroot
config -> configurationQuoted 4 lines
Quoted 2 lines
"better runtime experience" is somewhat fuzzy. Can we have a better description ?
If the exprience is "so much better", why don't we forcibly select it? Is it only because hwdata is big?
yes, hwdata is big. "better experience" means you see vendors/devices real names and not USB VID/PID. Moreover hwdata install PCI and USB infos and PCI is not needed in that case.
Quoted 18 lines
[--SNIP--]
Quoted 8 lines
Quoted 8 lines
linux/linux.mk +8
-0+ cd $(LINUX_DIR)/tools/usb/usbip && rm -rf config.cache && \ + $(TARGET_CONFIGURE_OPTS) \ + $(TARGET_CONFIGURE_ARGS) \ + $(USBIP_CONF_ENV) \ + CONFIG_SITE=/dev/null \ + ./configure \ + --target=$(GNU_TARGET_NAME) \ + --host=$(GNU_TARGET_NAME) \Quoted 8 lines
linux/linux.mk
Quoted 7 lines
linux/linux.mk +7
-0+ --build=$(GNU_HOST_NAME) \ + --prefix=/usr \ + --exec-prefix=/usr \ + --sysconfdir=/etc \ + --localstatedir=/var \ + --program-prefix="" \ + $(QUIET) $(USBIP_CONF_OPTS)Shouldn't we add a configure step for linux-tools ?
Quoted 2 lines
Yes. That does not need to be a separate patch, but that'd be better if it were.
Understood.
Thanks and regards, and merry Christmas ;-)
Julien
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot