* [Buildroot] [PATCH] package/iproute2: disable iptables support for static build @ 2015-05-18 20:54 Romain Naour 2015-05-19 19:33 ` Peter Korsgaard 2015-05-21 21:22 ` Peter Korsgaard 0 siblings, 2 replies; 6+ messages in thread From: Romain Naour @ 2015-05-18 20:54 UTC (permalink / raw) To: buildroot 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 <romain.naour@openwide.fr> --- package/iproute2/iproute2.mk | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/package/iproute2/iproute2.mk b/package/iproute2/iproute2.mk index 9595165..d97e75e 100644 --- a/package/iproute2/iproute2.mk +++ b/package/iproute2/iproute2.mk @@ -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 -- 1.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] package/iproute2: disable iptables support for static build 2015-05-18 20:54 [Buildroot] [PATCH] package/iproute2: disable iptables support for static build Romain Naour @ 2015-05-19 19:33 ` Peter Korsgaard 2015-05-20 3:57 ` Gustavo Zacarias 2015-05-21 21:22 ` Peter Korsgaard 1 sibling, 1 reply; 6+ messages in thread From: Peter Korsgaard @ 2015-05-19 19:33 UTC (permalink / raw) To: buildroot >>>>> "Romain" == Romain Naour <romain.naour@openwide.fr> 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: --- output/build/iproute2-4.0.0/Config.without 2015-05-19 21:03:14.735183540 +0200 +++ output/build/iproute2-4.0.0/Config.with 2015-05-19 21:03:39.835184550 +0200 @@ -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? > Fixes: > http://autobuild.buildroot.net/results/3b3/3b37871bdf5766677fc20dca22c13177091d104f/ > Signed-off-by: Romain Naour <romain.naour@openwide.fr> > --- > package/iproute2/iproute2.mk | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > diff --git a/package/iproute2/iproute2.mk b/package/iproute2/iproute2.mk > index 9595165..d97e75e 100644 > --- a/package/iproute2/iproute2.mk > +++ b/package/iproute2/iproute2.mk > @@ -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 > -- > 1.9.3 > _______________________________________________ > buildroot mailing list > buildroot at busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot -- Bye, Peter Korsgaard ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] package/iproute2: disable iptables support for static build 2015-05-19 19:33 ` Peter Korsgaard @ 2015-05-20 3:57 ` Gustavo Zacarias 0 siblings, 0 replies; 6+ messages in thread From: Gustavo Zacarias @ 2015-05-20 3:57 UTC (permalink / raw) To: buildroot On 19/05/15 12:33, Peter Korsgaard wrote: > 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] package/iproute2: disable iptables support for static build 2015-05-18 20:54 [Buildroot] [PATCH] package/iproute2: disable iptables support for static build Romain Naour 2015-05-19 19:33 ` Peter Korsgaard @ 2015-05-21 21:22 ` Peter Korsgaard 2015-05-23 10:02 ` Romain Naour 1 sibling, 1 reply; 6+ messages in thread From: Peter Korsgaard @ 2015-05-21 21:22 UTC (permalink / raw) To: buildroot >>>>> "Romain" == Romain Naour <romain.naour@openwide.fr> 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! -- Bye, Peter Korsgaard ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] package/iproute2: disable iptables support for static build 2015-05-21 21:22 ` Peter Korsgaard @ 2015-05-23 10:02 ` Romain Naour 2015-05-25 20:32 ` Peter Korsgaard 0 siblings, 1 reply; 6+ messages in thread From: Romain Naour @ 2015-05-23 10:02 UTC (permalink / raw) To: buildroot Hi Peter, Le 21/05/2015 23:22, Peter Korsgaard a ?crit : >>>>>> "Romain" == Romain Naour <romain.naour@openwide.fr> 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 ;) 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... See also the note in the commit log ;-) http://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=06ec9039c3aa07924f9c23b0daa8885204704a62 Best regards, Romain > > Thanks! > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] package/iproute2: disable iptables support for static build 2015-05-23 10:02 ` Romain Naour @ 2015-05-25 20:32 ` Peter Korsgaard 0 siblings, 0 replies; 6+ messages in thread From: Peter Korsgaard @ 2015-05-25 20:32 UTC (permalink / raw) To: buildroot >>>>> "Romain" == Romain Naour <romain.naour@openwide.fr> writes: Hi, >> 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. > 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! -- Venlig hilsen, Peter Korsgaard ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-05-25 20:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-18 20:54 [Buildroot] [PATCH] package/iproute2: disable iptables support for static build Romain Naour 2015-05-19 19:33 ` Peter Korsgaard 2015-05-20 3:57 ` Gustavo Zacarias 2015-05-21 21:22 ` Peter Korsgaard 2015-05-23 10:02 ` Romain Naour 2015-05-25 20:32 ` Peter Korsgaard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox