[PATCH] iproute2: add option to build m

Netdev List
 help / color / mirror / Atom feed

* [PATCH] iproute2: add option to build m_xt as a tc module.
@ 2010-04-12 11:55 Andreas Henriksson
  2010-04-12 15:33 ` Stephen Hemminger
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Henriksson @ 2010-04-12 11:55 UTC (permalink / raw)
  To: shemminger; +Cc: netdev

Add TC_CONFIG_XT_MODULE option that can be added
either to Config (after ./configure) or as an argument to "make".

This will build the xt module (action ipt) of tc as a
shared object that is linked at runtime by tc if used,
rather then built into tc.

This is similar to how the atm qdisc support
is handled (q_atm.so).

Signed-off-by: Andreas Henriksson <andreas@fatal.se>

---

The reason for this is simply to be able to avoid
the tc binary from being linked to libxtables. This way
distributions who ship binary packages can
avoid a dependency on the iptables package by
ignoring m_xt.so in the dependency analysis
and let actual users of the tc arguments "action ipt"
make sure they have iptables installed.
(See http://bugs.debian.org/576953 )

This was not a problem with the old/deprecated
m_ipt module which did runtime linking of
the iptables library.

Having the split inside tc, rather then between tc and the required
library, is preferred. This way we'll notice at build-time
when the required library breaks API/ABI rather
then having to rely on people that uses the functionality
to report back when the ABI is broken.
(We've learned this the hard way in debian after many
angry bugreports.)

I've had jamal pre-review this and he didn't see any
problems with this.

diff --git a/tc/Makefile b/tc/Makefile
index 805c108..3af33cf 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -43,10 +43,18 @@ TCMODULES += em_cmp.o
 TCMODULES += em_u32.o
 TCMODULES += em_meta.o

+TCSO :=
+ifeq ($(TC_CONFIG_ATM),y)
+  TCSO += q_atm.so
+endif

 ifeq ($(TC_CONFIG_XT),y)
-  TCMODULES += m_xt.o
-  LDLIBS += -lxtables
+  ifeq ($(TC_CONFIG_XT_MODULE),y)
+    TCSO += m_xt.so
+  else
+    TCMODULES += m_xt.o
+    LDLIBS += -lxtables
+  endif
 else
   ifeq ($(TC_CONFIG_XT_OLD),y)
     TCMODULES += m_xt_old.o
@@ -81,11 +89,6 @@ ifneq ($(IPT_LIB_DIR),)
 	CFLAGS += -DIPT_LIB_DIR=\"$(IPT_LIB_DIR)\"
 endif

-TCSO :=
-ifeq ($(TC_CONFIG_ATM),y)
-  TCSO += q_atm.so
-endif
-
 YACC := bison
 LEX := flex

@@ -114,6 +117,9 @@ clean:
 q_atm.so: q_atm.c
 	$(CC) $(CFLAGS) $(LDFLAGS) -shared -fpic -o q_atm.so q_atm.c -latm

+m_xt.so: m_xt.c
+	$(CC) $(CFLAGS) $(LDFLAGS) -shared -fpic -o m_xt.so m_xt.c -lxtables
+
 %.yacc.c: %.y
 	$(YACC) $(YACCFLAGS) -o $@ $<

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] iproute2: add option to build m_xt as a tc module.
  2010-04-12 11:55 [PATCH] iproute2: add option to build m_xt as a tc module Andreas Henriksson
@ 2010-04-12 15:33 ` Stephen Hemminger
  2010-04-12 18:13   ` Andreas Henriksson
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2010-04-12 15:33 UTC (permalink / raw)
  To: Andreas Henriksson; +Cc: netdev

On Mon, 12 Apr 2010 13:55:38 +0200
Andreas Henriksson <andreas@fatal.se> wrote:

> Add TC_CONFIG_XT_MODULE option that can be added
> either to Config (after ./configure) or as an argument to "make".

I like the idea and will incorporate it, but do not like having more
build options. Adding more configuration options like this is just
lazy design "we can't figure this out, let's make the user do it".

So put the patch in but there it will always be true.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iproute2: add option to build m_xt as a tc module.
  2010-04-12 15:33 ` Stephen Hemminger
