tc ipt action

29 messages, 6 authors, 2012-12-20 · open the first message on its own page

tc ipt action

From: Yury Stankevich <hidden>
Date: 2012-12-09 12:20:51

Hello,

i not sure this is correct list, please advise if not.

i'm trying to use ipt action, and got a problem:

#tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action ipt -j CONNMARK --restore-mark action mirred egress redirect dev ifb0
-> bad action type ipt

from strace:
open("/usr/lib/tc//m_gact.so", O_RDONLY) = -1 ENOENT (No such file or
directory)
write(2, "bad action type ipt\n", 20bad action type ipt

well. i'm trying to use xt:
#tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action xt -j CONNMARK --restore-mark action mirred egress redirect dev ifb0
xt: unrecognized option '--restore-mark'

from strace:
open("/lib/xtables/libxt_CONNMARK.so", O_RDONLY) = 4
read(4,
"\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\200\6\0\0004\0\0\0"...,
512) = 512
fstat64(4, {st_mode=S_IFREG|0644, st_size=9756, ...}) = 0
mmap2(NULL, 12548, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 4, 0)
= 0xf76f3000
mmap2(0xf76f5000, 8192, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 4, 0x1) = 0xf76f5000
close(4)                                = 0
mprotect(0xf76f5000, 4096, PROT_READ)   = 0
socket(PF_INET, SOCK_RAW, IPPROTO_RAW)  = 4
fcntl64(4, F_SETFD, FD_CLOEXEC)         = 0
lstat64("/proc/net/ip_tables_names", {st_mode=S_IFREG|0440, st_size=0,
...}) = 0
statfs64("/proc/net/ip_tables_names", 84, {f_type="PROC_SUPER_MAGIC",
f_bsize=4096, f_blocks=0, f_bfree=0, f_bavail=0, f_files=0, f_ffree=0,
f_fsid={0, 0}, f_namelen=255, f_frsize=4096}) = 0
getsockopt(4, SOL_IP, 0x43 /* IP_??? */,
"CONNMARK\0\367\f\300\0\0\0po\367l8p\367\364/p\367:}\302\1", [30]) = 0
close(4)                                = 0
write(2, "xt: unrecognized option '--resto"..., 41xt: unrecognized
option '--restore-mark'

so... i make something wrong or this is a bug ?

ps: 3.6.8 kernel 64 bit kernel with 32 bit userspace, iproute 20121001
from debian-experimental,
module act_ipt is loaded.
pps: please, cc me in reply.


-- 
Linux registered user #402966 // pub 1024D/E99AF373 <pgp.mit.edu>

Re: tc ipt action

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-13 10:58:07

Yury,

This appears to be an ABI breakage on iptables/netfilter side.
I will look at it (and hopefully fix it) over the weekend.

cheers,
jamal

On 12-12-09 07:20 AM, Yury Stankevich wrote:
Hello,

i not sure this is correct list, please advise if not.

i'm trying to use ipt action, and got a problem:

#tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action ipt -j CONNMARK --restore-mark action mirred egress redirect dev ifb0
-> bad action type ipt

from strace:
open("/usr/lib/tc//m_gact.so", O_RDONLY) = -1 ENOENT (No such file or
directory)
write(2, "bad action type ipt\n", 20bad action type ipt

well. i'm trying to use xt:
#tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action xt -j CONNMARK --restore-mark action mirred egress redirect dev ifb0
xt: unrecognized option '--restore-mark'

from strace:
open("/lib/xtables/libxt_CONNMARK.so", O_RDONLY) = 4
read(4,
"\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\200\6\0\0004\0\0\0"...,
512) = 512
fstat64(4, {st_mode=S_IFREG|0644, st_size=9756, ...}) = 0
mmap2(NULL, 12548, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 4, 0)
= 0xf76f3000
mmap2(0xf76f5000, 8192, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 4, 0x1) = 0xf76f5000
close(4)                                = 0
mprotect(0xf76f5000, 4096, PROT_READ)   = 0
socket(PF_INET, SOCK_RAW, IPPROTO_RAW)  = 4
fcntl64(4, F_SETFD, FD_CLOEXEC)         = 0
lstat64("/proc/net/ip_tables_names", {st_mode=S_IFREG|0440, st_size=0,
...}) = 0
statfs64("/proc/net/ip_tables_names", 84, {f_type="PROC_SUPER_MAGIC",
f_bsize=4096, f_blocks=0, f_bfree=0, f_bavail=0, f_files=0, f_ffree=0,
f_fsid={0, 0}, f_namelen=255, f_frsize=4096}) = 0
getsockopt(4, SOL_IP, 0x43 /* IP_??? */,
"CONNMARK\0\367\f\300\0\0\0po\367l8p\367\364/p\367:}\302\1", [30]) = 0
close(4)                                = 0
write(2, "xt: unrecognized option '--resto"..., 41xt: unrecognized
option '--restore-mark'

so... i make something wrong or this is a bug ?

ps: 3.6.8 kernel 64 bit kernel with 32 bit userspace, iproute 20121001
from debian-experimental,
module act_ipt is loaded.
pps: please, cc me in reply.

Re: tc ipt action

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-15 21:19:32

Yury,

I took a brief look and run some quick tests on ubuntu 12.04. I am going
to be lazy and try and involve the netfilter folks.
It seems that if you left out the args to CONNMARK (includes other 
targets like MARK etc) you will succeed - but you get default values.


Example, the following should work for
tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action ipt -j CONNMARK \
action mirred egress redirect dev ifb0

Here is what the output looks like when you dont pass the parameters.

-------
j@ubuntu:~$ sudo tc filter show dev eth0 parent ffff:
filter protocol ip pref 1 u32
filter protocol ip pref 1 u32 fh 800: ht divisor 1
filter protocol ip pref 1 u32 fh 800::800 order 2048 key ht 800 bkt 0 
flowid 1:15
   match 0a000015/ffffffff at 12
	action order 1: tablename: mangle  hook: NF_IP_PRE_ROUTING
	target  MARK and 0xffffffff
	index 2 ref 1 bind 1

filter protocol ip pref 49149 u32
filter protocol ip pref 49149 u32 fh 804: ht divisor 1
filter protocol ip pref 49149 u32 fh 804::800 order 2048 key ht 804 bkt 
0 flowid 1:12
   match 00000000/00000000 at 0
	action order 33: tablename: mangle  hook: NF_IP_PRE_ROUTING
	target  CONNMARK and 0x0
	index 123 ref 1 bind 1
----------------

Pablo, Hasan Chowdhury tells me this broke after iptable 1.4.10
Hasan also sent me a small patch to fake "xt" instead of "ipt" - but i 
think there's more than meets the eye here; some interface we are using 
to talk to xtables on user space seems to have changed.

cheers,
jamal

On 12-12-13 05:58 AM, Jamal Hadi Salim wrote:
Yury,

This appears to be an ABI breakage on iptables/netfilter side.
I will look at it (and hopefully fix it) over the weekend.

cheers,
jamal

On 12-12-09 07:20 AM, Yury Stankevich wrote:
quoted
Hello,

i not sure this is correct list, please advise if not.

i'm trying to use ipt action, and got a problem:

#tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action ipt -j CONNMARK --restore-mark action mirred egress redirect
dev ifb0
-> bad action type ipt

from strace:
open("/usr/lib/tc//m_gact.so", O_RDONLY) = -1 ENOENT (No such file or
directory)
write(2, "bad action type ipt\n", 20bad action type ipt

well. i'm trying to use xt:
#tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action xt -j CONNMARK --restore-mark action mirred egress redirect dev
ifb0
xt: unrecognized option '--restore-mark'

from strace:
open("/lib/xtables/libxt_CONNMARK.so", O_RDONLY) = 4
read(4,
"\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\200\6\0\0004\0\0\0"...,
512) = 512
fstat64(4, {st_mode=S_IFREG|0644, st_size=9756, ...}) = 0
mmap2(NULL, 12548, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 4, 0)
= 0xf76f3000
mmap2(0xf76f5000, 8192, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 4, 0x1) = 0xf76f5000
close(4)                                = 0
mprotect(0xf76f5000, 4096, PROT_READ)   = 0
socket(PF_INET, SOCK_RAW, IPPROTO_RAW)  = 4
fcntl64(4, F_SETFD, FD_CLOEXEC)         = 0
lstat64("/proc/net/ip_tables_names", {st_mode=S_IFREG|0440, st_size=0,
...}) = 0
statfs64("/proc/net/ip_tables_names", 84, {f_type="PROC_SUPER_MAGIC",
f_bsize=4096, f_blocks=0, f_bfree=0, f_bavail=0, f_files=0, f_ffree=0,
f_fsid={0, 0}, f_namelen=255, f_frsize=4096}) = 0
getsockopt(4, SOL_IP, 0x43 /* IP_??? */,
"CONNMARK\0\367\f\300\0\0\0po\367l8p\367\364/p\367:}\302\1", [30]) = 0
close(4)                                = 0
write(2, "xt: unrecognized option '--resto"..., 41xt: unrecognized
option '--restore-mark'

so... i make something wrong or this is a bug ?

ps: 3.6.8 kernel 64 bit kernel with 32 bit userspace, iproute 20121001
from debian-experimental,
module act_ipt is loaded.
pps: please, cc me in reply.

Re: tc ipt action

From: Jan Engelhardt <hidden>
Date: 2012-12-15 23:06:40

On Saturday 2012-12-15 22:19, Jamal Hadi Salim wrote:
Example, the following should work for
tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action ipt -j CONNMARK \
action mirred egress redirect dev ifb0
If I try that command (substituting ipt->xt and eth0->dummy0,
ifb0->dummy1), all I get is the dreaded "Invalid argument".
So the kernel rejected the command, which could indicate that
userspace construction might have been ok.

# tc filter add dev dummy0 parent ffff: protocol ip u32 match u32 0 0 \
action xt -j CONNMARK action mirred egress redirect dev dummy1

tablename: mangle hook: NF_IP_PRE_ROUTING
        target:  CONNMARK and 0x0 index 0
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
Pablo, Hasan Chowdhury tells me this broke after iptable 1.4.10
Hasan also sent me a small patch to fake "xt" instead of "ipt" - but i think
there's more than meets the eye here; some interface we are using to talk to
xtables on user space seems to have changed.
What was the last combination that worked?

Re: tc ipt action

From: Jan Engelhardt <hidden>
Date: 2012-12-16 00:26:32

On Sunday 2012-12-16 00:06, Jan Engelhardt wrote:
On Saturday 2012-12-15 22:19, Jamal Hadi Salim wrote:
quoted
Example, the following should work for
tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action ipt -j CONNMARK \
action mirred egress redirect dev ifb0
If I try that command (substituting ipt->xt and eth0->dummy0,
ifb0->dummy1), all I get is the dreaded "Invalid argument".
So the kernel rejected the command, which could indicate that
userspace construction might have been ok.

# tc filter add dev dummy0 parent ffff: protocol ip u32 match u32 0 0 \
action xt -j CONNMARK action mirred egress redirect dev dummy1

tablename: mangle hook: NF_IP_PRE_ROUTING
       target:  CONNMARK and 0x0 index 0
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
quoted
Pablo, Hasan Chowdhury tells me this broke after iptable 1.4.10
Hasan also sent me a small patch to fake "xt" instead of "ipt" - but i think
there's more than meets the eye here; some interface we are using to talk to
xtables on user space seems to have changed.
What was the last combination that worked?
For added fun, it works even less in iproute2-3.7.0.

commit e4fc4ada3317ea94452576add25981279d705b14
Author: Mike Frysinger [off-list ref]
Date:   Thu Nov 8 11:41:17 2012 -0500

    allow pkg-config to be customized
    
    Rather than hard coding `pkg-config`, use ${PKG_CONFIG} so people can
    override it to their specific version (like when cross-compiling).
    
    This is the same way the upstream pkg-config code works.
    
    Signed-off-by: Mike Frysinger [off-list ref]


broke it by causing tc/m_xt.so to no longer link against libxtables.so,
leading to:

# tc [above parameters]
tc: symbol lookup error: /usr/lib64/tc//m_xt.so: undefined symbol:
xtables_init_all


(Makefiles being simpler than $other_buildsys? A distant reality!)

Re: tc ipt action

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: 2012-12-16 00:28:00

Hi Jamal!

On Sat, Dec 15, 2012 at 04:19:29PM -0500, Jamal Hadi Salim wrote:
Yury,

I took a brief look and run some quick tests on ubuntu 12.04. I am going
to be lazy and try and involve the netfilter folks.
It seems that if you left out the args to CONNMARK (includes other
targets like MARK etc) you will succeed - but you get default
values.


Example, the following should work for
tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action ipt -j CONNMARK \
action mirred egress redirect dev ifb0

Here is what the output looks like when you dont pass the parameters.

-------
j@ubuntu:~$ sudo tc filter show dev eth0 parent ffff:
filter protocol ip pref 1 u32
filter protocol ip pref 1 u32 fh 800: ht divisor 1
filter protocol ip pref 1 u32 fh 800::800 order 2048 key ht 800 bkt
0 flowid 1:15
  match 0a000015/ffffffff at 12
	action order 1: tablename: mangle  hook: NF_IP_PRE_ROUTING
	target  MARK and 0xffffffff
	index 2 ref 1 bind 1

filter protocol ip pref 49149 u32
filter protocol ip pref 49149 u32 fh 804: ht divisor 1
filter protocol ip pref 49149 u32 fh 804::800 order 2048 key ht 804
bkt 0 flowid 1:12
  match 00000000/00000000 at 0
	action order 33: tablename: mangle  hook: NF_IP_PRE_ROUTING
	target  CONNMARK and 0x0
	index 123 ref 1 bind 1
----------------

Pablo, Hasan Chowdhury tells me this broke after iptable 1.4.10
Hasan also sent me a small patch to fake "xt" instead of "ipt" - but
i think there's more than meets the eye here; some interface we are
using to talk to xtables on user space seems to have changed.
The binary interface was broken in 1.4.11 with the guided option
parser:

commit 7299fa4b615d7f7ee12cde444266f6b31f667f9f
Author: Jan Engelhardt [off-list ref]
Date:   Sun Mar 6 15:54:58 2011 +0100

    libxt_CONNMARK: use guided option parser

You need a patch to use the new interface to stay in sync with current
iptables libraries. I'll make it for tc and send it to you.

BTW, I think it would be good if we find the way to check for
libxtables current version (see iptables/configure.ac), so you can
know that we broke binary compatibility again.

Cheers,
Pablo

[PATCH] build: unbreak linkage of m_xt.so

From: Jan Engelhardt <hidden>
Date: 2012-12-16 00:32:48

Commit v3.7.0~10 caused the variable new PKG_CONFIG variable never
to be present at the time of calling make, leading to tc/m_xt.so
not linked with -lxtables (result from pkg-config xtables --libs),
that in turn leading to

tc: symbol lookup error: /usr/lib64/tc//m_xt.so: undefined symbol:
xtables_init_all

Fixing that.

Signed-off-by: Jan Engelhardt <redacted>
---
 configure |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 9912114..573ee55 100755
--- a/configure
+++ b/configure
@@ -4,7 +4,6 @@
 INCLUDE=${1:-"$PWD/include"}
 : ${PKG_CONFIG:=pkg-config}
 : ${CC=gcc}
-echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
 
 # Make a temp directory in build tree.
 TMPDIR=$(mktemp -d config.XXXXXX)
@@ -224,6 +223,7 @@ rm -f $TMPDIR/ipsettest.c $TMPDIR/ipsettest
 }
 
 echo "# Generated config based on" $INCLUDE >Config
+echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
 
 echo "TC schedulers"
 
-- 
1.7.10.4

Re: tc ipt action

From: Jan Engelhardt <hidden>
Date: 2012-12-16 00:59:37

On Sunday 2012-12-16 01:27, Pablo Neira Ayuso wrote:
On Sat, Dec 15, 2012 at 04:19:29PM -0500, Jamal Hadi Salim wrote:
quoted
Example, the following should work for
tc filter add dev eth0 parent ffff: protocol ip u32 match u32 0 0
action ipt -j CONNMARK \
action mirred egress redirect dev ifb0
commit 7299fa4b615d7f7ee12cde444266f6b31f667f9f
   libxt_CONNMARK: use guided option parser

BTW, I think it would be good if we find the way to check for
libxtables current version (see iptables/configure.ac), so you can
know that we broke binary compatibility again.
For the C level, there is XTABLES_VERSION_CODE.

#if XTABLES_VERSION_CODE >= 6
	if (m != NULL && m->x6_parse != NULL)
		m->x6_parse(...)
#else
	else if (m != NULL && m->parse != NULL)
		m->parse(...)
	...

We can also export this through pkgconfig, similar to how
downstream users are to discover the plugin dir
(`pkg-config xtables --variable libdir`).

Re: tc ipt action

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-16 10:22:13

On 12-12-15 06:06 PM, Jan Engelhardt wrote:
If I try that command (substituting ipt->xt and eth0->dummy0,
ifb0->dummy1), all I get is the dreaded "Invalid argument".
So the kernel rejected the command, which could indicate that
userspace construction might have been ok.

# tc filter add dev dummy0 parent ffff: protocol ip u32 match u32 0 0 \
action xt -j CONNMARK action mirred egress redirect dev dummy1

tablename: mangle hook: NF_IP_PRE_ROUTING
         target:  CONNMARK and 0x0 index 0
RTNETLINK answers: Invalid argument
We have an error talking to the kernel
No problem sending it to the kernel here on ubuntu 12.04.
I also upgraded to current linus git tree, same result.
The problem is the parameters are not accepted in user space as
you can see for connmark and what gets sent (eg CONNMARK and 0x0)
doesnt seem sensible.
What was the last combination that worked?
First time this got reported to me (or i got CCed on the problem) - I am 
told it broke after iptables 1.4.11.

cheers,
jamal

Re: tc ipt action

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-16 10:26:02

Hi Pablo,

On 12-12-15 07:27 PM, Pablo Neira Ayuso wrote:
The binary interface was broken in 1.4.11 with the guided option
parser:
Ah. Thanks that would explain it.

You need a patch to use the new interface to stay in sync with current
iptables libraries. I'll make it for tc and send it to you.
Much thanks. I just scanned it and things have changed; old way used to 
take multiparams. New one a single struct, so would have taken much 
longer for me to resolve.
BTW, I think it would be good if we find the way to check for
libxtables current version (see iptables/configure.ac), so you can
know that we broke binary compatibility again.
will do.

cheers,
jamal

Re: [PATCH] build: unbreak linkage of m_xt.so

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-16 10:30:53

On 12-12-15 07:32 PM, Jan Engelhardt wrote:
Commit v3.7.0~10 caused the variable new PKG_CONFIG variable never
to be present at the time of calling make, leading to tc/m_xt.so
not linked with -lxtables (result from pkg-config xtables --libs),
that in turn leading to

tc: symbol lookup error: /usr/lib64/tc//m_xt.so: undefined symbol:
xtables_init_all
Yep - run into this problem, scratching my head thinking something
wrong with my environment with latest iproute2 git tree. I hacked
mine to just always include xtables in LDLIBS.
Fixing that.

Signed-off-by: Jan Engelhardt <redacted>
I can confirm it builds fine for me now if i take out the hack I had and 
use this patch.
Acked-by: Jamal Hadi Salim <redacted>

Stephen, I think this patch would be equivalent of "stable fix".

cheers,
jamal

Re: tc ipt action

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-16 10:43:13

On 12-12-15 07:59 PM, Jan Engelhardt wrote:
For the C level, there is XTABLES_VERSION_CODE.

#if XTABLES_VERSION_CODE >= 6
	if (m != NULL && m->x6_parse != NULL)
		m->x6_parse(...)
#else
	else if (m != NULL && m->parse != NULL)
		m->parse(...)
	...
I think you are suggesting this to be done in tc. That would make it 
easier to fix.
IMO, it is easier to keep backward compat if you left the old
APIs around for a period of time and maybe log a warning that they
will be deprecated over a period of time (sort of like kernel approach 
to changing APIs).

BTW: another interface that seems to have changed that we
need is m->final_check().

cheers,
jamal
We can also export this through pkgconfig, similar to how
downstream users are to discover the plugin dir
(`pkg-config xtables --variable libdir`).

Re: [PATCH] build: unbreak linkage of m_xt.so

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-16 17:03:55

On 12-12-16 05:30 AM, Jamal Hadi Salim wrote:
I can confirm it builds fine for me now if i take out the hack I had and
use this patch.

Sorry, I take what i said back and went back to explicitly adding -l 
xtables. The problem is still the intepretation of tc/Makefile. Here's 
the compile output.
----
gcc -Wall -Wstrict-prototypes -O2 -I../include -DRESOLVE_HOSTNAMES 
-DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE 
-DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" 
-DYY_NO_INPUT -Wl,-export-dynamic -shared -fpic -o m_xt.so m_xt.c 
$(pkg-config xtables --cflags --libs)
----

Note the missing expansion.

cheers,
jamal

Re: tc ipt action

From: Jan Engelhardt <hidden>
Date: 2012-12-16 17:21:11

On Sunday 2012-12-16 11:43, Jamal Hadi Salim wrote:
On 12-12-15 07:59 PM, Jan Engelhardt wrote:
quoted
quoted
For the C level, there is XTABLES_VERSION_CODE.

#if XTABLES_VERSION_CODE >= 6
	if (m != NULL && m->x6_parse != NULL)
		m->x6_parse(...)
#else
	else if (m != NULL && m->parse != NULL)
		m->parse(...)
	...
I think you are suggesting this to be done in tc. That would make it easier to
fix.
IMO, it is easier to keep backward compat if you left the old
APIs around for a period of time
As you can see, the old ->parse() that goes back to libxtables.so.0
still remains. It's just that... only 5 out of 99 plugins still come
with an old parse function.

	[m_xt] -> [libxtables] <- (plugins: libxt_*.so)
and maybe log a warning that they
will be deprecated over a period of time (sort of like kernel approach to
changing APIs).
old parse has not entered any deprecation stage yet, since there are still
plugins out there (both the 5 and external ones) that make use of it.
Right now, both parse and x6_parse are valid.
BTW: another interface that seems to have changed that we
need is m->final_check().
Yes, all those with an x6_ prefix are new.
Mh, I already dream of plans reducing m_xt to something that
does not require libxtables.so anymore.

Re: [PATCH] build: unbreak linkage of m_xt.so

From: Jan Engelhardt <hidden>
Date: 2012-12-16 17:43:05

On Sunday 2012-12-16 18:03, Jamal Hadi Salim wrote:
On 12-12-16 05:30 AM, Jamal Hadi Salim wrote:
quoted
I can confirm it builds fine for me now if i take out the hack I had and
use this patch.

Sorry, I take what i said back and went back to explicitly adding -l xtables.
The problem is still the intepretation of tc/Makefile. Here's the compile
output.
----
gcc -Wall -Wstrict-prototypes -O2 -I../include -DRESOLVE_HOSTNAMES
-DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -DCONFIG_GACT
-DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT
-Wl,-export-dynamic -shared -fpic -o m_xt.so m_xt.c $(pkg-config xtables
--cflags --libs)
----
I saw the same during make, _but_, on running `ldd tc/m_xt.so`, I got a
libxtables.so entry, so I thought I was fine.



"$() "is something for the shell to expand, not make. See this testcase.

$ make
echo $(pkg-config xtables --cflags --libs)
-I/usr/include/iptables-1.4.16.3 -lxtables
$ cat Makefile 
a:
        echo $$(pkg-config xtables --cflags --libs)

Re: tc ipt action

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-16 17:47:13

On 12-12-16 12:21 PM, Jan Engelhardt wrote:

As you can see, the old ->parse() that goes back to libxtables.so.0
still remains. It's just that... only 5 out of 99 plugins still come
with an old parse function.
I see.
So calling m->XXX may not be a wise long term solution.
Hasan's patch has a call to xtables_option_tpcall(), if that is the 
right interface I hope that going forward if any of the m->parseXX
changes you will take care of hiding all that stuff.
old parse has not entered any deprecation stage yet, since there are still
plugins out there (both the 5 and external ones) that make use of it.
Right now, both parse and x6_parse are valid.
True - but we are getting broken because we are using a call that only 5 
or so users provide. It would have saved us time if we got the
a good log message instead of checking for if !m->parse()
Yes, all those with an x6_ prefix are new.
Mh, I already dream of plans reducing m_xt to something that
does not require libxtables.so anymore.
That would be nice - but someone is going to have to link to libxtables, no?

cheers,
jamal

Re: [PATCH] build: unbreak linkage of m_xt.so

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-16 18:05:32

On 12-12-16 12:43 PM, Jan Engelhardt wrote:
On Sunday 2012-12-16 18:03, Jamal Hadi Salim wrote:
I saw the same during make, _but_, on running `ldd tc/m_xt.so`, I got a
libxtables.so entry, so I thought I was fine.
Sorry, you are right. Without your patch that doesnt happen. I had 
removed the global dlopen while debugging, so it was using the wrong
m_xt.so

So my Ack is back on;->

cheers,
jamal

Re: tc ipt action

From: Jan Engelhardt <hidden>
Date: 2012-12-16 18:59:26

On Sunday 2012-12-16 18:47, Jamal Hadi Salim wrote:
quoted
old parse has not entered any deprecation stage yet, since there are still
plugins out there (both the 5 and external ones) that make use of it.
Right now, both parse and x6_parse are valid.
True - but we are getting broken because we are using a call that only 5 or so
users provide. It would have saved us time if we got the
a good log message instead of checking for if !m->parse()
A certainly safe bet would be to write, at the top of tc/m_xt.c,

#if XTABLES_VERSION_CODE > 9
#	error Someone call the guy who changed iptables and \
		make him fix it^W^W^W^W say you need help.
#endif

Then I would automatically notify myself of "oh I need fix that too" when I
feed any new releases of {iptables, iproute} through the Open Build Service.
quoted
Yes, all those with an x6_ prefix are new.
Mh, I already dream of plans reducing m_xt to something that
does not require libxtables.so anymore.
That would be nice - but someone is going to have to link to libxtables, no?
I hope the complete opposite.

The following is a rough, it-compiles, untested never-run, draft of a
module. The vision here is that userspace only ever sends a chain
name to the kernel. The git tree/branch for it is

	git://git.inai.de/linux xt2-pktsched

commit 42c559c148cbbc22bf2cc29f2ad08bc330891838

    net_sched: act: new action to call into Xtables2 chains

 include/net/netfilter/xt_core.h    |    8 ++
 include/uapi/linux/tc_act/tc_ipt.h |    2 +
 net/netfilter/xt_core.c            |    3 +-
 net/sched/Kconfig                  |    9 ++
 net/sched/Makefile                 |    1 +
 net/sched/act_xtables.c            |  238 ++++++++++++++++++++++++++++++++++++
 6 files changed, 260 insertions(+), 1 deletion(-)

Re: tc ipt action

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-16 20:35:10

On 12-12-16 01:59 PM, Jan Engelhardt wrote:

A certainly safe bet would be to write, at the top of tc/m_xt.c,

#if XTABLES_VERSION_CODE > 9
#	error Someone call the guy who changed iptables and \
		make him fix it^W^W^W^W say you need help.
#endif
Excellent idea ;->

The following is a rough, it-compiles, untested never-run, draft of a
module. The vision here is that userspace only ever sends a chain
name to the kernel. The git tree/branch for it is

	git://git.inai.de/linux xt2-pktsched

commit 42c559c148cbbc22bf2cc29f2ad08bc330891838

I'll look at it later - very slow connection at the moment so cloning 
will take a while.

cheers,
jamal

Re: tc ipt action

From: Jan Engelhardt <hidden>
Date: 2012-12-16 21:21:19

On Sunday 2012-12-16 21:35, Jamal Hadi Salim wrote:
quoted
	git://git.inai.de/linux xt2-pktsched
commit 42c559c148cbbc22bf2cc29f2ad08bc330891838
I'll look at it later - very slow connection at the moment so cloning will take
a while.
If you have a preexisting clone of any linux tree, you can utilize
`git remote add ...` to only grab the deltas.

Re: [PATCH] build: unbreak linkage of m_xt.so

From: Mike Frysinger <hidden>
Date: 2012-12-16 22:02:22

On Saturday 15 December 2012 19:32:48 Jan Engelhardt wrote:
quoted hunk
--- a/configure
+++ b/configure
@@ -4,7 +4,6 @@
 INCLUDE=${1:-"$PWD/include"}

 : ${PKG_CONFIG:=pkg-config}
 : ${CC=gcc}

-echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config

 # Make a temp directory in build tree.
 TMPDIR=$(mktemp -d config.XXXXXX)
@@ -224,6 +223,7 @@ rm -f $TMPDIR/ipsettest.c $TMPDIR/ipsettest
 }

 echo "# Generated config based on" $INCLUDE >Config
+echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config

 echo "TC schedulers"
the use of un-indented shell functions makes the code read in a way it doesn't 
actually execute.  i'd suggest moving this logic into a function to match 
existing style rather than simply moving the Config write.  i'll post a patch.
-mike

Re: tc ipt action

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-17 12:58:38

On 12-12-16 04:21 PM, Jan Engelhardt wrote:
If you have a preexisting clone of any linux tree, you can utilize
`git remote add ...` to only grab the deltas.
It downloaded eventually.
So looking at this quickly, basic question: is xtables2 different API 
wise from what we do today in act_ipt?
Second: Are chain names unique system wide? i.e at the moment we send
a hook and table selection?
The patch i have currently for the kernel tries to pursue an approach 
that maximizes code reuse - depending on your answer I may go the 
approach of having a separate act_xt and hope you can build on top of that.

cheers,
jamal

Re: tc ipt action

From: Jan Engelhardt <hidden>
Date: 2012-12-17 13:28:57

On Monday 2012-12-17 13:58, Jamal Hadi Salim wrote:
On 12-12-16 04:21 PM, Jan Engelhardt wrote:
quoted
If you have a preexisting clone of any linux tree, you can utilize
`git remote add ...` to only grab the deltas.
It downloaded eventually. So looking at this quickly, basic
question: is xtables2 different API wise from what we do today in
act_ipt?
AFAICS, (one instance of) act_ipt today directly invokes (exactly one
instance of) a target. With act_xt2 as drafted, it instead invokes a
chain, which would

1. leave the construction of the target data and calling it
   to the subsystems they conceptually belong to - the packet filter
2. lets you do matches, jumps and all that.
Second: Are chain names unique system wide?
Good thing you ask. Chain names are unique within a netns, and this
act_xtables.c draft looks at the packet to get to know its netns, so
that seems fine.

However, your question also leads to looking at whether TC Actions
themselves are sufficiently netns-ified, and it seems this is _not_
the case. Am I right in the observation that variables like
"tcf_ipt_ht" are in fact global rather tha per-netns?

Re: tc ipt action

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-18 13:29:43

On 12-12-17 08:28 AM, Jan Engelhardt wrote:
On Monday 2012-12-17 13:58, Jamal Hadi Salim wrote:
AFAICS, (one instance of) act_ipt today directly invokes (exactly one
instance of) a target.
Design intent.
You can have the same target instance used by specifying the same index
on the command line.
With act_xt2 as drafted, it instead invokes a chain, which would

1. leave the construction of the target data and calling it
    to the subsystems they conceptually belong to - the packet filter
2. lets you do matches, jumps and all that.
I like #2. For #1 as long as it doesnt deviate from desire to have one 
or more instances of targets, we should be fine.
Good thing you ask. Chain names are unique within a netns, and this
act_xtables.c draft looks at the packet to get to know its netns, so
that seems fine.
My motivation for that question:
Is it possible to ignore the hook and tablename and just use the chain
name?
However, your question also leads to looking at whether TC Actions
themselves are sufficiently netns-ified, and it seems this is _not_
the case. Am I right in the observation that variables like
"tcf_ipt_ht" are in fact global rather tha per-netns?
In general we dont need to worry about netns since actions are attached 
to the filters which are dependent on qdiscs which are dependent on 
netdevs which are per netns.
I believe actions (not filters or qdiscs) have a way where this can
be circumvented in one scenario (I can configure them bypassing the 
filter interface). Thanks for bringing this up - I will look at it.

cheers,
jamal

Re: tc ipt action

From: Jan Engelhardt <hidden>
Date: 2012-12-18 13:58:56

On Tuesday 2012-12-18 14:23, Jamal Hadi Salim wrote:
On 12-12-17 08:28 AM, Jan Engelhardt wrote:
quoted
With act_xt2 as drafted, it instead invokes a chain, which would

1. leave the construction of the target data and calling it
   to the subsystems they conceptually belong to - the packet filter
2. lets you do matches, jumps and all that.
I like #2. For #1 as long as it doesnt deviate from desire to have
one or more instances of targets, we should be fine.
Chains can store multiple targets, so no loss.
quoted
Good thing you ask. Chain names are unique within a netns, and this
act_xtables.c draft looks at the packet to get to know its netns, so
that seems fine.
My motivation for that question:
Is it possible to ignore the hook and tablename and just use the chain
name?
1. table

First, I think some targets need to relax their restrictions, such as
with xt_DSCP.

Then, only a handful of extensions remain: CT, <all NATs>,
TPROXY and REJECT. Would anyone want to call these from act_ipt?
I doubt it. :)

2. hooks

Extensions with hook limit: <NAT>, TPROXY, REJECT, CLASSIFY.
Again, I don't quite see the value of attempting to NAT from act_ipt.
CLASSIFY {c|sh?}ould be relaxed, unless I am missing something.

Re: [PATCH] build: unbreak linkage of m_xt.so

From: Stephen Hemminger <hidden>
Date: 2012-12-18 17:21:30

On Sun, 16 Dec 2012 01:32:48 +0100
Jan Engelhardt [off-list ref] wrote:
quoted hunk
Commit v3.7.0~10 caused the variable new PKG_CONFIG variable never
to be present at the time of calling make, leading to tc/m_xt.so
not linked with -lxtables (result from pkg-config xtables --libs),
that in turn leading to

tc: symbol lookup error: /usr/lib64/tc//m_xt.so: undefined symbol:
xtables_init_all

Fixing that.

Signed-off-by: Jan Engelhardt <redacted>
---
 configure |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 9912114..573ee55 100755
--- a/configure
+++ b/configure
@@ -4,7 +4,6 @@
 INCLUDE=${1:-"$PWD/include"}
 : ${PKG_CONFIG:=pkg-config}
 : ${CC=gcc}
-echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
 
 # Make a temp directory in build tree.
 TMPDIR=$(mktemp -d config.XXXXXX)
@@ -224,6 +223,7 @@ rm -f $TMPDIR/ipsettest.c $TMPDIR/ipsettest
 }
 
 echo "# Generated config based on" $INCLUDE >Config
+echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
 
 echo "TC schedulers"
 
Ok, manually did the diff (conflicted with other previous changes).

Re: [PATCH] build: unbreak linkage of m_xt.so

From: Mike Frysinger <hidden>
Date: 2012-12-18 18:47:58

On Tuesday 18 December 2012 12:21:30 Stephen Hemminger wrote:
On Sun, 16 Dec 2012 01:32:48 +0100 Jan Engelhardt wrote:
quoted
Commit v3.7.0~10 caused the variable new PKG_CONFIG variable never
to be present at the time of calling make, leading to tc/m_xt.so
not linked with -lxtables (result from pkg-config xtables --libs),
that in turn leading to

tc: symbol lookup error: /usr/lib64/tc//m_xt.so: undefined symbol:
xtables_init_all

Fixing that.
--- a/configure
+++ b/configure
@@ -4,7 +4,6 @@
 INCLUDE=${1:-"$PWD/include"}
 : ${PKG_CONFIG:=pkg-config}
 : ${CC=gcc}
-echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config

 # Make a temp directory in build tree.
 TMPDIR=$(mktemp -d config.XXXXXX)
@@ -224,6 +223,7 @@ rm -f $TMPDIR/ipsettest.c $TMPDIR/ipsettest
 }
 
 echo "# Generated config based on" $INCLUDE >Config
+echo "PKG_CONFIG:=${PKG_CONFIG}" >>Config
 echo "TC schedulers"
Ok, manually did the diff (conflicted with other previous changes).
this patch is no longer necessary one you merged my:
	configure: move toolchain init to a function

it's actually undesirable to apply this after that since it makes the configure 
script less clear again ...

sorry if my commit message wasn't obvious.
-mike

Re: tc ipt action

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-19 11:43:25

On 12-12-18 08:58 AM, Jan Engelhardt wrote:
Chains can store multiple targets, so no loss.
Nice.
1. table

First, I think some targets need to relax their restrictions, such as
with xt_DSCP.
Saw your other patch to get rid of mangle hardcoding.
Then, only a handful of extensions remain: CT, <all NATs>,
TPROXY and REJECT. Would anyone want to call these from act_ipt?
I doubt it. :)
Tempted to say tproxy.
2. hooks

Extensions with hook limit: <NAT>, TPROXY, REJECT, CLASSIFY.
Again, I don't quite see the value of attempting to NAT from act_ipt.
CLASSIFY {c|sh?}ould be relaxed, unless I am missing something.

I could live with that. It would be an improvement over whats there 
today. I would prefer however for this to be an improvement over
act_xt.c i posted as opposed to have even more interfaces for xt.
We've suffered enough already ;-> i.e add your patches on top.

cheers,
jamal

Re: [PATCH] build: unbreak linkage of m_xt.so

From: Stephen Hemminger <hidden>
Date: 2012-12-20 00:03:17

On Tue, 18 Dec 2012 13:47:58 -0500
Mike Frysinger [off-list ref] wrote:
this patch is no longer necessary one you merged my:
	configure: move toolchain init to a function

it's actually undesirable to apply this after that since it makes the configure 
script less clear again ...

sorry if my commit message wasn't obvious.
-mike
ok, went back to old way.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help