package/iproute2: disable iptables support for static build

PATCH New Romain Naour 3 participants 6 messages May 18, 2015 – May 25, 2015

lore

Share & tools

lore

Romain Naour

author

2015-05-18 20:54 UTC #0 lore raw

PATCH 1/1 package/iproute2: disable iptables support for static build

If iptable support is enabled, it will try to build m_xt.so even for static build only.

So, disable iptables support for static build and remove TC_CONFIG_IPSET and TC_CONFIG_XT if previously added by the configure script.

Fixes:
http://autobuild.buildroot.net/results/3b3/3b37871bdf5766677fc20dca22c13177091d104f/

Signed-off-by Romain Naour <[email protected]>

---
 package/iproute2/iproute2.mk | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

package/iproute2/iproute2.mk +4 -1

index 9595165..d97e75e 100644
@@ -18,7 +18,7 @@ IPROUTE2_DEPENDENCIES += busybox
 endif
 
 # If we've got iptables enable xtables support for tc
-ifeq ($(BR2_PACKAGE_IPTABLES),y)
+ifeq ($(BR2_PACKAGE_IPTABLES)x$(BR2_STATIC_LIBS),yx)
 IPROUTE2_DEPENDENCIES += iptables
 define IPROUTE2_WITH_IPTABLES
 	# Makefile is busted so it never passes IPT_LIB_DIR properly

@@ -27,6 +27,9 @@ define IPROUTE2_WITH_IPTABLES
 endef
 else
 define IPROUTE2_WITH_IPTABLES
+	# delete TC_CONFIG_IPSET and TC_CONFIG_XT if any
+	$(SED) '/TC_CONFIG_IPSET/d' $(IPROUTE2_DIR)/Config
+	$(SED) '/TC_CONFIG_XT/d' $(IPROUTE2_DIR)/Config
 	# em_ipset needs xtables, but configure misdetects it
 	echo "TC_CONFIG_IPSET:=n" >>$(IPROUTE2_DIR)/Config
 endef

Signature · 1 line

1.9.3

Peter Korsgaard

2015-05-19 19:33 UTC #1 lore raw

PATCH 1/1 package/iproute2: disable iptables support for static build

Quoted 8 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Romain" == Romain Naour <[email protected]> writes:
If iptable support is enabled, it will try to build m_xt.so even for static build only.
So, disable iptables support for static build and remove TC_CONFIG_IPSET and TC_CONFIG_XT if previously added by the configure script.

The first part I get, but what are those sed invocations needed for?

I see the configure script is using pkg-config but we don't depend on host-pkgconf. Running the configure script with/without I see the following differences:

build/iproute2-4.0.0/Config.with 2015-05-19 21:03:39.835184550 +0200 renamed from build/iproute2-4.0.0/Config.without 2015-05-19 21:03:14.735183540 +0200 +2 -1

@@ -2,8 +2,9 @@
 PKG_CONFIG:=/home/peko/source/buildroot/output/host/usr/bin/pkg-config
 AR:=/home/peko/source/buildroot/output/host/usr/bin/arm-linux-ar
 CC:=/home/peko/source/buildroot/output/host/usr/bin/arm-linux-gcc
+TC_CONFIG_XT:=y
 TC_CONFIG_IPSET:=y
-IPT_LIB_DIR:=/lib/xtables
+IPT_LIB_DIR:=/usr/lib/xtables
 HAVE_LATEX:=y
 HAVE_PDFLATEX:=y
 IPT_LIB_DIR:=/usr/lib/xtables

The whole iptables handling seems pretty broken. TC_CONFIG_XT gets correctly detected if available, so we don't need to overwrite that, and looking at the code, both -DIPT_LIB_DIR= and -DXT_LIB_DIR should be passed to the compiler, as the default values fit neither of them.

I'm not sure why we are mangling CONFIG_IPSET as the only place it is used is in tc/Makefile:

ifeq ($(TC_CONFIG_IPSET), y)
ifeq ($(TC_CONFIG_XT), y)

    TCMODULES += em_ipset.o

endif
endif

So as long as the detection works it should be enough to force TC_CONFIG_XT off in the static case.

So what about we do:

- Add host-pkgconf to _DEPENDENCIES
- Add the BR2_STATIC_LIBS conditional for iptables and force TC_CONFIG_XT:=n in the else clause.
- Drop the TC_CONFIG_XT:=y and TC_CONFIG_IPSET:=n overrides
- Replace the s/-DIPT/-DXT/ handling for iptables with something like:
$(SED) 's/XT_LIB_DIR/IPT_LIB_DIR/' $(@D)/tc/m_xt.c
- Drop the IPT_LIB_DIR override in _CONFIGURE_CMDS

Gustavo, you normally take care of iproute2, what do you say?

Quoted 39 lines

Fixes:

http://autobuild.buildroot.net/results/3b3/3b37871bdf5766677fc20dca22c13177091d104f/

Signed-off-by Romain Naour <[email protected]>

---
 package/iproute2/iproute2.mk | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

package/iproute2/iproute2.mk +4 -1

index 9595165..d97e75e 100644
@@ -18,7 +18,7 @@ IPROUTE2_DEPENDENCIES += busybox
 endif
 
 # If we've got iptables enable xtables support for tc
-ifeq ($(BR2_PACKAGE_IPTABLES),y)
+ifeq ($(BR2_PACKAGE_IPTABLES)x$(BR2_STATIC_LIBS),yx)
 IPROUTE2_DEPENDENCIES += iptables
 define IPROUTE2_WITH_IPTABLES
 	# Makefile is busted so it never passes IPT_LIB_DIR properly