@ 2010-04-12 18:13   ` Andreas Henriksson
  2010-04-12 18:19     ` [PATCH] iproute2: add option to build m_xt as a tc module (v2) Andreas Henriksson
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Henriksson @ 2010-04-12 18:13 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

On Mon, Apr 12, 2010 at 08:33:17AM -0700, Stephen Hemminger wrote:
> On Mon, 12 Apr 2010 13:55:38 +0200
> Andreas Henriksson <andreas@fatal.se> wrote:
> 
> > Add TC_CONFIG_XT_MODULE option that can be added
> > either to Config (after ./configure) or as an argument to "make".
> 
> I like the idea and will incorporate it, but do not like having more
> build options. Adding more configuration options like this is just
> lazy design "we can't figure this out, let's make the user do it".
> 
> So put the patch in but there it will always be true.

Sure..... unfortunately I found problems for the patch to work.
(dlopen needs full path and module name needs to match action name)
Will send new patch as a followup to this mail.

Note: I'm not a user of any of this functionality. (including "make install"!)
I've tried testing "action ipt" with m_xt.so now, but m_xt_old.so is
completely untested!

I used the following command when testing m_xt.so:
tc qdisc add dev lo ingress
tc filter add dev lo parent ffff: protocol ip prio 1 u32  match ip src 127.1.1.1/32 action ipt -j FOOBAR

...which gave me the error "failed to find target FOOBAR",
and this is to me an indication of success since I don't have that target.

-- 
Andreas Henriksson

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] iproute2: add option to build m_xt as a tc module (v2)
  2010-04-12 18:13   ` Andreas Henriksson
@ 2010-04-12 18:19     ` Andreas Henriksson
  2010-04-12 18:24       ` [PATCH] iproute2: add option to build m_xt as a tc module (v3) Andreas Henriksson
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Henriksson @ 2010-04-12 18:19 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

This will build the xt module (action ipt) of tc as a
shared object that is linked at runtime by tc if used,
rather then built into tc.

This is similar to how the atm qdisc support
is handled (q_atm.so).

Signed-off-by: Andreas Henriksson <andreas@xxxxxxxx>

---

v2:
Updated to not be a configurable option, always
build as module.
Also build m_xt_old as module (untested!).
Give dlopen full path to modules rather then just filename.
Fix make install module destination directory problems (untested!).


diff --git a/tc/Makefile b/tc/Makefile
index 805c108..23dbca8 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -43,19 +43,20 @@ TCMODULES += em_cmp.o
 TCMODULES += em_u32.o
 TCMODULES += em_meta.o
 
+TCSO :=
+ifeq ($(TC_CONFIG_ATM),y)
+  TCSO += q_atm.so
+endif
 
 ifeq ($(TC_CONFIG_XT),y)
-  TCMODULES += m_xt.o
-  LDLIBS += -lxtables
+  TCSO += m_xt.so
 else
   ifeq ($(TC_CONFIG_XT_OLD),y)
-    TCMODULES += m_xt_old.o
-    LDLIBS += -lxtables
+    TCSO += m_xt_old.so
   else
     ifeq ($(TC_CONFIG_XT_OLD_H),y)
 	CFLAGS += -DTC_CONFIG_XT_H
-	TCMODULES += m_xt_old.o
-	LDLIBS += -lxtables
+	TCSO += m_xt_old.so
     else
       TCMODULES += m_ipt.o
     endif
@@ -81,14 +82,11 @@ ifneq ($(IPT_LIB_DIR),)
 	CFLAGS += -DIPT_LIB_DIR=\"$(IPT_LIB_DIR)\"
 endif
 
-TCSO :=
-ifeq ($(TC_CONFIG_ATM),y)
-  TCSO += q_atm.so
-endif
-
 YACC := bison
 LEX := flex
 
+MODDESTDIR := $(DESTDIR)$(patsubst /usr%,%,$(LIBDIR))/tc
+
 %.so: %.c
 	$(CC) $(CFLAGS) $(LDFLAGS) -shared -fpic $< -o $@
 
@@ -101,11 +99,18 @@ libtc.a: $(TCLIB)
 	$(AR) rcs $@ $(TCLIB)
 
 install: all
-	mkdir -p $(DESTDIR)$(LIBDIR)/tc
-	install -m 0755 tc $(DESTDIR)$(SBINDIR)
+	echo mkdir -p $(MODDESTDIR)
+	echo install -m 0755 tc $(DESTDIR)$(SBINDIR)
 	for i in $(TCSO); \
-	do install -m 755 $$i $(DESTDIR)$(LIBDIR)/tc; \
+	do echo install -m 755 $$i $(MODDESTDIR); \
 	done
+	if [ ! -f $(MODDESTDIR)/m_ipt.so ]; then \
+	if [ -f $(MODDESTDIR)/m_xt.so ]; \
+		then ln -s m_xt.so $(MODDESTDIR)/m_ipt.so ; \
+	elif [ -f $(MODDESTDIR)/m_xt_old.so ]; \
+		then ln -s m_xt_old.so $(MODDESTDIR)/m_ipt.so ; \
+	fi; \
+	fi
 
 clean:
 	rm -f $(TCOBJ) $(TCLIB) libtc.a tc *.so emp_ematch.yacc.h; \
@@ -114,6 +119,12 @@ clean:
 q_atm.so: q_atm.c
 	$(CC) $(CFLAGS) $(LDFLAGS) -shared -fpic -o q_atm.so q_atm.c -latm
 
+m_xt.so: m_xt.c
+	$(CC) $(CFLAGS) $(LDFLAGS) -shared -fpic -o m_xt.so m_xt.c -lxtables
+
+m_xt_old.so: m_xt_old.c
+	$(CC) $(CFLAGS) $(LDFLAGS) -shared -fpic -o m_xt.so m_xt.c -lxtables
+
 %.yacc.c: %.y
 	$(YACC) $(YACCFLAGS) -o $@ $<
 
diff --git a/tc/m_action.c b/tc/m_action.c
index 9f24022..a198158 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -98,7 +98,7 @@ restart_s:
 			return a;
 	}
 
-	snprintf(buf, sizeof(buf), "m_%s.so", str);
+	snprintf(buf, sizeof(buf), "%s/m_%s.so", get_tc_lib(), str);
 	dlh = dlopen(buf, RTLD_LAZY);
 	if (dlh == NULL) {
 		dlh = aBODY;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] iproute2: add option to build m_xt as a tc module (v3)
  2010-04-12 18:19     ` [PATCH] iproute2: add option to build m_xt as a tc module (v2) Andreas Henriksson
