Skip to contentSkip to conversation

iproute: fix tc ipt/xt module.

PATCH In review Andreas Henriksson 3 participants 9 messages Dec 2, 2009 – Dec 26, 2009

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover iproute: fix tc ipt/xt module.

The xtables API has changed since the tc xt module was written. This mini-series moves the legacy stuff out of the way (kept for compatibility with older iptables versions) and starts a new fresh xt module.

Hopefully the xtables api will be kept stable in the future. If so, this new xt module should continue working since it only uses exposed functions (unlike the old one which had an internal header to access internal functions in xtables).

Andreas Henriksson (3):
Keep the old tc/ipt/xt module for compatibility. Add new (iptables 1.4.5 compatible) tc/ipt/xt module. Slightly improve the configure script.

 configure     |   93 ++++++++++--
 tc/Makefile   |   13 ++-
 tc/m_xt.c     |  161 +++++----------------
 tc/m_xt_old.c |  433 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 557 insertions(+), 143 deletions(-)
 create mode 100644 tc/m_xt_old.c
#1 lore raw

PATCH 1/3 Keep the old tc/ipt/xt module for compatibility.

Move the file and rename the configure flags.
The file is being kept around for iptables < 1.4.5 compatibility.

---
 configure     |    4 +-
 tc/Makefile   |   13 ++-
 tc/m_xt.c     |  433 ---------------------------------------------------------
 tc/m_xt_old.c |  433 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 444 insertions(+), 439 deletions(-)
 delete mode 100644 tc/m_xt.c
 create mode 100644 tc/m_xt_old.c
configure +2 -2
index df40370..f3b79b2 100755
@@ -53,7 +53,7 @@ gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl >/dev/null 2>&1
 
 if [ $? -eq 0 ]
 then
-	echo "TC_CONFIG_XT:=y" >>Config
+	echo "TC_CONFIG_XT_OLD:=y" >>Config
 	echo "using xtables seems no need for internal.h"
 else
 	echo "failed test 2"
@@ -86,7 +86,7 @@ gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl >/dev/null 2>&1
 if [ $? -eq 0 ]
 then
 	echo "using xtables instead of iptables (need for internal.h)"
-	echo "TC_CONFIG_XT_H:=y" >>Config
+	echo "TC_CONFIG_XT_OLD_H:=y" >>Config
 
 else
 	echo "failed test 3 using iptables"
tc/Makefile +9 -4
index 027055c..f3dd2b7 100644
@@ -48,12 +48,17 @@ ifeq ($(TC_CONFIG_XT),y)
   TCMODULES += m_xt.o
   LDLIBS += -lxtables
 else
-  ifeq ($(TC_CONFIG_XT_H),y)
+  ifeq ($(TC_CONFIG_XT_OLD),y)
+    TCMODULES += m_xt_old.o
+    LDLIBS += -lxtables
+  else
+    ifeq ($(TC_CONFIG_XT_OLD_H),y)
 	CFLAGS += -DTC_CONFIG_XT_H
-	TCMODULES += m_xt.o
+	TCMODULES += m_xt_old.o
 	LDLIBS += -lxtables
-  else
-  TCMODULES += m_ipt.o
+    else
+      TCMODULES += m_ipt.o
+    endif
   endif
 endif
 
tc/m_xt.c deleted +0 -433

433 lines not on this page · raw message

tc/m_xt_old.c new file +433 -0

433 lines not on this page · raw message

Signature · 1 line
1.6.5.3
#2 lore raw

PATCH 2/3 Add new (iptables 1.4.5 compatible) tc/ipt/xt module.

In reply to Andreas Henriksson

Add a new cleaned up m_xt.c based on m_xt_old.c
The new m_xt.c has been updated to use the new names and new api that xtables exposes in iptables 1.4.5.
All the old internal api cruft has also been dropped.

Additionally, a configure script test is added to check for the new xtables api and set the TC_CONFIG_XT flag in Config. (tc/Makefile already handles this flag in previous commit.)

---
 configure |   27 +++++
 tc/m_xt.c |  346 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 373 insertions(+), 0 deletions(-)
 create mode 100644 tc/m_xt.c
configure +27 -0
index f3b79b2..4fda7cb 100755
@@ -28,6 +28,33 @@ rm -f /tmp/atmtest.c /tmp/atmtest
 
 echo -n " IPT	"
 