@@ -27,6 +27,9 @@ define IPROUTE2_WITH_IPTABLES
 endef
 else
 define IPROUTE2_WITH_IPTABLES
+	# delete TC_CONFIG_IPSET and TC_CONFIG_XT if any
+	$(SED) '/TC_CONFIG_IPSET/d' $(IPROUTE2_DIR)/Config
+	$(SED) '/TC_CONFIG_XT/d' $(IPROUTE2_DIR)/Config
 	# em_ipset needs xtables, but configure misdetects it
 	echo "TC_CONFIG_IPSET:=n" >>$(IPROUTE2_DIR)/Config
 endef

Signature · 6 lines

1.9.3

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 1 line

Bye, Peter Korsgaard

Gustavo Zacarias

2015-05-20 03:57 UTC #2 lore raw

PATCH 1/1 package/iproute2: disable iptables support for static build

Quoted 11 lines · Peter Korsgaard

So what about we do:
- Add host-pkgconf to _DEPENDENCIES
- Add the BR2_STATIC_LIBS conditional for iptables and force TC_CONFIG_XT:=n in the else clause.
- Drop the TC_CONFIG_XT:=y and TC_CONFIG_IPSET:=n overrides
- Replace the s/-DIPT/-DXT/ handling for iptables with something like:
   $(SED) 's/XT_LIB_DIR/IPT_LIB_DIR/' $(@D)/tc/m_xt.c
- Drop the IPT_LIB_DIR override in _CONFIGURE_CMDS
Gustavo, you normally take care of iproute2, what do you say?

The iproute2 "build system" periodically changes and breaks things up, so it doesn't surprise me at all.
Normally the extensions dir (IPT_LIB_DIR) should point to /usr/lib/xtables as defined by the iptables package.
The ipset bits on tc are to match an ipset (rather than the usual ip/subnet) to a class to shape/whatever.
I'm currently AFH (away from home) so i don't have my networking testing rig handy, but in principle it sounds ok as long as the checks in the makefiles/code have been fixed.
I'll be back on monday, so we can either wait for testing or just go with it and i'll test when i'm back and scream if something breaks ;)
Caveat: normally netfilter devs don't test/care that much about static scenarios, they'll take patches but don't expect things to remain unbroken for an extended period of time.
Regards.

Peter Korsgaard

2015-05-21 21:22 UTC #3 lore raw

PATCH 1/1 package/iproute2: disable iptables support for static build

In reply to Romain Naour

Quoted 11 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Romain" == Romain Naour <[email protected]> writes:
If iptable support is enabled, it will try to build m_xt.so even for static build only.
So, disable iptables support for static build and remove TC_CONFIG_IPSET and TC_CONFIG_XT if previously added by the configure script.
Fixes:
http://autobuild.buildroot.net/results/3b3/3b37871bdf5766677fc20dca22c13177091d104f/

FYI, I ended up applying Baruch's patch (and then some followup patches of my own). Your two patches were almost identical, but for his patch I didn't have to drop the SED lines, and I was lazy ;)

Thanks!

Signature · 1 line

Bye, Peter Korsgaard

Romain Naour

author

2015-05-23 10:02 UTC #4 lore raw

PATCH 1/1 package/iproute2: disable iptables support for static build

Hi Peter,

Le 21/05/2015 23:22, Peter Korsgaard a ?crit :

Quoted 9 lines

Quoted 8 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Romain" == Romain Naour <[email protected]> writes:
If iptable support is enabled, it will try to build m_xt.so even for static build only.
So, disable iptables support for static build and remove TC_CONFIG_IPSET and TC_CONFIG_XT if previously added by the configure script.

Quoted 6 lines

Quoted 2 lines
Fixes:
http://autobuild.buildroot.net/results/3b3/3b37871bdf5766677fc20dca22c13177091d104f/
FYI, I ended up applying Baruch's patch (and then some followup patches of my own). Your two patches were almost identical, but for his patch I didn't have to drop the SED lines, and I was lazy ;)

Ok, no problem and sorry for the late answer...

The sed invocation to remove -Werror is just a nop and can also be removed. See:
http://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=3140e9a3a343d37a8d828ee27e16ad9f3f9ab389

The real fix would be to write a real autotools configure script but it's probably too late for the Buildroot release...

Best regards,
Romain

Quoted 3 lines

Thanks!

Peter Korsgaard

2015-05-25 20:32 UTC #5 lore raw

PATCH 1/1 package/iproute2: disable iptables support for static build

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Romain" == Romain Naour <[email protected]> writes:

Hi,

Quoted 9 lines

Quoted 3 lines
FYI, I ended up applying Baruch's patch (and then some followup patches of my own). Your two patches were almost identical, but for his patch I didn't have to drop the SED lines, and I was lazy ;)
Ok, no problem and sorry for the late answer...
The sed invocation to remove -Werror is just a nop and can also be removed. See:
http://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=3140e9a3a343d37a8d828ee27e16ad9f3f9ab389

Ahh, thanks - Will remove.

Quoted 5 lines

The real fix would be to write a real autotools configure script but it's probably too late for the Buildroot release...
See also the note in the commit log ;-)
http://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=06ec9039c3aa07924f9c23b0daa8885204704a62

Yeah!

Signature · 2 lines

Venlig hilsen,
Peter Korsgaard