@ 2010-04-12 18:24       ` Andreas Henriksson
  2010-04-12 18:41         ` Stephen Hemminger
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Henriksson @ 2010-04-12 18:24 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

This will build the xt module (action ipt) of tc as a
shared object that is linked at runtime by tc if used,
rather then built into tc.

This is similar to how the atm qdisc support
is handled (q_atm.so).

Signed-off-by: Andreas Henriksson <andreas@xxxxxxxx>

---

v3:
fix cut'n'paste problem in m_xt_old.so make rule. SORRY!

v2:
Updated to not be a configurable option, always
build as module.
Also build m_xt_old as module (untested!).
Give dlopen full path to modules rather then just filename.
Fix make install module destination directory problems (untested!).


diff --git a/tc/Makefile b/tc/Makefile
index 805c108..01a16fc 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -43,19 +43,20 @@ TCMODULES += em_cmp.o
 TCMODULES += em_u32.o
 TCMODULES += em_meta.o
 
+TCSO :=
+ifeq ($(TC_CONFIG_ATM),y)
+  TCSO += q_atm.so
+endif
 
 ifeq ($(TC_CONFIG_XT),y)
-  TCMODULES += m_xt.o
-  LDLIBS += -lxtables
+  TCSO += m_xt.so
 else
   ifeq ($(TC_CONFIG_XT_OLD),y)
-    TCMODULES += m_xt_old.o
-    LDLIBS += -lxtables
+    TCSO += m_xt_old.so
   else
     ifeq ($(TC_CONFIG_XT_OLD_H),y)
 	CFLAGS += -DTC_CONFIG_XT_H
-	TCMODULES += m_xt_old.o
-	LDLIBS += -lxtables
+	TCSO += m_xt_old.so
     else
       TCMODULES += m_ipt.o
     endif
@@ -81,14 +82,11 @@ ifneq ($(IPT_LIB_DIR),)
 	CFLAGS += -DIPT_LIB_DIR=\"$(IPT_LIB_DIR)\"
 endif
 