+#check if we have xtables from iptables >= 1.4.5.
+cat >/tmp/ipttest.c <<EOF
+#include <xtables.h>
+#include <linux/netfilter.h>
+static struct xtables_globals test_globals = {
+	.option_offset = 0,
+	.program_name = "tc-ipt",
+	.program_version = XTABLES_VERSION,
+	.orig_opts = NULL,
+	.opts = NULL,
+	.exit_err = NULL,
+};
+
+int main(int argc, char **argv)
+{
+	xtables_init_all(&test_globals, NFPROTO_IPV4);
+	return 0;
+}
+
+EOF
+
+if gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl -lxtables >/dev/null 2>&1
+then
+	echo "TC_CONFIG_XT:=y" >>Config
+	echo "using xtables instead of iptables"
+fi
+
 #check if we need dont our internal header ..
 cat >/tmp/ipttest.c <<EOF
 #include <xtables.h>
tc/m_xt.c new file +346 -0

346 lines not on this page · raw message

Signature · 1 line
1.6.5.3
#5 lore raw

PATCH 3/3 Slightly improve the configure script.

In reply to Andreas Henriksson

Split up in functions. Make XT checks bail if previous XT check was successful.

This result improves the output of the configure script to not indicate using iptables only because the last test failed (when previous ones could have already succeded).

---
 configure |   66 +++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 51 insertions(+), 15 deletions(-)
configure +51 -15
index 4fda7cb..a903bb0 100755
@@ -3,11 +3,8 @@
 #
 INCLUDE=${1:-"$PWD/include"}
 
-echo "# Generated config based on" $INCLUDE >Config
-
-echo "TC schedulers"
-
-echo -n " ATM	"
+function check_atm
+{
 cat >/tmp/atmtest.c <<EOF
 #include <atm.h>
 int main(int argc, char **argv) {
@@ -25,9 +22,10 @@ else
     echo no
 fi
 rm -f /tmp/atmtest.c /tmp/atmtest
+}
 
-echo -n " IPT	"
-
+function check_xt
+{
 #check if we have xtables from iptables >= 1.4.5.
 cat >/tmp/ipttest.c <<EOF
 #include <xtables.h>
@@ -52,7 +50,17 @@ EOF
 if gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl -lxtables >/dev/null 2>&1
 then
 	echo "TC_CONFIG_XT:=y" >>Config
-	echo "using xtables instead of iptables"
+	echo "using xtables"
+fi
+rm -f /tmp/ipttest.c /tmp/ipttest
+}
+
+function check_xt_old
+{
+# bail if previous XT checks has already succeded.
+if grep TC_CONFIG_XT Config > /dev/null
+then
+	return
 fi
 
 #check if we need dont our internal header ..
@@ -81,9 +89,17 @@ gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl >/dev/null 2>&1
 if [ $? -eq 0 ]
 then
 	echo "TC_CONFIG_XT_OLD:=y" >>Config
-	echo "using xtables seems no need for internal.h"
-else
-	echo "failed test 2"
+	echo "using old xtables (no need for xt-internal.h)"
+fi
+rm -f /tmp/ipttest.c /tmp/ipttest
+}
+
+function check_xt_old_internal_h
+{
+# bail if previous XT checks has already succeded.
+if grep TC_CONFIG_XT Config > /dev/null
+then
+	return
 fi
 
 #check if we need our own internal.h
@@ -112,10 +128,30 @@ gcc -I$INCLUDE $IPTC -o /tmp/ipttest /tmp/ipttest.c $IPTL -ldl >/dev/null 2>&1
 
 if [ $? -eq 0 ]
 then
-	echo "using xtables instead of iptables (need for internal.h)"
+	echo "using old xtables with xt-internal.h"
 	echo "TC_CONFIG_XT_OLD_H:=y" >>Config
-
-else
-	echo "failed test 3 using iptables"
 fi
 rm -f /tmp/ipttest.c /tmp/ipttest
+}
+
+function check_ipt
+{
+	if ! grep TC_CONFIG_XT Config > /dev/null
+	then
+		echo "using iptables"
+	fi
+}
+
+echo "# Generated config based on" $INCLUDE >Config
+
+echo "TC schedulers"
+
+echo -n " ATM	"
+check_atm
+
+echo -n " IPT	"
+check_xt
+check_xt_old
+check_xt_old_internal_h
+check_ipt
+
Signature · 1 line
1.6.5.3
#3 lore raw

Re: [PATCH 2/3] Add new (iptables 1.4.5 compatible) tc/ipt/xt module.

In reply to Andreas Henriksson
Quoted 10 lines · On Wed, 2 Dec 2009 16:11:50 +0100 Andreas Henriksson <[email protected]> wrote:

Add a new cleaned up m_xt.c based on m_xt_old.c
The new m_xt.c has been updated to use the new names and new api that xtables exposes in iptables 1.4.5.
All the old internal api cruft has also been dropped.

Additionally, a configure script test is added to check for the new xtables api and set the TC_CONFIG_XT flag in Config. (tc/Makefile already handles this flag in previous commit.)

Will this still build on older versions? The iproute source is supposed to build even with really old kernels and older iptables. It has maintained source and binary compatibility.

#4 lore raw

Re: [PATCH 2/3] Add new (iptables 1.4.5 compatible) tc/ipt/xt module.

Quoted 4 lines · Stephen Hemminger
Quoted 1 line · On Wed, 2 Dec 2009 16:11:50 +0100 Andreas Henriksson <[email protected]> wrote:

Add a new cleaned up m_xt.c based on m_xt_old.c

[..]

Quoted 4 lines

Will this still build on older versions? The iproute source is supposed to build even with really old kernels and older iptables. It has maintained source and binary compatibility.

Which of tc/m_ipt.c, tc/m_xt_old.c and tc/m_xt.c will be built depends on which version of iptables you have installed.
The configure script sets matching flags in Config and tc/Makefile picks that up and includes the suitable source file.

So tc/m_xt.c will not build with older iptables and it won't be used with older iptables.

This is not ideal, since if the xtables headers can't be found (ie. the iptables-dev package isn't installed) the ancient m_ipt.c will be built and the brokenness won't be detected until the user tries to use the module.

Distributors will need to watch out for this. It's currently broken in atleast fedora and debian.

Signature · 2 lines
Regards,
Andreas Henriksson
#6 lore raw
In reply to Andreas Henriksson
Quoted 10 lines · Andreas Henriksson

The xtables API has changed since the tc xt module was written. This mini-series moves the legacy stuff out of the way (kept for compatibility with older iptables versions) and starts a new fresh xt module.

Hopefully the xtables api will be kept stable in the future. If so, this new xt module should continue working since it only uses exposed functions (unlike the old one which had an internal header to access internal functions in xtables).

Quoted 4 lines

Andreas Henriksson (3):
Keep the old tc/ipt/xt module for compatibility. Add new (iptables 1.4.5 compatible) tc/ipt/xt module. Slightly improve the configure script.

Andreas, in the future can you CC me please - I dont read netdev in real time (sometimes i catch up in a month and skim over).

cheers,
jamal

#7 lore raw
Quoted 4 lines · jamal

Andreas, in the future can you CC me please - I dont read netdev in real time (sometimes i catch up in a month and skim over).

Yes, I'm sorry, ofcourse I should have!
(I was busy fighting the git patch formatting and submission tools and forgot to figure out how to add multiple CCs.)

Signature · 1 line
Andreas Henriksson
#8 lore raw
In reply to jamal
Quoted 17 lines · On Thu, 03 Dec 2009 08:09:38 -0500 jamal <[email protected]> wrote:
Quoted 10 lines · Andreas Henriksson

The xtables API has changed since the tc xt module was written. This mini-series moves the legacy stuff out of the way (kept for compatibility with older iptables versions) and starts a new fresh xt module.

Hopefully the xtables api will be kept stable in the future. If so, this new xt module should continue working since it only uses exposed functions (unlike the old one which had an internal header to access internal functions in xtables).

Quoted 4 lines

Andreas Henriksson (3):
Keep the old tc/ipt/xt module for compatibility. Add new (iptables 1.4.5 compatible) tc/ipt/xt module. Slightly improve the configure script.

Quoted 8 lines

Andreas, in the future can you CC me please - I dont read netdev in real time (sometimes i catch up in a month and skim over).

cheers,
jamal

These all went int to the 2.6.32 version of iproute2