-TCSO :=
-ifeq ($(TC_CONFIG_ATM),y)
-  TCSO += q_atm.so
-endif
-
 YACC := bison
 LEX := flex
 
+MODDESTDIR := $(DESTDIR)$(patsubst /usr%,%,$(LIBDIR))/tc
+
 %.so: %.c
 	$(CC) $(CFLAGS) $(LDFLAGS) -shared -fpic $< -o $@
 
@@ -101,11 +99,18 @@ libtc.a: $(TCLIB)
 	$(AR) rcs $@ $(TCLIB)
 
 install: all
-	mkdir -p $(DESTDIR)$(LIBDIR)/tc
-	install -m 0755 tc $(DESTDIR)$(SBINDIR)
+	echo mkdir -p $(MODDESTDIR)
+	echo install -m 0755 tc $(DESTDIR)$(SBINDIR)
 	for i in $(TCSO); \
-	do install -m 755 $$i $(DESTDIR)$(LIBDIR)/tc; \
+	do echo install -m 755 $$i $(MODDESTDIR); \
 	done
+	if [ ! -f $(MODDESTDIR)/m_ipt.so ]; then \
+	if [ -f $(MODDESTDIR)/m_xt.so ]; \
+		then ln -s m_xt.so $(MODDESTDIR)/m_ipt.so ; \
+	elif [ -f $(MODDESTDIR)/m_xt_old.so ]; \
+		then ln -s m_xt_old.so $(MODDESTDIR)/m_ipt.so ; \
+	fi; \
+	fi
 
 clean:
 	rm -f $(TCOBJ) $(TCLIB) libtc.a tc *.so emp_ematch.yacc.h; \
@@ -114,6 +119,12 @@ clean:
 q_atm.so: q_atm.c
 	$(CC) $(CFLAGS) $(LDFLAGS) -shared -fpic -o q_atm.so q_atm.c -latm
 
+m_xt.so: m_xt.c
+	$(CC) $(CFLAGS) $(LDFLAGS) -shared -fpic -o m_xt.so m_xt.c -lxtables
+
+m_xt_old.so: m_xt_old.c
+	$(CC) $(CFLAGS) $(LDFLAGS) -shared -fpic -o m_xt_old.so m_xt_old.c -lxtables
+
 %.yacc.c: %.y
 	$(YACC) $(YACCFLAGS) -o $@ $<
 
diff --git a/tc/m_action.c b/tc/m_action.c
index 9f24022..a198158 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -98,7 +98,7 @@ restart_s:
 			return a;
 	}
 
-	snprintf(buf, sizeof(buf), "m_%s.so", str);
+	snprintf(buf, sizeof(buf), "%s/m_%s.so", get_tc_lib(), str);
 	dlh = dlopen(buf, RTLD_LAZY);
 	if (dlh == NULL) {
 		dlh = aBODY;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] iproute2: add option to build m_xt as a tc module (v3)
  2010-04-12 18:24       ` [PATCH] iproute2: add option to build m_xt as a tc module (v3) Andreas Henriksson
@ 2010-04-12 18:41         ` Stephen Hemminger
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2010-04-12 18:41 UTC (permalink / raw)
  To: Andreas Henriksson; +Cc: netdev

On Mon, 12 Apr 2010 20:24:23 +0200
Andreas Henriksson <andreas@fatal.se> wrote:

> This will build the xt module (action ipt) of tc as a
> shared object that is linked at runtime by tc if used,
> rather then built into tc.
> 
> This is similar to how the atm qdisc support
> is handled (q_atm.so).
> 
> Signed-off-by: Andreas Henriksson <andreas@xxxxxxxx>

Applied thanks.

-- 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-04-12 18:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-12 11:55 [PATCH] iproute2: add option to build m_xt as a tc module Andreas Henriksson
2010-04-12 15:33 ` Stephen Hemminger
2010-04-12 18:13   ` Andreas Henriksson
2010-04-12 18:19     ` [PATCH] iproute2: add option to build m_xt as a tc module (v2) Andreas Henriksson
2010-04-12 18:24       ` [PATCH] iproute2: add option to build m_xt as a tc module (v3) Andreas Henriksson
2010-04-12 18:41         ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox