MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]

On Fri, Jul 25, 2003 at 09:56:57PM +0400, kuznet@ms2.inr.ac.ru wrote:

> Hello!

Hi Alexey,  I have to follow-up on this old thraed.

> > Hmm, what's your routing setup?  And what kernel?  It's possible with
> > wierd setups, like source routing.
> 
> Unlikely, source address is unspecified here. Most likely, it is fwmark.
> 
> Unrelated: giving out->ifindex is a bug, by the way. It can screw up
> the things a lot. In this context, if you want to be sure that packet
> will go out expected interface you do plain lookup and drop packet
> if it gave you some strange route.

Your proposed change (key.oif = 0 instead of out->ifindex) went into
2.4.23, and we've received a number of bug reports like
https://bugzilla.netfilter.org/cgi-bin/bugzilla/show_bug.cgi?id=144
http://www.ussg.iu.edu/hypermail/linux/kernel/0312.0/0465.html
http://www.ussg.iu.edu/hypermail/linux/kernel/0312.0/0408.html

This means that ip_route_output_key() returns a route with a different
outgoing interface than the skb->dst->dev of our to-be-masqueraded
packet.  

Why was it wrong to specify skb->dst->dev->ifindex of the previous
'real' routing decision as key to our current routing decision?

As an example case where I would suspect problems: The packet could be
coming from a local socket, and the socket be bound to a specific
interface (sk->bound_dev_if).

Please comment, thanks.

> Alexey

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Julian Anastasov]

	Hello,

On Sun, 11 Jan 2004, Harald Welte wrote:

> As an example case where I would suspect problems: The packet could be
> coming from a local socket, and the socket be bound to a specific
> interface (sk->bound_dev_if).

	IMO, the real example is that the people use multipath routes
and providing oif was the only way MASQUERADE to meet the netfilter
and firewalling expectations of not changing the output device during
hooks. OTOH, providing oif=0 is the valid approach for selecting the
right route but as long as the above expectation exists there are
two options for the users:

- provide oif learned from the input route (as before the discussed
change). May be in 99% of the setups it selects the right route.
I think, we should use this, at least for 2.4.

- use CONNMARK or similar functionality to keep the connection
bound to its path. As long as CONNMARK is not a standard feature
there is no safe way to use multipath routes with MASQUERADE and
SNAT in the latest kernels. Even before this change it was risky
to rely on the routing cache to keep NAT connections bound to
its path in the multipath route - the cache entries expire.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

On Sun, 11 Jan 2004, Julian Anastasov wrote:

> - use CONNMARK or similar functionality to keep the connection
> bound to its path. As long as CONNMARK is not a standard feature
> there is no safe way to use multipath routes with MASQUERADE and
> SNAT in the latest kernels. Even before this change it was risky
> to rely on the routing cache to keep NAT connections bound to
> its path in the multipath route - the cache entries expire.

As the aurhor of CONNMARK I certainly do not mind having this progress 
beyond path-o-matic extras..

While it was invented to solve a special-case issue, it has over the years 
found many additional and more general uses. Today it is in use for

a) Multihomed setups of a shared network with limited routing tables

b) Reliable and easy to understand multipath+NAT routing.

c) Interception routing without NAT, routing specific TCP sessions
(including RELATED and ICMP messages) a special path, usually for 
interception caching outside of the router without loss of addressing 
information.

and probably several other applications I do not know about or simply 
forgotten.

Regards
Henrik

Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

[Stripped Cc' list since it is not the original subject]

On Sun, Jan 11, 2004 at 10:11:43PM +0100, Henrik Nordstrom wrote:
> On Sun, 11 Jan 2004, Julian Anastasov wrote:
> 
> > - use CONNMARK or similar functionality to keep the connection
> > bound to its path. As long as CONNMARK is not a standard feature
> > there is no safe way to use multipath routes with MASQUERADE and
> > SNAT in the latest kernels. Even before this change it was risky
> > to rely on the routing cache to keep NAT connections bound to
> > its path in the multipath route - the cache entries expire.
> 
> As the aurhor of CONNMARK I certainly do not mind having this progress 
> beyond path-o-matic extras..

I think we could push CONNMARK to the mainstream kernel in the next
couple of weeks.  

I'd like to discuss one change with you first... that is using the mark
field as a bitmask.

We had that idea for a very long time, and didn't ever change it for the
skb->nfmark field for the sake of compatibility.

But now, when introducing a new mark field (the conntrack->mark field),
I'd rather prefer implement matching/setting individual bitmasks from
the beginning.  What do you think?  Would you be willing to add that
feature, or alternatively don't mind if I'd add the respective changes
myself?

Also, please add a Copyright notice including your name and a reference
to the GPL on top of the .c files.

Another minor cosmetic issue: Could you please update it to use
C99-style structure initializers in the code?

Last, but not least:  We'd need an etry for the iptables(8) manpage.

Thanks.

> Regards
> Henrik

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

On Sat, 17 Jan 2004, Harald Welte wrote:

> I think we could push CONNMARK to the mainstream kernel in the next
> couple of weeks.  

Great!

> I'd like to discuss one change with you first... that is using the mark
> field as a bitmask.
> 
> We had that idea for a very long time, and didn't ever change it for the
> skb->nfmark field for the sake of compatibility.
> 
> But now, when introducing a new mark field (the conntrack->mark field),
> I'd rather prefer implement matching/setting individual bitmasks from
> the beginning.  What do you think?  Would you be willing to add that
> feature, or alternatively don't mind if I'd add the respective changes
> myself?

It is fine by me. It is a trivial thing to add as the extension is not 
yet frozen.

> Also, please add a Copyright notice including your name and a reference
> to the GPL on top of the .c files.

Will do.

> Another minor cosmetic issue: Could you please update it to use
> C99-style structure initializers in the code?

Sure.

> Last, but not least:  We'd need an etry for the iptables(8) manpage.

Already there from the beginning, but lost in p-o-m due to the lack of a
clear structure on how manpage additions should be maintained for not yet 
official extensions. (the help is copy-paste from the manpage output)

Should I also write a more module like help text for these modules?

Btw, any plans on how iptables manpage snippets for extra/not yet
submitted matches/targets should be maintained?

The requested changes has been done but is not yet tested. Testing should 
be finished tomorrow when I will submit the changes.

Regards
Henrik

IP Options with libipq

[XiChimos]

I hate to repost, but I still need help.  I need to use libipq to add 
and remove an IP option defined by me.  I am working from the sample in 
the man page and also samples from the CVS tools folder.  Does anyone 
have *any* idea how to do this?

Chris

Re: IP Options with libipq

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

On Sat, Jan 17, 2004 at 07:54:47AM -0500, XiChimos wrote:
> I hate to repost, but I still need help.  I need to use libipq to add 
> and remove an IP option defined by me.  I am working from the sample in 
> the man page and also samples from the CVS tools folder.  Does anyone 
> have *any* idea how to do this?

where's the exact problem?  You get the packet via
ipq_read()/ipq_get_packet(), then modify it according to your needs, and
send it back via ipq_set_verdict(handle, id, NF_ACCEPT, new_len,
buffer).

> Chris

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1912 bytes --]

On Sat, Jan 17, 2004 at 06:50:21PM +0100, Henrik Nordstrom wrote:
> It is fine by me. It is a trivial thing to add as the extension is not 
> yet frozen.

yup. Please submit your changes against pom-ng (and patch-o-matic if you
want, but as of now I don't think we'll see another pom-old release).

> > Last, but not least:  We'd need an etry for the iptables(8) manpage.
> 
> Already there from the beginning, but lost in p-o-m due to the lack of a
> clear structure on how manpage additions should be maintained for not yet 
> official extensions. (the help is copy-paste from the manpage output)
> 
> Should I also write a more module like help text for these modules?

the 'help' file in patch-o-matic-ng/CONNMARK/ is fine.

> Btw, any plans on how iptables manpage snippets for extra/not yet
> submitted matches/targets should be maintained?

well, we could do some semi-automated mechanism, like splitting it up in
one file per extension, and then concatenate the whole manpage from
those snippets depending on PF_EXT_LIBS / PF6_EXT_LIBS during the
iptables userspace make process.

If somebody wants to take that job, I'd be more than happy.

As for pkttables, the design includes automatic manpage generation from the
helptexts (included in libpktt_FOO.so).  But that's yet-to-do.

> The requested changes has been done but is not yet tested. Testing should 
> be finished tomorrow whenI will submit the changes.

great.  I'm again on the road (LWE), so expect some delays.

> Regards
> Henrik

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 832 bytes --]

On Sun, 18 Jan 2004, Harald Welte wrote:

> yup. Please submit your changes against pom-ng (and patch-o-matic if you
> want, but as of now I don't think we'll see another pom-old release).

Attached you find 

iptables userspace patch
	- manpage additions
	- mask options
	- copyright & license
	- C99 struct initializers
iptables pom-ng	patch
	- mask operations
	- copyright & license
	- C99 struct initializers
iptables pom patch
	- identical to the pom-ng patch 

> well, we could do some semi-automated mechanism, like splitting it up in
> one file per extension, and then concatenate the whole manpage from
> those snippets depending on PF_EXT_LIBS / PF6_EXT_LIBS during the
> iptables userspace make process.

Good idea. Should be pretty trivial except for the sorting (may need to
fight a little with locale)

Regards
Henrik

[-- Attachment #2: Type: TEXT/PLAIN, Size: 11415 bytes --]

Index: extra/CONNMARK.patch
===================================================================
RCS file: /cvspublic/netfilter/patch-o-matic/extra/CONNMARK.patch,v
retrieving revision 1.4
diff -u -p -r1.4 CONNMARK.patch
--- extra/CONNMARK.patch	23 Aug 2003 22:17:08 -0000	1.4
+++ extra/CONNMARK.patch	18 Jan 2004 17:03:29 -0000
@@ -36,27 +36,60 @@ diff -uN --exclude .* --exclude *.o linu
  		LIST_DELETE(&ip_conntrack_expect_list, expected);
  		expected->expectant->expecting--;
  		nf_conntrack_get(&master_ct(conntrack)->infos[0]);
-diff -uN linux-2.4.3-pre3/include/linux/netfilter_ipv4/ipt_connmark.h linux-2.4.3-pre3-uml/include/linux/netfilter_ipv4/ipt_connmark.h
---- linux-2.4.3-pre3/include/linux/netfilter_ipv4/ipt_connmark.h	Thu Jan  1 01:00:00 1970
-+++ linux-2.4.3-pre3-uml/include/linux/netfilter_ipv4/ipt_connmark.h	Wed Mar 21 11:38:46 2001
-@@ -0,0 +1,9 @@
+diff -ruN --exclude CVS linux-orig/include/linux/netfilter_ipv4/ipt_connmark.h linux/include/linux/netfilter_ipv4/ipt_connmark.h
+--- linux-orig/include/linux/netfilter_ipv4/ipt_connmark.h	Thu Jan  1 01:00:00 1970
++++ linux/include/linux/netfilter_ipv4/ipt_connmark.h	Sun Jan 18 17:33:42 2004
+@@ -0,0 +1,18 @@
 +#ifndef _IPT_CONNMARK_H
 +#define _IPT_CONNMARK_H
 +
++/* Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
++ * by Henrik Nordstrom <hno@marasystems.com>
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; either version 2 of the License, or
++ * (at your option) any later version.
++ */
++
 +struct ipt_connmark_info {
 +	unsigned long mark, mask;
 +	u_int8_t invert;
 +};
 +
 +#endif /*_IPT_CONNMARK_H*/
-diff -uN --exclude .* --exclude *.o linux-2.4.3-pre3/net/ipv4/netfilter/ipt_connmark.c linux-2.4.3-pre3-uml/net/ipv4/netfilter/ipt_connmark.c
---- linux-2.4.3-pre3/net/ipv4/netfilter/ipt_connmark.c	Thu Jan  1 01:00:00 1970
-+++ linux-2.4.3-pre3-uml/net/ipv4/netfilter/ipt_connmark.c	Wed Mar 21 13:23:33 2001
-@@ -0,0 +1,55 @@
-+/* Kernel module to match connection mark values. */
+diff -ruN --exclude CVS linux-orig/net/ipv4/netfilter/ipt_connmark.c linux/net/ipv4/netfilter/ipt_connmark.c
+--- linux-orig/net/ipv4/netfilter/ipt_connmark.c	Thu Jan  1 01:00:00 1970
++++ linux/net/ipv4/netfilter/ipt_connmark.c	Sun Jan 18 17:57:58 2004
+@@ -0,0 +1,83 @@
++/* This kernel module matches connection mark values set by the
++ * CONNMARK target
++ *
++ * Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
++ * by Henrik Nordstrom <hno@marasystems.com>
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; either version 2 of the License, or
++ * (at your option) any later version.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program; if not, write to the Free Software
++ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
++ */
++
 +#include <linux/module.h>
 +#include <linux/skbuff.h>
 +
++MODULE_AUTHOR("Henrik Nordstrom <hno@marasytems.com>");
++MODULE_DESCRIPTION("IP tables connmark match module");
++MODULE_LICENSE("GPL");
++
 +#include <linux/netfilter_ipv4/ip_tables.h>
 +#include <linux/netfilter_ipv4/ipt_connmark.h>
 +#include <linux/netfilter_ipv4/ip_conntrack.h>
@@ -82,10 +115,10 @@ diff -uN --exclude .* --exclude *.o linu
 +
 +static int
 +checkentry(const char *tablename,
-+           const struct ipt_ip *ip,
-+           void *matchinfo,
-+           unsigned int matchsize,
-+           unsigned int hook_mask)
++	   const struct ipt_ip *ip,
++	   void *matchinfo,
++	   unsigned int matchsize,
++	   unsigned int hook_mask)
 +{
 +	if (matchsize != IPT_ALIGN(sizeof(struct ipt_connmark_info)))
 +		return 0;
@@ -93,8 +126,12 @@ diff -uN --exclude .* --exclude *.o linu
 +	return 1;
 +}
 +
-+static struct ipt_match connmark_match
-+= { { NULL, NULL }, "connmark", &match, &checkentry, NULL, THIS_MODULE };
++static struct ipt_match connmark_match = {
++	.name = "connmark",
++	.match = &match,
++	.checkentry = &checkentry,
++	.me = THIS_MODULE
++};
 +
 +static int __init init(void)
 +{
@@ -108,37 +145,68 @@ diff -uN --exclude .* --exclude *.o linu
 +
 +module_init(init);
 +module_exit(fini);
-diff -uN linux-2.4.3-pre3/include/linux/netfilter_ipv4/ipt_CONNMARK.h linux-2.4.3-pre3-uml/include/linux/netfilter_ipv4/ipt_CONNMARK.h
---- linux-2.4.3-pre3/include/linux/netfilter_ipv4/ipt_CONNMARK.h	Thu Jan  1 01:00:00 1970
-+++ linux-2.4.3-pre3-uml/include/linux/netfilter_ipv4/ipt_CONNMARK.h	Wed Mar 21 12:25:20 2001
-@@ -0,0 +1,15 @@
+diff -ruN --exclude CVS linux-orig/include/linux/netfilter_ipv4/ipt_CONNMARK.h linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h
+--- linux-orig/include/linux/netfilter_ipv4/ipt_CONNMARK.h	Thu Jan  1 01:00:00 1970
++++ linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h	Sun Jan 18 17:58:43 2004
+@@ -0,0 +1,25 @@
 +#ifndef _IPT_CONNMARK_H_target
 +#define _IPT_CONNMARK_H_target
 +
++/* Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
++ * by Henrik Nordstrom <hno@marasystems.com>
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; either version 2 of the License, or
++ * (at your option) any later version.
++ */
++
 +enum {
-+    IPT_CONNMARK_SET = 0,
-+    IPT_CONNMARK_SAVE,
-+    IPT_CONNMARK_RESTORE
++	IPT_CONNMARK_SET = 0,
++	IPT_CONNMARK_SAVE,
++	IPT_CONNMARK_RESTORE
 +};
 +
 +struct ipt_connmark_target_info {
 +	unsigned long mark;
++	unsigned long mask;
 +	u_int8_t mode;
 +};
 +
 +#endif /*_IPT_CONNMARK_H_target*/
-diff -uN --exclude .* --exclude *.o linux-2.4.3-pre3/net/ipv4/netfilter/ipt_CONNMARK.c linux-2.4.3-pre3-uml/net/ipv4/netfilter/ipt_CONNMARK.c
---- linux-2.4.3-pre3/net/ipv4/netfilter/ipt_CONNMARK.c	Thu Jan  1 01:00:00 1970
-+++ linux-2.4.3-pre3-uml/net/ipv4/netfilter/ipt_CONNMARK.c	Wed May 20 17:49:10 2001
-@@ -0,0 +1,87 @@
-+/* This is a module which is used for setting/remembering the mark field of
-+ * an connection, or optionally restore it to the skb
+diff -ruN --exclude CVS linux-orig/net/ipv4/netfilter/ipt_CONNMARK.c linux/net/ipv4/netfilter/ipt_CONNMARK.c
+--- linux-orig/net/ipv4/netfilter/ipt_CONNMARK.c	Thu Jan  1 01:00:00 1970
++++ linux/net/ipv4/netfilter/ipt_CONNMARK.c	Sun Jan 18 17:58:15 2004
+@@ -0,0 +1,118 @@
++/* This kernel module is used to modify the connection mark values, or
++ * to optionally restore the skb nfmark from the connection mark
++ *
++ * Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
++ * by Henrik Nordstrom <hno@marasystems.com>
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; either version 2 of the License, or
++ * (at your option) any later version.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program; if not, write to the Free Software
++ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 + */
 +#include <linux/module.h>
 +#include <linux/skbuff.h>
 +#include <linux/ip.h>
 +#include <net/checksum.h>
 +
++MODULE_AUTHOR("Henrik Nordstrom <hno@marasytems.com>");
++MODULE_DESCRIPTION("IP tables CONNMARK matching module");
++MODULE_LICENSE("GPL");
++
 +#include <linux/netfilter_ipv4/ip_tables.h>
 +#include <linux/netfilter_ipv4/ipt_CONNMARK.h>
 +#include <linux/netfilter_ipv4/ip_conntrack.h>
@@ -152,20 +220,29 @@ diff -uN --exclude .* --exclude *.o linu
 +       void *userinfo)
 +{
 +	const struct ipt_connmark_target_info *markinfo = targinfo;
++	unsigned long diff;
++	unsigned long nfmark;
++	unsigned long newmark;
 +
 +	enum ip_conntrack_info ctinfo;
 +	struct ip_conntrack *ct = ip_conntrack_get((*pskb), &ctinfo);
 +	if (ct) {
 +	    switch(markinfo->mode) {
 +	    case IPT_CONNMARK_SET:
-+		ct->mark = markinfo->mark;
++		newmark = (ct->mark & ~markinfo->mask) | markinfo->mark;
++		if (newmark != ct->mark)
++		    ct->mark = newmark;
 +		break;
 +	    case IPT_CONNMARK_SAVE:
-+		ct->mark = (*pskb)->nfmark;
++		newmark = (ct->mark & ~markinfo->mask) | ((*pskb)->nfmark & markinfo->mask);
++		if (ct->mark != newmark)
++		    ct->mark = newmark;
 +		break;
 +	    case IPT_CONNMARK_RESTORE:
-+		if (ct->mark != (*pskb)->nfmark) {
-+		    (*pskb)->nfmark = ct->mark;
++		nfmark = (*pskb)->nfmark;
++		diff = (ct->mark ^ nfmark & markinfo->mask);
++		if (diff != 0) {
++		    (*pskb)->nfmark = nfmark ^ diff;
 +		    (*pskb)->nfcache |= NFC_ALTERED;
 +		}
 +		break;
@@ -178,9 +255,9 @@ diff -uN --exclude .* --exclude *.o linu
 +static int
 +checkentry(const char *tablename,
 +	   const struct ipt_entry *e,
-+           void *targinfo,
-+           unsigned int targinfosize,
-+           unsigned int hook_mask)
++	   void *targinfo,
++	   unsigned int targinfosize,
++	   unsigned int hook_mask)
 +{
 +	struct ipt_connmark_target_info *matchinfo = targinfo;
 +	if (targinfosize != IPT_ALIGN(sizeof(struct ipt_connmark_target_info))) {
@@ -200,15 +277,16 @@ diff -uN --exclude .* --exclude *.o linu
 +	return 1;
 +}
 +
-+static struct ipt_target ipt_connmark_reg
-+= { { NULL, NULL }, "CONNMARK", target, checkentry, NULL, THIS_MODULE };
++static struct ipt_target ipt_connmark_reg = {
++	.name = "CONNMARK",
++	.target = &target,
++	.checkentry = &checkentry,
++	.me = THIS_MODULE
++};
 +
 +static int __init init(void)
 +{
-+	if (ipt_register_target(&ipt_connmark_reg))
-+		return -EINVAL;
-+
-+	return 0;
++	return ipt_register_target(&ipt_connmark_reg);
 +}
 +
 +static void __exit fini(void)
Index: extra/CONNMARK.patch.configure.help
===================================================================
RCS file: /cvspublic/netfilter/patch-o-matic/extra/CONNMARK.patch.configure.help,v
retrieving revision 1.1
diff -u -p -r1.1 CONNMARK.patch.configure.help
--- extra/CONNMARK.patch.configure.help	21 May 2002 17:49:14 -0000	1.1
+++ extra/CONNMARK.patch.configure.help	18 Jan 2004 17:03:29 -0000
@@ -20,3 +20,8 @@ connmark match support
 CONFIP_IP_NF_MATCH_CONNMARK
   This option adds a `connmark' match, which allows you to match the
   connection mark value previously set for the session by `CONNMARK'. 
+
+  If you want to compile it as a module, say M here and read
+  Documentation/modules.txt.  The module will be called
+  ipt_connmark.o.  If unsure, say `N'.
+

[-- Attachment #3: Type: TEXT/PLAIN, Size: 9745 bytes --]

Index: CONNMARK//configure.help
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/configure.help,v
retrieving revision 1.1
diff -u -p -r1.1 configure.help
--- CONNMARK//configure.help	18 Dec 2003 18:55:59 -0000	1.1
+++ CONNMARK//configure.help	18 Jan 2004 17:04:01 -0000
@@ -20,3 +20,8 @@ connmark match support
 CONFIP_IP_NF_MATCH_CONNMARK
   This option adds a `connmark' match, which allows you to match the
   connection mark value previously set for the session by `CONNMARK'. 
+
+  If you want to compile it as a module, say M here and read
+  Documentation/modules.txt.  The module will be called
+  ipt_connmark.o.  If unsure, say `N'.
+
Index: CONNMARK//linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h,v
retrieving revision 1.1
diff -u -p -r1.1 ipt_CONNMARK.h
--- CONNMARK//linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h	18 Dec 2003 18:47:53 -0000	1.1
+++ CONNMARK//linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h	18 Jan 2004 17:04:01 -0000
@@ -1,14 +1,24 @@
 #ifndef _IPT_CONNMARK_H_target
 #define _IPT_CONNMARK_H_target
 
+/* Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
 enum {
-    IPT_CONNMARK_SET = 0,
-    IPT_CONNMARK_SAVE,
-    IPT_CONNMARK_RESTORE
+	IPT_CONNMARK_SET = 0,
+	IPT_CONNMARK_SAVE,
+	IPT_CONNMARK_RESTORE
 };
 
 struct ipt_connmark_target_info {
 	unsigned long mark;
+	unsigned long mask;
 	u_int8_t mode;
 };
 
Index: CONNMARK//linux/include/linux/netfilter_ipv4/ipt_connmark.h
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ipt_connmark.h,v
retrieving revision 1.1
diff -u -p -r1.1 ipt_connmark.h
--- CONNMARK//linux/include/linux/netfilter_ipv4/ipt_connmark.h	18 Dec 2003 18:47:53 -0000	1.1
+++ CONNMARK//linux/include/linux/netfilter_ipv4/ipt_connmark.h	18 Jan 2004 17:04:01 -0000
@@ -1,6 +1,15 @@
 #ifndef _IPT_CONNMARK_H
 #define _IPT_CONNMARK_H
 
+/* Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
 struct ipt_connmark_info {
 	unsigned long mark, mask;
 	u_int8_t invert;
Index: CONNMARK//linux/net/ipv4/netfilter/ipt_CONNMARK.c
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/linux/net/ipv4/netfilter/ipt_CONNMARK.c,v
retrieving revision 1.1
diff -u -p -r1.1 ipt_CONNMARK.c
--- CONNMARK//linux/net/ipv4/netfilter/ipt_CONNMARK.c	18 Dec 2003 18:47:53 -0000	1.1
+++ CONNMARK//linux/net/ipv4/netfilter/ipt_CONNMARK.c	18 Jan 2004 17:04:01 -0000
@@ -1,11 +1,32 @@
-/* This is a module which is used for setting/remembering the mark field of
- * an connection, or optionally restore it to the skb
+/* This kernel module is used to modify the connection mark values, or
+ * to optionally restore the skb nfmark from the connection mark
+ *
+ * Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/ip.h>
 #include <net/checksum.h>
 
+MODULE_AUTHOR("Henrik Nordstrom <hno@marasytems.com>");
+MODULE_DESCRIPTION("IP tables CONNMARK matching module");
+MODULE_LICENSE("GPL");
+
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv4/ipt_CONNMARK.h>
 #include <linux/netfilter_ipv4/ip_conntrack.h>
@@ -19,20 +40,29 @@ target(struct sk_buff **pskb,
        void *userinfo)
 {
 	const struct ipt_connmark_target_info *markinfo = targinfo;
+	unsigned long diff;
+	unsigned long nfmark;
+	unsigned long newmark;
 
 	enum ip_conntrack_info ctinfo;
 	struct ip_conntrack *ct = ip_conntrack_get((*pskb), &ctinfo);
 	if (ct) {
 	    switch(markinfo->mode) {
 	    case IPT_CONNMARK_SET:
-		ct->mark = markinfo->mark;
+		newmark = (ct->mark & ~markinfo->mask) | markinfo->mark;
+		if (newmark != ct->mark)
+		    ct->mark = newmark;
 		break;
 	    case IPT_CONNMARK_SAVE:
-		ct->mark = (*pskb)->nfmark;
+		newmark = (ct->mark & ~markinfo->mask) | ((*pskb)->nfmark & markinfo->mask);
+		if (ct->mark != newmark)
+		    ct->mark = newmark;
 		break;
 	    case IPT_CONNMARK_RESTORE:
-		if (ct->mark != (*pskb)->nfmark) {
-		    (*pskb)->nfmark = ct->mark;
+		nfmark = (*pskb)->nfmark;
+		diff = (ct->mark ^ nfmark & markinfo->mask);
+		if (diff != 0) {
+		    (*pskb)->nfmark = nfmark ^ diff;
 		    (*pskb)->nfcache |= NFC_ALTERED;
 		}
 		break;
@@ -45,9 +75,9 @@ target(struct sk_buff **pskb,
 static int
 checkentry(const char *tablename,
 	   const struct ipt_entry *e,
-           void *targinfo,
-           unsigned int targinfosize,
-           unsigned int hook_mask)
+	   void *targinfo,
+	   unsigned int targinfosize,
+	   unsigned int hook_mask)
 {
 	struct ipt_connmark_target_info *matchinfo = targinfo;
 	if (targinfosize != IPT_ALIGN(sizeof(struct ipt_connmark_target_info))) {
@@ -67,15 +97,16 @@ checkentry(const char *tablename,
 	return 1;
 }
 
-static struct ipt_target ipt_connmark_reg
-= { { NULL, NULL }, "CONNMARK", target, checkentry, NULL, THIS_MODULE };
+static struct ipt_target ipt_connmark_reg = {
+	.name = "CONNMARK",
+	.target = &target,
+	.checkentry = &checkentry,
+	.me = THIS_MODULE
+};
 
 static int __init init(void)
 {
-	if (ipt_register_target(&ipt_connmark_reg))
-		return -EINVAL;
-
-	return 0;
+	return ipt_register_target(&ipt_connmark_reg);
 }
 
 static void __exit fini(void)
Index: CONNMARK//linux/net/ipv4/netfilter/ipt_connmark.c
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/linux/net/ipv4/netfilter/ipt_connmark.c,v
retrieving revision 1.1
diff -u -p -r1.1 ipt_connmark.c
--- CONNMARK//linux/net/ipv4/netfilter/ipt_connmark.c	18 Dec 2003 18:47:53 -0000	1.1
+++ CONNMARK//linux/net/ipv4/netfilter/ipt_connmark.c	18 Jan 2004 17:04:01 -0000
@@ -1,7 +1,31 @@
-/* Kernel module to match connection mark values. */
+/* This kernel module matches connection mark values set by the
+ * CONNMARK target
+ *
+ * Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
 #include <linux/module.h>
 #include <linux/skbuff.h>
 
+MODULE_AUTHOR("Henrik Nordstrom <hno@marasytems.com>");
+MODULE_DESCRIPTION("IP tables connmark match module");
+MODULE_LICENSE("GPL");
+
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv4/ipt_connmark.h>
 #include <linux/netfilter_ipv4/ip_conntrack.h>
@@ -27,10 +51,10 @@ match(const struct sk_buff *skb,
 
 static int
 checkentry(const char *tablename,
-           const struct ipt_ip *ip,
-           void *matchinfo,
-           unsigned int matchsize,
-           unsigned int hook_mask)
+	   const struct ipt_ip *ip,
+	   void *matchinfo,
+	   unsigned int matchsize,
+	   unsigned int hook_mask)
 {
 	if (matchsize != IPT_ALIGN(sizeof(struct ipt_connmark_info)))
 		return 0;
@@ -38,8 +62,12 @@ checkentry(const char *tablename,
 	return 1;
 }
 
-static struct ipt_match connmark_match
-= { { NULL, NULL }, "connmark", &match, &checkentry, NULL, THIS_MODULE };
+static struct ipt_match connmark_match = {
+	.name = "connmark",
+	.match = &match,
+	.checkentry = &checkentry,
+	.me = THIS_MODULE
+};
 
 static int __init init(void)
 {

[-- Attachment #4: Type: TEXT/PLAIN, Size: 11863 bytes --]

Index: iptables.8
===================================================================
RCS file: /cvspublic/iptables/iptables.8,v
retrieving revision 1.38
diff -u -p -r1.38 iptables.8
--- iptables.8	29 Jun 2003 18:00:08 -0000	1.38
+++ iptables.8	18 Jan 2004 17:03:14 -0000
@@ -382,6 +382,16 @@ to invert the sense of the match.
 This module matches the SPIs in AH header of IPSec packets.
 .TP
 .BR "--ahspi " "[!] \fIspi\fP[:\fIspi\fP]"
+.SS connmark
+This module matches the netfilter mark field associated with a connection
+(which can be set using the
+.B CONNMARK
+target below).
+.TP
+.BI "--mark " "value[/mask]"
+Matches packets in connections with the given mark value (if a mask is
+specified, this is logically ANDed with the mark before the
+comparison).
 .SS conntrack
 This module, when combined with connection tracking, allows access to
 more connection tracking information than the "state" match.
@@ -713,6 +723,23 @@ malformed or unusual.  This is regarded 
 .SH TARGET EXTENSIONS
 iptables can use extended target modules: the following are included
 in the standard distribution.
+.SS CONNMARK
+This is used to set the netfilter mark value associated with the
+connection
+.TP
+.B --set-mark mark[/mask]
+Set connection mark. If a mask is specified then only those bits set in the
+mask is modified.
+.TP
+.B --save-mark [--mask mask]
+Copy the netfilter packet mark value to the connection mark. If a mask
+is specified then only those bits are copied.
+.TP
+.B --restore-mark [--mask mask]
+Copy the connection mark value to the packet. If a mask is specified
+then only those bits are copied. This is only valid in the
+.B mangle
+table.
 .SS DNAT
 This target is only valid in the
 .B nat
Index: extensions/libipt_connmark.c
===================================================================
RCS file: /cvspublic/iptables/extensions/libipt_connmark.c,v
retrieving revision 1.5
diff -u -p -r1.5 libipt_connmark.c
--- extensions/libipt_connmark.c	20 Sep 2002 15:25:13 -0000	1.5
+++ extensions/libipt_connmark.c	18 Jan 2004 17:03:14 -0000
@@ -1,4 +1,24 @@
-/* Shared library add-on to iptables to add CONNMARK matching support. */
+/* Shared library add-on to iptables to add connmark matching support.
+ *
+ * (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * Version 1.1
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
 #include <stdio.h>
 #include <netdb.h>
 #include <string.h>
@@ -50,7 +70,7 @@ parse(int c, char **argv, int invert, un
 		if (*end == '/') {
 			markinfo->mask = strtoul(end+1, &end, 0);
 		} else
-			markinfo->mask = 0xffffffff;
+			markinfo->mask = ~0;
 		if (*end != '\0' || end == optarg)
 			exit_error(PARAMETER_PROBLEM, "Bad MARK value `%s'", optarg);
 		if (invert)
@@ -67,7 +87,7 @@ parse(int c, char **argv, int invert, un
 static void
 print_mark(unsigned long mark, unsigned long mask, int numeric)
 {
-	if(mask != 0xffffffff)
+	if(mask != ~0)
 		printf("0x%lx/0x%lx ", mark, mask);
 	else
 		printf("0x%lx ", mark);
@@ -96,7 +116,7 @@ print(const struct ipt_ip *ip,
 	print_mark(info->mark, info->mask, numeric);
 }
 
-/* Saves the union ipt_matchinfo in parsable form to stdout. */
+/* Saves the matchinfo in parsable form to stdout. */
 static void
 save(const struct ipt_ip *ip, const struct ipt_entry_match *match)
 {
@@ -109,23 +129,21 @@ save(const struct ipt_ip *ip, const stru
 	print_mark(info->mark, info->mask, 0);
 }
 
-static
-struct iptables_match mark
-= { NULL,
-    "connmark",
-    IPTABLES_VERSION,
-    IPT_ALIGN(sizeof(struct ipt_connmark_info)),
-    IPT_ALIGN(sizeof(struct ipt_connmark_info)),
-    &help,
-    &init,
-    &parse,
-    &final_check,
-    &print,
-    &save,
-    opts
+static struct iptables_match connmark_match = {
+    .name          = "connmark",
+    .version       = IPTABLES_VERSION,
+    .size          = IPT_ALIGN(sizeof(struct ipt_connmark_info)),
+    .userspacesize = IPT_ALIGN(sizeof(struct ipt_connmark_info)),
+    .help          = &help,
+    .init          = &init,
+    .parse         = &parse,
+    .final_check   = &final_check,
+    .print         = &print,
+    .save          = &save,
+    .extra_opts    = opts
 };
 
 void _init(void)
 {
-	register_match(&mark);
+	register_match(&connmark_match);
 }
Index: extensions/libipt_CONNMARK.c
===================================================================
RCS file: /cvspublic/iptables/extensions/libipt_CONNMARK.c,v
retrieving revision 1.3
diff -u -p -r1.3 libipt_CONNMARK.c
--- extensions/libipt_CONNMARK.c	4 Jun 2002 07:46:30 -0000	1.3
+++ extensions/libipt_CONNMARK.c	18 Jan 2004 17:03:14 -0000
@@ -1,4 +1,24 @@
-/* Shared library add-on to iptables to add CONNMARK target support. */
+/* Shared library add-on to iptables to add CONNMARK target support.
+ *
+ * (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * Version 1.1
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -21,9 +41,9 @@ help(void)
 {
 	printf(
 "CONNMARK target v%s options:\n"
-"  --set-mark value              Set conntrack mark value\n"
-"  --save-mark                   Save the packet nfmark on the connection\n"
-"  --restore-mark                Restore saved nfmark value\n"
+"  --set-mark value[/mask]       Set conntrack mark value\n"
+"  --save-mark [--mask mask]     Save the packet nfmark in the connection\n"
+"  --restore-mark [--mask mask]  Restore saved nfmark value\n"
 "\n",
 IPTABLES_VERSION);
 }
@@ -32,6 +52,7 @@ static struct option opts[] = {
 	{ "set-mark", 1, 0, '1' },
 	{ "save-mark", 0, 0, '2' },
 	{ "restore-mark", 0, 0, '3' },
+	{ "mask", 1, 0, '4' },
 	{ 0 }
 };
 
@@ -55,7 +76,10 @@ parse(int c, char **argv, int invert, un
 		char *end;
 	case '1':
 		markinfo->mode = IPT_CONNMARK_SET;
+		markinfo->mask = ~0;
 		markinfo->mark = strtoul(optarg, &end, 0);
+		if (*end == '/' && end[1] != '\0')
+		    markinfo->mask = strtoul(end+1, &end, 0);
 		if (*end != '\0' || end == optarg)
 			exit_error(PARAMETER_PROBLEM, "Bad MARK value `%s'", optarg);
 		if (*flags)
@@ -65,6 +89,7 @@ parse(int c, char **argv, int invert, un
 		break;
 	case '2':
 		markinfo->mode = IPT_CONNMARK_SAVE;
+		markinfo->mask = ~0;
 		if (*flags)
 			exit_error(PARAMETER_PROBLEM,
 			           "CONNMARK target: Can't specify --save-mark twice");
@@ -72,11 +97,20 @@ parse(int c, char **argv, int invert, un
 		break;
 	case '3':
 		markinfo->mode = IPT_CONNMARK_RESTORE;
+		markinfo->mask = ~0;
 		if (*flags)
 			exit_error(PARAMETER_PROBLEM,
 			           "CONNMARK target: Can't specify --restore-mark twice");
 		*flags = 1;
 		break;
+	case '4':
+		if (!*flags)
+			exit_error(PARAMETER_PROBLEM,
+			           "CONNMARK target: Can't specify --mask without a operation");
+		markinfo->mark = strtoul(optarg, &end, 0);
+		if (*end != '\0' || end == optarg)
+			exit_error(PARAMETER_PROBLEM, "Bad MARK value `%s'", optarg);
+		break;
 	default:
 		return 0;
 	}
@@ -89,16 +123,10 @@ final_check(unsigned int flags)
 {
 	if (!flags)
 		exit_error(PARAMETER_PROBLEM,
-		           "CONNMARK target: Parameter --set-mark is required");
-}
-
-static void
-print_mark(unsigned long mark, int numeric)
-{
-	printf("0x%lx ", mark);
+		           "CONNMARK target: No operation specified");
 }
 
-/* Prints out the targinfo. */
+/* Prints out the target info. */
 static void
 print(const struct ipt_ip *ip,
       const struct ipt_entry_target *target,
@@ -108,14 +136,21 @@ print(const struct ipt_ip *ip,
 		(const struct ipt_connmark_target_info *)target->data;
 	switch (markinfo->mode) {
 	case IPT_CONNMARK_SET:
-	    printf("CONNMARK set ");
-	    print_mark(markinfo->mark, numeric);
+	    printf("CONNMARK set 0x%lx", markinfo->mark);
+	    if (markinfo->mask != ~0)
+		printf("/0x%lx", markinfo->mask);
+	    printf(" ");
 	    break;
 	case IPT_CONNMARK_SAVE:
 	    printf("CONNMARK save ");
+	    if (markinfo->mask != ~0)
+		printf("mask 0x%lx", markinfo->mask);
+	    printf(" ");
 	    break;
 	case IPT_CONNMARK_RESTORE:
 	    printf("CONNMARK restore ");
+	    if (markinfo->mask != ~0)
+		printf("mask 0x%lx", markinfo->mask);
 	    break;
 	default:
 	    printf("ERROR: UNKNOWN CONNMARK MODE ");
@@ -123,7 +158,7 @@ print(const struct ipt_ip *ip,
 	}
 }
 
-/* Saves the union ipt_targinfo in parsable form to stdout. */
+/* Saves the target into in parsable form to stdout. */
 static void
 save(const struct ipt_ip *ip, const struct ipt_entry_target *target)
 {
@@ -132,13 +167,20 @@ save(const struct ipt_ip *ip, const stru
 
 	switch (markinfo->mode) {
 	case IPT_CONNMARK_SET:
-	    printf("--set-mark 0x%lx ", markinfo->mark);
+	    printf("--set-mark 0x%lx", markinfo->mark);
+	    if (markinfo->mask != ~0)
+		printf("/0x%lx", markinfo->mask);
+	    printf(" ");
 	    break;
 	case IPT_CONNMARK_SAVE:
 	    printf("--save-mark ");
+	    if (markinfo->mask != ~0)
+		printf("--mask 0x%lx", markinfo->mask);
 	    break;
 	case IPT_CONNMARK_RESTORE:
 	    printf("--restore-mark ");
+	    if (markinfo->mask != ~0)
+		printf("--mask 0x%lx", markinfo->mask);
 	    break;
 	default:
 	    printf("ERROR: UNKNOWN CONNMARK MODE ");
@@ -146,23 +188,21 @@ save(const struct ipt_ip *ip, const stru
 	}
 }
 
-static
-struct iptables_target mark
-= { NULL,
-    "CONNMARK",
-    IPTABLES_VERSION,
-    IPT_ALIGN(sizeof(struct ipt_connmark_target_info)),
-    IPT_ALIGN(sizeof(struct ipt_connmark_target_info)),
-    &help,
-    &init,
-    &parse,
-    &final_check,
-    &print,
-    &save,
-    opts
+static struct iptables_target connmark_target = {
+    .name          = "CONNMARK",
+    .version       = IPTABLES_VERSION,
+    .size          = IPT_ALIGN(sizeof(struct ipt_connmark_target_info)),
+    .userspacesize = IPT_ALIGN(sizeof(struct ipt_connmark_target_info)),
+    .help          = &help,
+    .init          = &init,
+    .parse         = &parse,
+    .final_check   = &final_check,
+    .print         = &print,
+    .save          = &save,
+    .extra_opts    = opts
 };
 
 void _init(void)
 {
-	register_target(&mark);
+	register_target(&connmark_target);
 }

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Herve Eychenne]

On Sun, Jan 18, 2004 at 06:16:50PM +0100, Henrik Nordstrom wrote:

> On Sun, 18 Jan 2004, Harald Welte wrote:

> > yup. Please submit your changes against pom-ng (and patch-o-matic if you
> > want, but as of now I don't think we'll see another pom-old release).

> Attached you find 

> iptables userspace patch
> - manpage additions

Well... you were faster than me on this one.

> > well, we could do some semi-automated mechanism, like splitting it up in
> > one file per extension, and then concatenate the whole manpage from
> > those snippets depending on PF_EXT_LIBS / PF6_EXT_LIBS during the
> > iptables userspace make process.

> Good idea. Should be pretty trivial except for the sorting (may need to
> fight a little with locale)

Anyway, ideally, there should be only two descriptions of the
functionnality of each targets: a short one (as the one included in
iptables userspace modules, available through --help), and a longer one,
describing it exhaustively.
The manpage (and why not a more complete document like a howto, or
even a web page describing netfilter modules) should of course be generated
from the second (longest) one.
So there shouldn't be any need to include too much formatting "tags" in
the original text (describing the module functionnalites), and those should
be turned into the adequate tags if needed (for target languages like
XML, HTML...).

Now, several thoughts:

1) AFA the manpage is concerned, everyone seems to agree that p-o-m
  modules descriptions should not be included into the mainstream manpage.
  I agree with that.

2) p-o-m modules regularly tend to be included into mainstream
  kernels, and each new netfilter module description should then
  state that it is only valid as for a certain kernel version (because
  otherwise it would confuse people using older kernels).
  Even if this kind of information will tend to be more annoying over
  time, we can now raise the question of using the fact that
  "the iptables manpage will now be generated dynamically" in order to 
  generate the manpage according to the currently available netfilter
  modules.
3) However, I think it would be a bad idea to have the iptables manpage
  depend on what is currently installed at the time of its generation:
  this would introduce incompatibilities between manpages over
  different hosts, and people are not used to that (at least for
  manpages).
  Moreover, a kernel upgrade would require that the manpage gets
  rebuilded, which is impossible to ensure.
4) Yet, it would be interesting for people to have a way to know
  exactly which modules are currently usable on the system they have
  (and we have one on recent kernels, with /proc/*/*_{matches,targets}
  listings), and _how_ to use it
5) If we agree that the last point should be made possible, there
  should still be a way to know what is sure (coming from mainstream),
  and what is less (coming from p-o-m)

6) So should these descriptions be included in a separate "p-o-m
  extensions" manpage? It doubt it should.
  Why? The previous thoughts lead me to the conclusion they should
  not be contained in a manpage (I hardly believe that I should
  say that as the iptables manpage author ;-). And that's because
  manpages, static by essence (even if generated once) are not the
  place where ever changing things as netfilter modules (changing
  with kernels, and with a cvs p-o-m) should lie down.

That is why I'd rather limit the iptables manpage to pure iptables
syntax, and let a program display matches/target use according to the
user's choices:
- mainstream and/or coming from p-o-m
- usable on the current system and/or existing (even if not installed)

Please note that nothing would prevent from generating a summary document
(taking the form of a manpage or other) with all the matches/targets.

What should be done in that sense:
- Ensuring that module description is accurate after a kernel upgrade
  (possibly modified with p-o-m) (point 3.) would require that
  full descriptions are stored somewhere on the filesystem.
- Knowing which module is available where (points 2. and 5.) probably
  requires versioning of module descriptions (if not module version at
  all).
- Knowing if modules come from mainstream or p-o-m (point 5.) would
  require to include in binary modules the "comes from mainline or
  p-o-m" information. We could even think of more fine-tuned
  information bits, telling if module is considered stable or not
  (even if coming from patch-o-matic, submitted p-o-m modules can be
  considered stable, right?... and extra is more unstable than base,
  isn't it?)
- Writing the program itself (in Perl, probably).

Any comments?

 Herve

-- 
 _
(°=  Hervé Eychenne
//)
v_/_ WallFire project:  http://www.wallfire.org/

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

On Tue, 20 Jan 2004, Herve Eychenne wrote:

> 3) However, I think it would be a bad idea to have the iptables manpage
>   depend on what is currently installed at the time of its generation:
>   this would introduce incompatibilities between manpages over
>   different hosts, and people are not used to that (at least for
>   manpages).

To this I do not agree, or at least not fully. This way the man page 
documents what this specific version of the iptables user program 
supports. This may or may not be the same as the current running kernel.

It is impossible for the man page to correctly reflect all possible 
combinations.

If a certain feature is not supported by the current running kernel then 
this is found out pretty quickly. Unfortunately the error is not the most 
forgiving and this can be considered a major bug in the iptables command.

After some thinking I think what should be done is actually to do what we
have done in the Squid project, always include the documentation for
everything but if the feature is not currently enabled in the build
automatically add a comment explaining this. My personal preference for 
this type of reference information is man pages, and I do not doubt many 
shares this view.

With the loose coupling between p-o-m and the iptables userspace I do not 
see it feasible to document in the userspace documentation which 
matches/targets are from p-o-m or not, but on the other hand if all 
documentation is always included but with comments then most people will 
learn quickly what is mainstream and what is not, and if in doubt a quick 
look into the man page on your system will tell what you have.

I see a big merit in the iptables man page actually documenting the 
extensions available. And if done correcly this can be almost self 
maintaining.

Ideally this would be derived automatically from the C source of the
extension to avoid having to maintain so many files but that is a
substantially larger reorganisation than the split manpage.

This is not really the same kind of information you want to put into the
kernel help, or in the command help output, and it is not exacly howto 
material either.

For the userspace documentation to be complete there has to in my 
opinion be

* Reference documentation, explaining the function of each option. (man
page). Not having not yet submitted extensions in the reference
documentation is a major loss in the quality of the reference information
in my opinion, and also discourages the information from being maintained
making things even worse.

* Terse usage information for --help, acting no more than a reminder on 
the syntax.

* A howto explaining/discussing in more depth why/when/how certain matches
should be used, what p-o-m is, and how to use things from p-o-m. (the
extensions howto). This does not need to include the reference material
and can refer to the reference documentation for further details.

Regards
Henrik

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Herve Eychenne]

On Tue, Jan 20, 2004 at 12:48:29AM +0100, Henrik Nordstrom wrote:

> On Tue, 20 Jan 2004, Herve Eychenne wrote:

> > 3) However, I think it would be a bad idea to have the iptables manpage
> >   depend on what is currently installed at the time of its generation:
> >   this would introduce incompatibilities between manpages over
> >   different hosts, and people are not used to that (at least for
> >   manpages).

> To this I do not agree, or at least not fully. This way the man page 
> documents what this specific version of the iptables user program 
> supports.

The problem is not only about user program, but user/kernel
combination.

> This may or may not be the same as the current running kernel.

Yes, and that's what it would be good to take such (kernel, here) changes
into account.

> It is impossible for the man page to correctly reflect all possible 
> combinations.

Yes, as it is always some kind of "static".
You'll note that the reason why I prefer a program to a "static" (even
if generated at iptables compile time) manpage is that the manpage will
not reflect your current combination (at least durably), again.

> If a certain feature is not supported by the current running kernel then 
> this is found out pretty quickly.

Usually, in such a case, most users get an obscur error and write to the
mailing-list. Pretty clear and quick, isn't it? ;-)

> Unfortunately the error is not the most 
> forgiving and this can be considered a major bug in the iptables command.

Indeed... and I think it will be hard to change that for iptables...
So a documentation effort seems much more affordable to me...

> After some thinking I think what should be done is actually to do what we
> have done in the Squid project, always include the documentation for
> everything

Please note that I do not suggest this shouldn't be done. If we write
the program I'm talking about, it will be perfectly feasible (and
it should be done) to generate a document (manpage, or other)
documented every existing module.

> but if the feature is not currently enabled in the build
> automatically add a comment explaining this. My personal preference for 
> this type of reference information is man pages, and I do not doubt many 
> shares this view.

That doesn't prevent from providing a program which generates a
manpage dynamically (if you really want it in a manpage format)...

> With the loose coupling between p-o-m and the iptables userspace I do not 
> see it feasible to document in the userspace documentation
> which matches/targets are from p-o-m or not,

If you have a static document, clearly not. But that's not what I'm
talking about.

> but on the other hand if all
> documentation is always included but with comments then most people will 
> learn quickly what is mainstream and what is not,

Yes. That state is not always that clear. For example, a feature could belong to
p-o-m for a certain kernel version, and mainstream for kernel version + 1.
But! You (netfilter developer) perfectly know what this is all about.
Everyone is not that informed. I personnaly know some people who are
quite confused by these incertitudes.
People in real life (read "not developer life") often upgrade their
kernel and iptables command via packages, and if they get to know the
existence of p-o-m, they have only little time to keep tracking down if
their favorite p-o-m modules have been included into mainstream or removed
(from kernel, or cvs).

> and if in doubt a quick 
> look into the man page on your system will tell what you have.

I do not understand. A manpage with every extensions will only tell
you what you could have, and you would have to experiment quite much
(with uninformative error messages) to know what you can really _use_.
And this is all to be done again when you upgrade the kernel...
Sigh...
Besides, this situation is a nightmare for automatic programs (upper
layers) which try to take advantage of a maximum of extensions.

> Ideally this would be derived automatically from the C source of the
> extension to avoid having to maintain so many files but that is a
> substantially larger reorganisation than the split manpage.

Why not... as long as the information is stored somewhere...

> This is not really the same kind of information you want to put into the
> kernel help,

No, this is not, but information about kernel configuration is not
exactly of the same nature.

> or in the command help output, and it is not exacly howto 
> material either.

You are right, but I didn't recommend using a single description for
all these things, which may be different. It was only about
factorizing as much as we can (if we can).

> For the userspace documentation to be complete there has to in my 
> opinion be

> * Reference documentation, explaining the function of each option. (man
> page). Not having not yet submitted extensions in the reference
> documentation is a major loss in the quality of the reference information
> in my opinion,

The discussion was not about that, am I wrong? I think everyone agrees
that p-o-m extensions should be documented somewhere, where it would
be easily accessible, which is not the case for the moment.
The discussion was about how documenting everything in the clearest,
the most complete and accurate (according to what you really have at
runtime) way.

> and also discourages the information from being maintained
> making things even worse.

I want exactly the same thing: factorize, and generate things
dynamically to reduce maintainance as much as possible...

> * Terse usage information for --help, acting no more than a reminder on 
> the syntax.

Isn't it what we have currently? These messages seem perfect to me as
they are today. But we can discuss what is the best way to
store/generate it to minimize redundancy (and so maintainance),
although that's not a priority for me, as we could always factorize
what we can afterwards.

> * A howto explaining/discussing in more depth why/when/how certain matches
> should be used, what p-o-m is, and how to use things from p-o-m. (the
> extensions howto). This does not need to include the reference material
> and can refer to the reference documentation for further details.

Yes. Such a document doesn't really exist yet, although it would be
very useful. But it's orthogonal to the discussion, as it is an
"editorial" document, that cannot be generated by any tool (which
was the original subject of this thread, right?).

Maybe I should read your email one more time, but I don't see how it
contradicts my points, even if we seem to reach a completely opposite
conclusion considering manpage generation. ;-)

 Herve

-- 
 _
(°=  Hervé Eychenne
//)
v_/_ WallFire project:  http://www.wallfire.org/

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

On Tue, 20 Jan 2004, Herve Eychenne wrote:

> Yes, as it is always some kind of "static".
> You'll note that the reason why I prefer a program to a "static" (even
> if generated at iptables compile time) manpage is that the manpage will
> not reflect your current combination (at least durably), again.

But with iptables the best level or program is the same as you get with 
the proposed patch, the manpage reflects what your userspace binary 
supports, independent of what the kernel supports.

This for the same reasons as the error when trying to use a target/match 
not supported gives an obscure error message.

Actually, fixing the error message is not hard, but it requires some sligt 
additions to the kernel to allow querying if a given target/match is 
supported so users of old kernels will have to live with the obscure 
error even after the problem has been addressed.

> Please note that I do not suggest this shouldn't be done. If we write
> the program I'm talking about, it will be perfectly feasible (and
> it should be done) to generate a document (manpage, or other)
> documented every existing module.

What I don't get is what this program approach gives which the 
at-compiletime build of the manpage does not.

> That doesn't prevent from providing a program which generates a
> manpage dynamically (if you really want it in a manpage format)...

Not easily in a way which allows the manpage to be packaged I think. In 
addition it does not solve the problem of userspace/kernel mismatches (see 
below).

> Yes. That state is not always that clear. For example, a feature could belong to
> p-o-m for a certain kernel version, and mainstream for kernel version + 1.
> But! You (netfilter developer) perfectly know what this is all about.

So then we should try to have this better documented than it is today.  
Mentioning p-o-m in the iptables documentation would be a start..

> People in real life (read "not developer life") often upgrade their
> kernel and iptables command via packages, and if they get to know the
> existence of p-o-m, they have only little time to keep tracking down if
> their favorite p-o-m modules have been included into mainstream or removed
> (from kernel, or cvs).

Yes, and this is also assumed by me.

What the above means is that the average user will always have a manpage 
which correctly reflects the capabilities of the iptables package he has 
been provided by his distributor, including any p-o-m patches the 
distributor may have selected to be part of their distribution.

> I do not understand. A manpage with every extensions will only tell
> you what you could have, and you would have to experiment quite much
> (with uninformative error messages) to know what you can really _use_.

What I propose is a manpage with

a) Every extension which is known to your userspace binary (i.e. the 
extensions which was enabled in the kernel the binary is built for)

b) In addition, the documentation for the extensions (where such
documentation exists) you could have with documentation on how you get
these from p-o-m if you need them. This could be in the same page or a
different page if having them in the same page may confuse people (I think 
not, but I am not the average user)

Not a dumb page where all extensions is documented as if they were 
supported on the users system.

> And this is all to be done again when you upgrade the kernel...

Yes, and the iptables binary needs to be rebuilt as well to add binary 
support for the added/changed matches/targets. This is the current state 
of affairs anyway.

> > * A howto explaining/discussing in more depth why/when/how certain matches
> > should be used, what p-o-m is, and how to use things from p-o-m. (the
> > extensions howto). This does not need to include the reference material
> > and can refer to the reference documentation for further details.
> 
> Yes. Such a document doesn't really exist yet, although it would be
> very useful. But it's orthogonal to the discussion, as it is an
> "editorial" document, that cannot be generated by any tool (which
> was the original subject of this thread, right?).

The iptables extensions HOWTO gets quite far on this, but is plauged 
somewhat by the same issues as discussed.

Regards
Henrik

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

On Tue, 20 Jan 2004, Henrik Nordstrom wrote:

> What the above means is that the average user will always have a manpage 
> which correctly reflects the capabilities of the iptables package he has 
> been provided by his distributor, including any p-o-m patches the 
> distributor may have selected to be part of their distribution.

Correction: What my proposed patch gives ..

Regards
Henrik

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Herve Eychenne]

On Tue, Jan 20, 2004 at 08:05:45AM +0100, Henrik Nordstrom wrote:

> On Tue, 20 Jan 2004, Herve Eychenne wrote:

> > Yes, as it is always some kind of "static".
> > You'll note that the reason why I prefer a program to a "static" (even
> > if generated at iptables compile time) manpage is that the manpage will
> > not reflect your current combination (at least durably), again.

> But with iptables the best level or program is the same as you get with 
> the proposed patch, the manpage reflects what your userspace binary 
> supports, independent of what the kernel supports.

Yes, and that is another question I want to raise...
I don't really understand why p-o-m would not add both parts (kernel/user).
That would mean not distributing an iptables tarball with every
extensions. According to me, adding an extension should be treated
equally on user and kernel sides (as they are heavily coupled).

> Actually, fixing the error message is not hard, but it requires some sligt 
> additions to the kernel to allow querying if a given target/match is 
> supported so users of old kernels will have to live with the obscure 
> error even after the problem has been addressed.

Why isn't this done already? ;-)

> > Please note that I do not suggest this shouldn't be done. If we write
> > the program I'm talking about, it will be perfectly feasible (and
> > it should be done) to generate a document (manpage, or other)
> > documented every existing module.

> What I don't get is what this program approach gives which the 
> at-compiletime build of the manpage does not.

If you think of the current scheme, where you're forced to recompile
userspace after having used patch-o-matic, not so much: filtering
capabilities... (mainstream or not, match or target, specific version
or all, currently available or not, etc.). 
A way to know all that is available in every kind of context is needed
anyway, independantly of the documentation, anyway.

> > That doesn't prevent from providing a program which generates a
> > manpage dynamically (if you really want it in a manpage format)...

> Not easily in a way which allows the manpage to be packaged I think.

Packaged in the sense of a distribution package?
Well... the "extension" manpage would simply not be part of the
package itself (which is static, by essence)... but the package would
include the doc (manpage, if we want) generator.

> In addition it does not solve the problem of userspace/kernel mismatches (see 
> below).

> > Yes. That state is not always that clear. For example, a feature could belong to
> > p-o-m for a certain kernel version, and mainstream for kernel version + 1.
> > But! You (netfilter developer) perfectly know what this is all about.

> So then we should try to have this better documented than it is today.  
> Mentioning p-o-m in the iptables documentation would be a start..

p-o-m is already mentionned, but in the netfilter extensions HOWTO, I
think. It is not mentionned in the manpage, but iptables manpage is
about userspace anyway. And that has been a moment since I think that
there really should be a manpage about userspace (iptables), a manpage
about kernel (there are some things to tell, and I have started one,
which I'll sent in a few days), and one about extensions.

> > I do not understand. A manpage with every extensions will only tell
> > you what you could have, and you would have to experiment quite much
> > (with uninformative error messages) to know what you can really _use_.

> What I propose is a manpage with

> a) Every extension which is known to your userspace binary (i.e. the 
> extensions which was enabled in the kernel the binary is built for)

> b) In addition, the documentation for the extensions (where such
> documentation exists) you could have with documentation on how you get
> these from p-o-m if you need them. This could be in the same page or a
> different page if having them in the same page may confuse people (I think 
> not, but I am not the average user)

> Not a dumb page where all extensions is documented as if they were 
> supported on the users system.

If we really want a userspace-compile-time-built manpage, then yes,
probably.

> > And this is all to be done again when you upgrade the kernel...

> Yes, and the iptables binary needs to be rebuilt as well to add binary 
> support for the added/changed matches/targets. This is the current state 
> of affairs anyway.

Yes, but that should be different. There's unfortunately no easy way
to have a good module management system (fast adding, no recompilation
needed, no binary interface problems) for iptables. :-(  (pkttables
will be the place to do that, as we discussed during the workshop)

 Herve

-- 
 _
(°=  Hervé Eychenne
//)
v_/_ WallFire project:  http://www.wallfire.org/

Buffer size

[XiChimos]

I was looking at the code for a userspace application and noticed that 
the buffer size was 2048.  Why is it this value if the maximum size of a 
IP datagram is 65535?  Does it get fragmented after the point of 2048 
octects?  Finally, will using 2048 ever get me into trouble?

Chris

Re: Buffer size

[Henrik Nordstrom]

On Tue, 20 Jan 2004, XiChimos wrote:

> I was looking at the code for a userspace application and noticed that 
> the buffer size was 2048.

What kind of userspace application?

Regards
Henrik

Re: Buffer size

[XiChimos]

The application adds an IP option to every TCP packet that it sees.

Henrik Nordstrom wrote:

>On Tue, 20 Jan 2004, XiChimos wrote:
>
>  
>
>>I was looking at the code for a userspace application and noticed that 
>>the buffer size was 2048.
>>    
>>
>
>What kind of userspace application?
>
>Regards
>Henrik
>  
>

Re: Buffer size

[Henrik Nordstrom]

What buffer are you referring to?

Regars
Henrik

On Tue, 20 Jan 2004, XiChimos wrote:

> The application adds an IP option to every TCP packet that it sees.
> 
> Henrik Nordstrom wrote:
> 
> >On Tue, 20 Jan 2004, XiChimos wrote:
> >
> >  
> >
> >>I was looking at the code for a userspace application and noticed that 
> >>the buffer size was 2048.
> >>    
> >>
> >
> >What kind of userspace application?
> >
> >Regards
> >Henrik
> >  
> >
> 
> 

Re: Buffer size

[XiChimos]

Well, basically one that contains the payload and IP header, which is 
returned by ipq_read.  Basically my question is if I am casting the 
payload of the ipq packet msg structure into a data-type, what is the 
maximum size that this type should be?  Or in other words, what is the 
largest value of the ipq packet msg structure's data length field?  I 
thought it should be 2^16 bytes, but when I saw that in the example code 
they had ipq_read only needing a buffer of size 2046 bytes, I started to 
question myself.

Henrik Nordstrom wrote:

>What buffer are you referring to?
>
>Regars
>Henrik
>
>On Tue, 20 Jan 2004, XiChimos wrote:
>
>  
>
>>The application adds an IP option to every TCP packet that it sees.
>>
>>Henrik Nordstrom wrote:
>>
>>    
>>
>>>On Tue, 20 Jan 2004, XiChimos wrote:
>>>
>>> 
>>>
>>>      
>>>
>>>>I was looking at the code for a userspace application and noticed that 
>>>>the buffer size was 2048.
>>>>   
>>>>
>>>>        
>>>>
>>>What kind of userspace application?
>>>
>>>Regards
>>>Henrik
>>> 
>>>
>>>      
>>>
>>    
>>

Re: Buffer size

[Henrik Nordstrom]

On Tue, 20 Jan 2004, XiChimos wrote:

> Well, basically one that contains the payload and IP header, which is 
> returned by ipq_read.  Basically my question is if I am casting the 
> payload of the ipq packet msg structure into a data-type, what is the 
> maximum size that this type should be?

The largest packet including payload you will be receiving. I think that
if you specify a smaller buffer then the pakets will be truncated which
may be acceptable if the application is only inspecting the headers and
beginning of protocol payload, but if you want to do packet modification
then you need the whole packet.

Regards
Henrik

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

On Wed, 21 Jan 2004, Herve Eychenne wrote:

> Why isn't this done already? ;-)

Because we are all programmers on this list and we never type wrongly?

More likely because we always think on how to solve the whole problem 
instead rather than solving the most obvious shortcomings. To solve the 
whole problem includes versioning and many other fancy features which is 
way beyond the scope of what can be done in 2.4/2.6 lifetime due to their 
freezed nature. To just solve the issue where a non-existing match/target 
is used requires far less.

> If you think of the current scheme, where you're forced to recompile
> userspace after having used patch-o-matic, not so much: filtering
> capabilities... (mainstream or not, match or target, specific version
> or all, currently available or not, etc.). 
> A way to know all that is available in every kind of context is needed
> anyway, independantly of the documentation, anyway.

To me the only two contexts really interesting is

a) What is supported on this system

b) What may be added if you need it.

all other contexts can be derived from there.

> > Not easily in a way which allows the manpage to be packaged I think.
> 
> Packaged in the sense of a distribution package?

Yes, which is what the majority of the users where one has to care about 
p-o-m/not p-o-m use.

Most users rolling their own binaries understand that both kernel and 
userspace may need upgradign when adding a previously unsupported item so 
I do not regard them as a problem in this discussion.

> If we really want a userspace-compile-time-built manpage, then yes,
> probably.

I defenitely vote that there should be a reference manpage covering the 
iptables extensions, in case there is any doubt on what my 
preference is ;-)

Being a long-timer UNIX guy I don't really like HOWTO style documents for
reference. But it is not what they are ment to be used for either. Man
pages does a excellent job for that purpose, and the iptables man page is
not bad at all it just lags behind a lot because extensions maintainers
don't care about the man page and this is what should be addressed first. 
Once documentation is structured in a way that allows it to be maintained 
in a sane manner it is easy to improve on the scheme if so is needed.

Today trying to make a scheme which solves the question on how the 
iptables/kernel should be kept in synch, documentation reflecting 100% 
correctly the status of different extensions etc is in my opinion trying 
to grasp for to much and not likely to succeed.

> Yes, but that should be different. There's unfortunately no easy way
> to have a good module management system (fast adding, no recompilation
> needed, no binary interface problems) for iptables. :-(  (pkttables
> will be the place to do that, as we discussed during the workshop)

With pkttables being rather far away at the moment and very far away for
mainstream, lets try to focus on how to solve the documentation question
for iptables. Any improvements done in iptables then sets the standards
the same documentation for pkttables should live up to when it's time.


Making close relation between p-o-m and the userspace is in my opinion a 
big mistake and creates a big nest of maintain issues. The scheme where 
the iptables userspace has to be built matching your kernel works 
reasonably well in my opinion.

If it was the case that we had all of iptables (kernel+user) in one single 
package with no real dependencies on the kernel version used then the 
situation would be different, but this is not how things work and 
everything moves in the opposite direction.

Regards
Henrik

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 2384 bytes --]

On Wed, Jan 21, 2004 at 12:21:03AM +0100, Herve Eychenne wrote:

> Yes, and that is another question I want to raise...
> I don't really understand why p-o-m would not add both parts (kernel/user).
> That would mean not distributing an iptables tarball with every
> extensions. According to me, adding an extension should be treated
> equally on user and kernel sides (as they are heavily coupled).

the issue is, that quite an amount of patch-o-matic kernel code
eventually appears in the mainstream kernel at some point.  For those
extensions it's better if their userspace counterparts are already
present in already-shipped iptables packages.

> Why isn't this done already? ;-)

As usual: Because nobody sent the respective patches.

> Well... the "extension" manpage would simply not be part of the
> package itself (which is static, by essence)... but the package would
> include the doc (manpage, if we want) generator.

No, this is not going to be accepted (by the distributions/vendors).

> > So then we should try to have this better documented than it is today.  
> > Mentioning p-o-m in the iptables documentation would be a start..
> 
> p-o-m is already mentionned, but in the netfilter extensions HOWTO, I
> think. It is not mentionned in the manpage, but iptables manpage is
> about userspace anyway. And that has been a moment since I think that
> there really should be a manpage about userspace (iptables), a manpage
> about kernel (there are some things to tell, and I have started one,
> which I'll sent in a few days), and one about extensions.

Please let's not replicate too much information that already exists in
some HOWTO inside of the manpages.  This results in a maintainance
nightmare, since every piece of documentation has to be updates once
something changes. 

I'd rather add the howto's to the iptables tar.gz, so the vendors can
put them in /usr/share/doc.  The manpage would then just point to them.

>  Herve
> (°=  Hervé Eychenne
-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 3556 bytes --]

On Tue, Jan 20, 2004 at 02:13:05AM +0100, Herve Eychenne wrote:
> > It is impossible for the man page to correctly reflect all possible 
> > combinations.
> 
> Yes, as it is always some kind of "static".
> You'll note that the reason why I prefer a program to a "static" (even
> if generated at iptables compile time) manpage is that the manpage will
> not reflect your current combination (at least durably), again.

I don't think that we should spend too much time developing such a
program for iptables.  I'd rather move any big new (sub-)projects or
changes to 2.7/pkttables.  (Yes, there's now some movement again... look
at my cvs commits/changes in the libpkttables directory).

However, if somebody wants to take the full job of 
- adding the respective information to the sourcecode
- generating netfilter-extension-HOWTO snippets in docbook XML
- generating the respective manpage snippets 

then I wouldn't refuse applying the patch.

My position on this whole discussion is:

'man iptables' should reflect what the iptables userspace command has
built in.  So making the manpage dependent on what extensions are
compiled at iptables compile time is fine with me.

And yes, I prefer the 'squid' way, listing all matches/targets and their
options, but mark unavailable ones with a comment.  This comment could
refer to a special section at the bottom of the page, where we list
possible causes why it is disabled, and which steps one might need to
take to make it available.

> Usually, in such a case, most users get an obscur error and write to the
> mailing-list. Pretty clear and quick, isn't it? ;-)

we have no more verbose error reporting due to the getsockopt/setsockopt
interface.  I don't see a way how this could change :(

> People in real life (read "not developer life") often upgrade their
> kernel and iptables command via packages, and if they get to know the
> existence of p-o-m, they have only little time to keep tracking down if
> their favorite p-o-m modules have been included into mainstream or removed
> (from kernel, or cvs).

If they only use vendor packages, it's fine.  The vendor would compile
iptables against the matching kernel sources and thus end up with a
manapge that either
a) lists only the available matches/targets
b) lists all matches/targets and marks unavailable ones as unavailable

if they use their own kernel, they shoud also use their own iptables or
otherwise expect some inconsistency in the documentation.

> I do not understand. A manpage with every extensions will only tell
> you what you could have, and you would have to experiment quite much
> (with uninformative error messages) to know what you can really _use_.

As long as it makes a comment about what is available/unavailable in the 
iptables userspace program you are currently running, I see this as an
advantage.

> And this is all to be done again when you upgrade the kernel...
> Sigh...

> Besides, this situation is a nightmare for automatic programs (upper
> layers) which try to take advantage of a maximum of extensions.

this is why we now have /proc/net//ip_*_names
 
>  Herve
-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: extensions manpage, howto etc

[Henrik Nordstrom]

On Tue, 20 Jan 2004, Harald Welte wrote:

> However, if somebody wants to take the full job of 
> - adding the respective information to the sourcecode
> - generating the respective manpage snippets

These two can relatively easily be done as the man page should essentially
be a superset of the terse extension help and requires very little markup,
and having them unified would help greatly in ensuring the reference
documentation is kept up to date with the extension it documents.

> - generating netfilter-extension-HOWTO snippets in docbook XML

This I do not think is wise to have derived from the extension source for
the reasons outlined before. The person writing the HOWTO class material
is most likely not the same as the extension maintainer as it requires
somewhat different skills in writing the extension and a howto on how it
is used (not only syntax), and there is also the question about
translation.

The HOWTO class material is in my opinion better maintained as a separate
document preferably maintained by a non-programmer with close user
contact, and should stay on the HOWTO level of things not dive into all
details about exact syntax etc like the reference man page.

The reference documentation (i.e. man page) should be maintained by
whomever maintains the extension implementations, and the framework should
encourage this as a natural part of the process of implementing and
maintaining extensions.

Regards
Henrik

Re: extensions manpage, howto etc

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1844 bytes --]

On Wed, Jan 21, 2004 at 01:17:42AM +0100, Henrik Nordstrom wrote:
> On Tue, 20 Jan 2004, Harald Welte wrote:
> 
> > However, if somebody wants to take the full job of 
> > - adding the respective information to the sourcecode
> > - generating the respective manpage snippets
> 
> These two can relatively easily be done as the man page should essentially
> be a superset of the terse extension help and requires very little markup,
> and having them unified would help greatly in ensuring the reference
> documentation is kept up to date with the extension it documents.
> 
> > - generating netfilter-extension-HOWTO snippets in docbook XML
> 
> This I do not think is wise to have derived from the extension source for
> the reasons outlined before. The person writing the HOWTO class material
> is most likely not the same as the extension maintainer as it requires
> somewhat different skills in writing the extension and a howto on how it
> is used (not only syntax), and there is also the question about
> translation.
> 
> The HOWTO class material is in my opinion better maintained as a separate
> document preferably maintained by a non-programmer with close user
> contact, and should stay on the HOWTO level of things not dive into all
> details about exact syntax etc like the reference man page.

Yes, I agree (in parts).  But still, the HOWTO should include an
auto-generated snippet of the reference, too.

> Regards
> Henrik

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: iptables error reporting

[Henrik Nordstrom]

On Tue, 20 Jan 2004, Harald Welte wrote:

> we have no more verbose error reporting due to the getsockopt/setsockopt
> interface.  I don't see a way how this could change :(

There is ways to noticably improve the error reporting in the userspace
part of things on the type of errors discussed.

Today there is a single error message covering all three cases (No
chain/target/match by that name), but since the userspace knows in detail
what chains exists it should be easy to make the userspace use another
error for invalid chain chain names allowing the error repoting to be
improved into

  No chain by that name
  No match/target by that name

In addition the chain error could easily include the chain name in
question. it is after all the userspace which decides the chain does not
exists by reading the old table from the kernel so it should be trivial to 
report which chain we did not find in the table making this error read like

  No chain named '%s'

And with the addition of the /proc/net/ip_XX_names the second error could
be extended to include which match/target is not existing by looking for
the first unknown match/target in the new table, allowing both 
separation of target or match and which target/match is missing, making 
this error read like

  No match named '%s'
  No target named '%s'

Anyone wanting to take a shot at this?


What can not currently be solved in iptables is the case where a kernel
extension rejects the information given by userspace, or where there is
structural mismatches between user/kernel such as seen by the TRACE patch
if user/kernel binaries is not in sync. These have no choice but to give a
"operation failed" type error message with at best a more verbose error
message in the kernel log explaining who rejected the table and why.

Regards
Henrik

Re: iptables extensions manpage

[Henrik Nordstrom]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1492 bytes --]

On Tue, 20 Jan 2004, Harald Welte wrote:

> 'man iptables' should reflect what the iptables userspace command has
> built in.  So making the manpage dependent on what extensions are
> compiled at iptables compile time is fine with me.
> 
> And yes, I prefer the 'squid' way, listing all matches/targets and their
> options, but mark unavailable ones with a comment.  This comment could
> refer to a special section at the bottom of the page, where we list
> possible causes why it is disabled, and which steps one might need to
> take to make it available.

Attached you find version 2 of my proposed patch to split the extensions
part of the man pages into snippets under extensions/. This time all
documented extensions is included in the resulting man page but the ones
not supported by the iptables build is marked with "Unsupported, see
Patch-O-Matic" with the intent that a Patch-O-Matic section should be
added documenting what Patch-O-Matic is. There is also a slight format
change in the snippets to reduce the amount of redundant information.

Note: The extensions/Makefile is somewhat more complex than it actually
need to be partly due to the fact that not all extensions have reference
documentation. If the reference documentation is unified into the C source
then the makefiles can be somewhat simplified.

As before the patch just includes the Makefile changes and a small script
that splits the existing manpages rather than full diffs of the split as 
such.

Regards
Henrik

[-- Attachment #2: Type: TEXT/PLAIN, Size: 7910 bytes --]

Index: split_manpages.sh
===================================================================
RCS file: split_manpages.sh
diff -N split_manpages.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ split_manpages.sh	21 Jan 2004 02:03:12 -0000
@@ -0,0 +1,74 @@
+#!/bin/sh
+
+cat iptables.8 | awk '
+BEGIN {
+	ext=""
+	out=""
+}
+/^.SH / {
+	ext=""
+	out=""
+}
+
+/^.SH .*EXTENSIONS/ {
+	ext=$2
+}
+
+/^.SS / {
+	if (ext != "") {
+		if (out == "")
+			print ".\\\" @" ext "@";
+		out=$2
+		next
+	}
+}
+
+{
+	if (out != "")
+		print >>"extensions/libipt_" out ".man"
+	else
+		print
+}
+' >iptables.8.in
+
+cat ip6tables.8 | awk '
+BEGIN {
+	ext=""
+	out=""
+}
+/^.SH / {
+	ext=""
+	out=""
+}
+
+/^.SH .*EXTENSIONS/ {
+	ext=$2
+}
+
+/^.SS / {
+	if (ext != "") {
+		if (out == "") {
+			print ".\\\" @" ext "@";
+		}
+		out=$2
+		next
+	}
+}
+
+/^\.\\" .SS / {
+	if (ext != "") {
+		if (out == "")
+			print ".\\\" @" ext "@";
+		out=$3
+		next
+	}
+}
+
+
+{
+	if (out != "")
+		print >>"extensions/libip6t_" out ".man"
+	else
+		print
+}
+' >ip6tables.8.in
Index: Makefile
===================================================================
RCS file: /cvspublic/iptables/Makefile,v
retrieving revision 1.76
diff -u -p -r1.76 Makefile
--- Makefile	2 Nov 2003 17:01:09 -0000	1.76
+++ Makefile	21 Jan 2004 02:03:12 -0000
@@ -53,7 +53,7 @@ LDFLAGS      = -static
 LDLIBS       =
 endif
 
-EXTRAS+=iptables iptables.o
+EXTRAS+=iptables iptables.o iptables.8
 EXTRA_INSTALLS+=$(DESTDIR)$(BINDIR)/iptables $(DESTDIR)$(MANDIR)/man8/iptables.8
 
 # No longer experimental.
@@ -61,7 +61,7 @@ EXTRAS+=iptables-save iptables-restore
 EXTRA_INSTALLS+=$(DESTDIR)$(BINDIR)/iptables-save $(DESTDIR)$(BINDIR)/iptables-restore $(DESTDIR)$(MANDIR)/man8/iptables-restore.8 $(DESTDIR)$(MANDIR)/man8/iptables-save.8
 
 ifeq ($(DO_IPV6), 1)
-EXTRAS+=ip6tables ip6tables.o
+EXTRAS+=ip6tables ip6tables.o ip6tables.8
 EXTRA_INSTALLS+=$(DESTDIR)$(BINDIR)/ip6tables $(DESTDIR)$(MANDIR)/man8/ip6tables.8
 EXTRAS_EXP+=ip6tables-save ip6tables-restore
 EXTRA_INSTALLS_EXP+=$(DESTDIR)$(BINDIR)/ip6tables-save $(DESTDIR)$(BINDIR)/ip6tables-restore # $(DESTDIR)$(MANDIR)/man8/iptables-restore.8 $(DESTDIR)$(MANDIR)/man8/iptables-save.8 $(DESTDIR)$(MANDIR)/man8/ip6tables-save.8 $(DESTDIR)$(MANDIR)/man8/ip6tables-restore.8
@@ -147,6 +147,11 @@ EXTRA_DEPENDS+=iptables-standalone.d ipt
 iptables-standalone.d iptables.d: %.d: %.c
 	@-$(CC) -M -MG $(CFLAGS) $< | sed -e 's@^.*\.o:@$*.d $*.o:@' > $@
 
+iptables.8: iptables.8.in extensions/libipt_matches.man extensions/libipt_targets.man
+	sed -e '/@MATCH@/ r extensions/libipt_matches.man' -e '/@TARGET@/ r extensions/libipt_targets.man' iptables.8.in >iptables.8
+
+ip6tables.8: ip6tables.8.in extensions/libip6t_matches.man extensions/libip6t_targets.man
+	sed -e '/@MATCH@/ r extensions/libip6t_matches.man' -e '/@TARGET@/ r extensions/libiptt_targets.man' ip6tables.8.in >ip6tables.8
 
 # Development Targets
 .PHONY: install-devel-man3
Index: extensions/Makefile
===================================================================
RCS file: /cvspublic/iptables/extensions/Makefile,v
retrieving revision 1.35
diff -u -p -r1.35 Makefile
--- extensions/Makefile	8 Oct 2003 10:17:05 -0000	1.35
+++ extensions/Makefile	21 Jan 2004 02:03:12 -0000
@@ -12,6 +12,29 @@ PF6_EXT_SLIB:=eui64 hl icmpv6 length lim
 PF_EXT_SLIB_OPTS:=$(foreach T,$(wildcard extensions/.*-test),$(shell KERNEL_DIR=$(KERNEL_DIR) $(T)))
 PF6_EXT_SLIB_OPTS:=$(foreach T,$(wildcard extensions/.*-test6),$(shell KERNEL_DIR=$(KERNEL_DIR) $(T)))
 
+PF_EXT_ALL_SLIB:=$(patsubst extensions/libipt_%.c, %, $(wildcard extensions/libipt_*.c))
+PF6_EXT_ALL_SLIB:=$(patsubst extensions/libip6t_%.c, %, $(wildcard extensions/libipt_*.c))
+
+PF_EXT_MAN_ALL_MATCHES:=$(foreach T,$(PF_EXT_ALL_SLIB),$(shell test -f extensions/libipt_$(T).man && grep -q register_match extensions/libipt_$(T).c  && echo $(T)))
+PF_EXT_MAN_ALL_TARGETS:=$(foreach T,$(PF_EXT_ALL_SLIB),$(shell test -f extensions/libipt_$(T).man && grep -q register_target extensions/libipt_$(T).c && echo $(T)))
+PF6_EXT_MAN_ALL_MATCHES:=$(foreach T,$(PF6_EXT_ALL_SLIB),$(shell test -f extensions/libip6t_$(T).man && grep -q register_match6 extensions/libip6t_$(T).c  && echo $(T)))
+PF6_EXT_MAN_ALL_TARGETS:=$(foreach T,$(PF6_EXT_ALL_SLIB),$(shell test -f extensions/libip6t_$(T).man && grep -q register_target6 extensions/libip6t_$(T).c && echo $(T)))
+
+PF_EXT_MAN_MATCHES:=$(filter $(PF_EXT_ALL_SLIB), $(PF_EXT_MAN_ALL_MATCHES))
+PF_EXT_MAN_TARGETS:=$(filter $(PF_EXT_ALL_SLIB), $(PF_EXT_MAN_ALL_TARGETS))
+PF_EXT_MAN_EXTRA_MATCHES:=$(filter-out $(PF_EXT_MAN_MATCHES), $(PF_EXT_MAN_ALL_MATCHES))
+PF_EXT_MAN_EXTRA_TARGETS:=$(filter-out $(PF_EXT_MAN_TARGETS), $(PF_EXT_MAN_ALL_TARGETS))
+PF6_EXT_MAN_MATCHES:=$(filter $(PF6_EXT_ALL_SLIB), $(PF6_EXT_MAN_ALL_MATCHES))
+PF6_EXT_MAN_TARGETS:=$(filter $(PF6_EXT_ALL_SLIB), $(PF6_EXT_MAN_ALL_TARGETS))
+PF6_EXT_MAN_EXTRA_MATCHES:=$(filter-out $(PF6_EXT_MAN_MATCHES), $(PF6_EXT_MAN_ALL_MATCHES))
+PF6_EXT_MAN_EXTRA_TARGETS:=$(filter-out $(PF6_EXT_MAN_TARGETS), $(PF6_EXT_MAN_ALL_TARGETS))
+
+
+allman:
+	@echo ALL_SLIB: $(PF_EXT_ALL_SLIB)
+	@echo ALL_MATCH: $(PF_EXT_MAN_ALL_MATCHES)
+	@echo ALL_TARGET: $(PF_EXT_MAN_ALL_TARGETS)
+
 PF_EXT_SLIB+=$(PF_EXT_SLIB_OPTS)
 PF6_EXT_SLIB+=$(PF6_EXT_SLIB_OPTS)
 
@@ -79,6 +102,58 @@ extensions/lib%.o: extensions/lib%.c
 
 endif
  
+EXTRAS += extensions/libipt_targets.man
+extensions/libipt_targets.man: $(patsubst %,extensions/libipt_%.man,$(PF_EXT_MAN_ALL_TARGETS))
+	@for ext in $(PF_EXT_MAN_TARGETS); do \
+	    echo ".SS $$ext" ;\
+	    cat extensions/libipt_$$ext.man ;\
+	done >extensions/libipt_targets.man
+	@if [ -n "$(PF_EXT_MAN_EXTRA_TARGETS)" ]; then \
+	    for ext in $(PF_EXT_MAN_EXTRA_TARGETS); do \
+		echo ".SS $$ext (not supported, see Patch-O-Matic)" ;\
+		cat extensions/libipt_$$ext.man ;\
+	    done ;\
+       	fi >>extensions/libipt_targets.man
+
+EXTRAS += extensions/libipt_matches.man
+extensions/libipt_matches.man: $(patsubst %,extensions/libipt_%.man,$(PF_EXT_MAN_ALL_MATCHES))
+	@for ext in $(PF_EXT_MAN_MATCHES); do \
+	    echo ".SS $$ext" ;\
+	    cat extensions/libipt_$$ext.man ;\
+	done >extensions/libipt_matches.man
+	@if [ -n "$(PF_EXT_MAN_EXTRA_MATCHES)" ]; then \
+	    for ext in $(PF_EXT_MAN_EXTRA_MATCHES); do \
+		echo ".SS $$ext (not supported, see Patch-O-Matic)" ;\
+		cat extensions/libipt_$$ext.man ;\
+	    done ;\
+       	fi >>extensions/libipt_matches.man
+
+EXTRAS += extensions/libip6t_targets.man
+extensions/libip6t_targets.man: $(patsubst %, extensions/libip6t_%.man, $(PF6_EXT_MAN_ALL_TARGETS))
+	@for ext in $(PF6_EXT_MAN_TARGETS); do \
+	    echo ".SS $$ext" ;\
+	    cat extensions/libip6t_$$ext.man ;\
+	done >extensions/libip6t_targets.man
+	@if [ -n "$(PF6_EXT_MAN_EXTRA_TARGETS)" ]; then \
+	    for ext in $(PF6_EXT_MAN_EXTRA_TARGETS); do \
+		echo ".SS $$ext (not supported, see Patch-O-Matic)" ;\
+		cat extensions/libip6t_$$ext.man ;\
+	    done ;\
+       	fi >>extensions/libip6t_targets.man
+
+EXTRAS += extensions/libip6t_matches.man
+extensions/libip6t_matches.man: $(patsubst %, extensions/libip6t_%.man, $(PF6_EXT_MAN_ALL_MATCHES))
+	@for ext in $(PF6_EXT_MAN_MATCHES); do \
+	    echo ".SS $$ext" ;\
+	    cat extensions/libip6t_$$ext.man ;\
+	done >extensions/libip6t_matches.man
+	@if [ -n "$(PF6_EXT_MAN_EXTRA_MATCHES)" ]; then \
+	    for ext in $(PF6_EXT_MAN_EXTRA_MATCHES); do \
+		echo ".SS $$ext (not supported, see Patch-O-Matic)" ;\
+		cat extensions/libip6t_$$ext.man ;\
+	    done ;\
+       	fi >>extensions/libip6t_matches.man
+
 $(DESTDIR)$(LIBDIR)/iptables/libipt_%.so: extensions/libipt_%.so
 	@[ -d $(DESTDIR)$(LIBDIR)/iptables ] || mkdir -p $(DESTDIR)$(LIBDIR)/iptables
 	cp $< $@

Re: iptables extensions manpage

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]

On Wed, Jan 21, 2004 at 03:16:21AM +0100, Henrik Nordstrom wrote:
> Attached you find version 2 of my proposed patch to split the extensions
> part of the man pages into snippets under extensions/. This time all
> documented extensions is included in the resulting man page but the ones
> not supported by the iptables build is marked with "Unsupported, see
> Patch-O-Matic" with the intent that a Patch-O-Matic section should be
> added documenting what Patch-O-Matic is. There is also a slight format
> change in the snippets to reduce the amount of redundant information.

thanks, I have committed your patch to the CVS repository.  I've also
added dozens of .man files that were missing in the old manpage.

> Regards
> Henrik
-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[patch, resent] Updated CONNMARK

[Henrik Nordstrom]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 729 bytes --]

Attached you find a slightly updated version of the previously submitted 
patches to bring CONNMARK up to the level discussed for kernel submission. 
You acknowledged getting the patches but the CVS is still not updated it 
seems.

iptables userspace patch
	- manpage additions
	- mask options
	- copyright & license
	- C99 struct initializers
iptables pom-ng	patch
	- mask operations
	- copyright & license
	- C99 struct initializers
iptables pom patch
	- identical to the pom-ng patch 


The differences from the previous submission is only some minor format 
cleanup to use a .ladd patch snipped for the ip_conntrack.h field 
addition and new style manpage snippets.

Please let me know if there is a problem.

Regards
Henrik

[-- Attachment #2: Type: TEXT/PLAIN, Size: 11886 bytes --]

Index: patch-o-matic-ng/CONNMARK/configure.help
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/configure.help,v
retrieving revision 1.1
diff -u -p -r1.1 configure.help
--- patch-o-matic-ng/CONNMARK/configure.help	18 Dec 2003 18:55:59 -0000	1.1
+++ patch-o-matic-ng/CONNMARK/configure.help	2 Feb 2004 23:24:23 -0000
@@ -20,3 +20,8 @@ connmark match support
 CONFIP_IP_NF_MATCH_CONNMARK
   This option adds a `connmark' match, which allows you to match the
   connection mark value previously set for the session by `CONNMARK'. 
+
+  If you want to compile it as a module, say M here and read
+  Documentation/modules.txt.  The module will be called
+  ipt_connmark.o.  If unsure, say `N'.
+
Index: patch-o-matic-ng/CONNMARK/patch
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/patch,v
retrieving revision 1.1
diff -u -p -r1.1 patch
--- patch-o-matic-ng/CONNMARK/patch	18 Dec 2003 18:47:53 -0000	1.1
+++ patch-o-matic-ng/CONNMARK/patch	2 Feb 2004 23:24:23 -0000
@@ -1,16 +1,3 @@
-diff -uN linux-2.4.3-pre3/include/linux/netfilter_ipv4/ip_conntrack.h linux-2.4.3-pre3-uml/include/linux/netfilter_ipv4/ip_conntrack.h
---- linux-2.4.3-pre3/include/linux/netfilter_ipv4/ip_conntrack.h	Fri Mar  9 21:43:28 2001
-+++ linux-2.4.3-pre3-uml/include/linux/netfilter_ipv4/ip_conntrack.h	Wed Mar 21 13:20:37 2001
-@@ -147,6 +147,9 @@
- 	} nat;
- #endif /* CONFIG_IP_NF_NAT_NEEDED */
- 
-+#if defined(CONFIG_IP_NF_CONNTRACK_MARK)
-+	unsigned long mark;
-+#endif
- };
- 
- /* Alter reply tuple (maybe alter helper).  If it's already taken,
 diff -uN --exclude .* --exclude *.o linux-2.4.3-pre3/net/ipv4/netfilter/ip_conntrack_standalone.c linux-2.4.3-pre3-uml/net/ipv4/netfilter/ip_conntrack_standalone.c
 --- linux-2.4.3-pre3/net/ipv4/netfilter/ip_conntrack_standalone.c	Thu Aug 10 21:35:15 2000
 +++ linux-2.4.3-pre3-uml/net/ipv4/netfilter/ip_conntrack_standalone.c	Wed Mar 21 13:04:19 2001
Index: patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ip_conntrack.h.ladd
===================================================================
RCS file: patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ip_conntrack.h.ladd
diff -N patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ip_conntrack.h.ladd
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ip_conntrack.h.ladd	2 Feb 2004 23:24:23 -0000
@@ -0,0 +1,5 @@
+#endif /* CONFIG_IP_NF_NAT_NEEDED */
+
+#if defined(CONFIG_IP_NF_CONNTRACK_MARK)
+	unsigned long mark;
+#endif
Index: patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h,v
retrieving revision 1.1
diff -u -p -r1.1 ipt_CONNMARK.h
--- patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h	18 Dec 2003 18:47:53 -0000	1.1
+++ patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h	2 Feb 2004 23:24:23 -0000
@@ -1,14 +1,24 @@
 #ifndef _IPT_CONNMARK_H_target
 #define _IPT_CONNMARK_H_target
 
+/* Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
 enum {
-    IPT_CONNMARK_SET = 0,
-    IPT_CONNMARK_SAVE,
-    IPT_CONNMARK_RESTORE
+	IPT_CONNMARK_SET = 0,
+	IPT_CONNMARK_SAVE,
+	IPT_CONNMARK_RESTORE
 };
 
 struct ipt_connmark_target_info {
 	unsigned long mark;
+	unsigned long mask;
 	u_int8_t mode;
 };
 
Index: patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ipt_connmark.h
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ipt_connmark.h,v
retrieving revision 1.1
diff -u -p -r1.1 ipt_connmark.h
--- patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ipt_connmark.h	18 Dec 2003 18:47:53 -0000	1.1
+++ patch-o-matic-ng/CONNMARK/linux/include/linux/netfilter_ipv4/ipt_connmark.h	2 Feb 2004 23:24:23 -0000
@@ -1,6 +1,15 @@
 #ifndef _IPT_CONNMARK_H
 #define _IPT_CONNMARK_H
 
+/* Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
 struct ipt_connmark_info {
 	unsigned long mark, mask;
 	u_int8_t invert;
Index: patch-o-matic-ng/CONNMARK/linux/net/ipv4/netfilter/ipt_CONNMARK.c
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/linux/net/ipv4/netfilter/ipt_CONNMARK.c,v
retrieving revision 1.1
diff -u -p -r1.1 ipt_CONNMARK.c
--- patch-o-matic-ng/CONNMARK/linux/net/ipv4/netfilter/ipt_CONNMARK.c	18 Dec 2003 18:47:53 -0000	1.1
+++ patch-o-matic-ng/CONNMARK/linux/net/ipv4/netfilter/ipt_CONNMARK.c	2 Feb 2004 23:24:23 -0000
@@ -1,11 +1,32 @@
-/* This is a module which is used for setting/remembering the mark field of
- * an connection, or optionally restore it to the skb
+/* This kernel module is used to modify the connection mark values, or
+ * to optionally restore the skb nfmark from the connection mark
+ *
+ * Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/ip.h>
 #include <net/checksum.h>
 
+MODULE_AUTHOR("Henrik Nordstrom <hno@marasytems.com>");
+MODULE_DESCRIPTION("IP tables CONNMARK matching module");
+MODULE_LICENSE("GPL");
+
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv4/ipt_CONNMARK.h>
 #include <linux/netfilter_ipv4/ip_conntrack.h>
@@ -19,20 +40,29 @@ target(struct sk_buff **pskb,
        void *userinfo)
 {
 	const struct ipt_connmark_target_info *markinfo = targinfo;
+	unsigned long diff;
+	unsigned long nfmark;
+	unsigned long newmark;
 
 	enum ip_conntrack_info ctinfo;
 	struct ip_conntrack *ct = ip_conntrack_get((*pskb), &ctinfo);
 	if (ct) {
 	    switch(markinfo->mode) {
 	    case IPT_CONNMARK_SET:
-		ct->mark = markinfo->mark;
+		newmark = (ct->mark & ~markinfo->mask) | markinfo->mark;
+		if (newmark != ct->mark)
+		    ct->mark = newmark;
 		break;
 	    case IPT_CONNMARK_SAVE:
-		ct->mark = (*pskb)->nfmark;
+		newmark = (ct->mark & ~markinfo->mask) | ((*pskb)->nfmark & markinfo->mask);
+		if (ct->mark != newmark)
+		    ct->mark = newmark;
 		break;
 	    case IPT_CONNMARK_RESTORE:
-		if (ct->mark != (*pskb)->nfmark) {
-		    (*pskb)->nfmark = ct->mark;
+		nfmark = (*pskb)->nfmark;
+		diff = (ct->mark ^ nfmark & markinfo->mask);
+		if (diff != 0) {
+		    (*pskb)->nfmark = nfmark ^ diff;
 		    (*pskb)->nfcache |= NFC_ALTERED;
 		}
 		break;
@@ -45,9 +75,9 @@ target(struct sk_buff **pskb,
 static int
 checkentry(const char *tablename,
 	   const struct ipt_entry *e,
-           void *targinfo,
-           unsigned int targinfosize,
-           unsigned int hook_mask)
+	   void *targinfo,
+	   unsigned int targinfosize,
+	   unsigned int hook_mask)
 {
 	struct ipt_connmark_target_info *matchinfo = targinfo;
 	if (targinfosize != IPT_ALIGN(sizeof(struct ipt_connmark_target_info))) {
@@ -67,15 +97,16 @@ checkentry(const char *tablename,
 	return 1;
 }
 
-static struct ipt_target ipt_connmark_reg
-= { { NULL, NULL }, "CONNMARK", target, checkentry, NULL, THIS_MODULE };
+static struct ipt_target ipt_connmark_reg = {
+	.name = "CONNMARK",
+	.target = &target,
+	.checkentry = &checkentry,
+	.me = THIS_MODULE
+};
 
 static int __init init(void)
 {
-	if (ipt_register_target(&ipt_connmark_reg))
-		return -EINVAL;
-
-	return 0;
+	return ipt_register_target(&ipt_connmark_reg);
 }
 
 static void __exit fini(void)
Index: patch-o-matic-ng/CONNMARK/linux/net/ipv4/netfilter/ipt_connmark.c
===================================================================
RCS file: /cvspublic/patch-o-matic-ng/CONNMARK/linux/net/ipv4/netfilter/ipt_connmark.c,v
retrieving revision 1.1
diff -u -p -r1.1 ipt_connmark.c
--- patch-o-matic-ng/CONNMARK/linux/net/ipv4/netfilter/ipt_connmark.c	18 Dec 2003 18:47:53 -0000	1.1
+++ patch-o-matic-ng/CONNMARK/linux/net/ipv4/netfilter/ipt_connmark.c	2 Feb 2004 23:24:23 -0000
@@ -1,7 +1,31 @@
-/* Kernel module to match connection mark values. */
+/* This kernel module matches connection mark values set by the
+ * CONNMARK target
+ *
+ * Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
 #include <linux/module.h>
 #include <linux/skbuff.h>
 
+MODULE_AUTHOR("Henrik Nordstrom <hno@marasytems.com>");
+MODULE_DESCRIPTION("IP tables connmark match module");
+MODULE_LICENSE("GPL");
+
 #include <linux/netfilter_ipv4/ip_tables.h>
 #include <linux/netfilter_ipv4/ipt_connmark.h>
 #include <linux/netfilter_ipv4/ip_conntrack.h>
@@ -27,10 +51,10 @@ match(const struct sk_buff *skb,
 
 static int
 checkentry(const char *tablename,
-           const struct ipt_ip *ip,
-           void *matchinfo,
-           unsigned int matchsize,
-           unsigned int hook_mask)
+	   const struct ipt_ip *ip,
+	   void *matchinfo,
+	   unsigned int matchsize,
+	   unsigned int hook_mask)
 {
 	if (matchsize != IPT_ALIGN(sizeof(struct ipt_connmark_info)))
 		return 0;
@@ -38,8 +62,12 @@ checkentry(const char *tablename,
 	return 1;
 }
 
-static struct ipt_match connmark_match
-= { { NULL, NULL }, "connmark", &match, &checkentry, NULL, THIS_MODULE };
+static struct ipt_match connmark_match = {
+	.name = "connmark",
+	.match = &match,
+	.checkentry = &checkentry,
+	.me = THIS_MODULE
+};
 
 static int __init init(void)
 {

[-- Attachment #3: Type: TEXT/PLAIN, Size: 12148 bytes --]

Index: userspace/extensions/libipt_CONNMARK.c
===================================================================
RCS file: /cvspublic/iptables/extensions/libipt_CONNMARK.c,v
retrieving revision 1.3
diff -u -p -r1.3 libipt_CONNMARK.c
--- userspace/extensions/libipt_CONNMARK.c	4 Jun 2002 07:46:30 -0000	1.3
+++ userspace/extensions/libipt_CONNMARK.c	2 Feb 2004 23:27:20 -0000
@@ -1,4 +1,24 @@
-/* Shared library add-on to iptables to add CONNMARK target support. */
+/* Shared library add-on to iptables to add CONNMARK target support.
+ *
+ * (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * Version 1.1
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -21,9 +41,9 @@ help(void)
 {
 	printf(
 "CONNMARK target v%s options:\n"
-"  --set-mark value              Set conntrack mark value\n"
-"  --save-mark                   Save the packet nfmark on the connection\n"
-"  --restore-mark                Restore saved nfmark value\n"
+"  --set-mark value[/mask]       Set conntrack mark value\n"
+"  --save-mark [--mask mask]     Save the packet nfmark in the connection\n"
+"  --restore-mark [--mask mask]  Restore saved nfmark value\n"
 "\n",
 IPTABLES_VERSION);
 }
@@ -32,6 +52,7 @@ static struct option opts[] = {
 	{ "set-mark", 1, 0, '1' },
 	{ "save-mark", 0, 0, '2' },
 	{ "restore-mark", 0, 0, '3' },
+	{ "mask", 1, 0, '4' },
 	{ 0 }
 };
 
@@ -55,7 +76,10 @@ parse(int c, char **argv, int invert, un
 		char *end;
 	case '1':
 		markinfo->mode = IPT_CONNMARK_SET;
+		markinfo->mask = ~0;
 		markinfo->mark = strtoul(optarg, &end, 0);
+		if (*end == '/' && end[1] != '\0')
+		    markinfo->mask = strtoul(end+1, &end, 0);
 		if (*end != '\0' || end == optarg)
 			exit_error(PARAMETER_PROBLEM, "Bad MARK value `%s'", optarg);
 		if (*flags)
@@ -65,6 +89,7 @@ parse(int c, char **argv, int invert, un
 		break;
 	case '2':
 		markinfo->mode = IPT_CONNMARK_SAVE;
+		markinfo->mask = ~0;
 		if (*flags)
 			exit_error(PARAMETER_PROBLEM,
 			           "CONNMARK target: Can't specify --save-mark twice");
@@ -72,11 +97,20 @@ parse(int c, char **argv, int invert, un
 		break;
 	case '3':
 		markinfo->mode = IPT_CONNMARK_RESTORE;
+		markinfo->mask = ~0;
 		if (*flags)
 			exit_error(PARAMETER_PROBLEM,
 			           "CONNMARK target: Can't specify --restore-mark twice");
 		*flags = 1;
 		break;
+	case '4':
+		if (!*flags)
+			exit_error(PARAMETER_PROBLEM,
+			           "CONNMARK target: Can't specify --mask without a operation");
+		markinfo->mark = strtoul(optarg, &end, 0);
+		if (*end != '\0' || end == optarg)
+			exit_error(PARAMETER_PROBLEM, "Bad MARK value `%s'", optarg);
+		break;
 	default:
 		return 0;
 	}
@@ -89,16 +123,10 @@ final_check(unsigned int flags)
 {
 	if (!flags)
 		exit_error(PARAMETER_PROBLEM,
-		           "CONNMARK target: Parameter --set-mark is required");
-}
-
-static void
-print_mark(unsigned long mark, int numeric)
-{
-	printf("0x%lx ", mark);
+		           "CONNMARK target: No operation specified");
 }
 
-/* Prints out the targinfo. */
+/* Prints out the target info. */
 static void
 print(const struct ipt_ip *ip,
       const struct ipt_entry_target *target,
@@ -108,14 +136,21 @@ print(const struct ipt_ip *ip,
 		(const struct ipt_connmark_target_info *)target->data;
 	switch (markinfo->mode) {
 	case IPT_CONNMARK_SET:
-	    printf("CONNMARK set ");
-	    print_mark(markinfo->mark, numeric);
+	    printf("CONNMARK set 0x%lx", markinfo->mark);
+	    if (markinfo->mask != ~0)
+		printf("/0x%lx", markinfo->mask);
+	    printf(" ");
 	    break;
 	case IPT_CONNMARK_SAVE:
 	    printf("CONNMARK save ");
+	    if (markinfo->mask != ~0)
+		printf("mask 0x%lx", markinfo->mask);
+	    printf(" ");
 	    break;
 	case IPT_CONNMARK_RESTORE:
 	    printf("CONNMARK restore ");
+	    if (markinfo->mask != ~0)
+		printf("mask 0x%lx", markinfo->mask);
 	    break;
 	default:
 	    printf("ERROR: UNKNOWN CONNMARK MODE ");
@@ -123,7 +158,7 @@ print(const struct ipt_ip *ip,
 	}
 }
 
-/* Saves the union ipt_targinfo in parsable form to stdout. */
+/* Saves the target into in parsable form to stdout. */
 static void
 save(const struct ipt_ip *ip, const struct ipt_entry_target *target)
 {
@@ -132,13 +167,20 @@ save(const struct ipt_ip *ip, const stru
 
 	switch (markinfo->mode) {
 	case IPT_CONNMARK_SET:
-	    printf("--set-mark 0x%lx ", markinfo->mark);
+	    printf("--set-mark 0x%lx", markinfo->mark);
+	    if (markinfo->mask != ~0)
+		printf("/0x%lx", markinfo->mask);
+	    printf(" ");
 	    break;
 	case IPT_CONNMARK_SAVE:
 	    printf("--save-mark ");
+	    if (markinfo->mask != ~0)
+		printf("--mask 0x%lx", markinfo->mask);
 	    break;
 	case IPT_CONNMARK_RESTORE:
 	    printf("--restore-mark ");
+	    if (markinfo->mask != ~0)
+		printf("--mask 0x%lx", markinfo->mask);
 	    break;
 	default:
 	    printf("ERROR: UNKNOWN CONNMARK MODE ");
@@ -146,23 +188,21 @@ save(const struct ipt_ip *ip, const stru
 	}
 }
 
-static
-struct iptables_target mark
-= { NULL,
-    "CONNMARK",
-    IPTABLES_VERSION,
-    IPT_ALIGN(sizeof(struct ipt_connmark_target_info)),
-    IPT_ALIGN(sizeof(struct ipt_connmark_target_info)),
-    &help,
-    &init,
-    &parse,
-    &final_check,
-    &print,
-    &save,
-    opts
+static struct iptables_target connmark_target = {
+    .name          = "CONNMARK",
+    .version       = IPTABLES_VERSION,
+    .size          = IPT_ALIGN(sizeof(struct ipt_connmark_target_info)),
+    .userspacesize = IPT_ALIGN(sizeof(struct ipt_connmark_target_info)),
+    .help          = &help,
+    .init          = &init,
+    .parse         = &parse,
+    .final_check   = &final_check,
+    .print         = &print,
+    .save          = &save,
+    .extra_opts    = opts
 };
 
 void _init(void)
 {
-	register_target(&mark);
+	register_target(&connmark_target);
 }
Index: userspace/extensions/libipt_CONNMARK.man
===================================================================
RCS file: /cvspublic/iptables/extensions/libipt_CONNMARK.man,v
retrieving revision 1.1
diff -u -p -r1.1 libipt_CONNMARK.man
--- userspace/extensions/libipt_CONNMARK.man	22 Jan 2004 15:04:24 -0000	1.1
+++ userspace/extensions/libipt_CONNMARK.man	2 Feb 2004 23:27:20 -0000
@@ -1,13 +1,15 @@
-This target allows you to mark that connection with an arbitrary walue.  This
-value can later be matched via the 
-.B connmark
-match.
+This module sets the netfilter mark value associated with a connection
 .TP
-.BI "--set-mark " "mark"
-Set the conntrack mark,
+.B --set-mark mark[/mask]
+Set connection mark. If a mask is specified then only those bits set in the
+mask is modified.
 .TP
-.BI "--save-mark"
-Save the packet nfmark on the connection mark.
+.B --save-mark [--mask mask]
+Copy the netfilter packet mark value to the connection mark. If a mask
+is specified then only those bits are copied.
 .TP
-.BI "--restore-mark"
-Restore the saved nfmark value from the connection mark.
+.B --restore-mark [--mask mask]
+Copy the connection mark value to the packet. If a mask is specified
+then only those bits are copied. This is only valid in the
+.B mangle
+table.
Index: userspace/extensions/libipt_connmark.c
===================================================================
RCS file: /cvspublic/iptables/extensions/libipt_connmark.c,v
retrieving revision 1.5
diff -u -p -r1.5 libipt_connmark.c
--- userspace/extensions/libipt_connmark.c	20 Sep 2002 15:25:13 -0000	1.5
+++ userspace/extensions/libipt_connmark.c	2 Feb 2004 23:27:20 -0000
@@ -1,4 +1,24 @@
-/* Shared library add-on to iptables to add CONNMARK matching support. */
+/* Shared library add-on to iptables to add connmark matching support.
+ *
+ * (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * Version 1.1
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
 #include <stdio.h>
 #include <netdb.h>
 #include <string.h>
@@ -50,7 +70,7 @@ parse(int c, char **argv, int invert, un
 		if (*end == '/') {
 			markinfo->mask = strtoul(end+1, &end, 0);
 		} else
-			markinfo->mask = 0xffffffff;
+			markinfo->mask = ~0;
 		if (*end != '\0' || end == optarg)
 			exit_error(PARAMETER_PROBLEM, "Bad MARK value `%s'", optarg);
 		if (invert)
@@ -67,7 +87,7 @@ parse(int c, char **argv, int invert, un
 static void
 print_mark(unsigned long mark, unsigned long mask, int numeric)
 {
-	if(mask != 0xffffffff)
+	if(mask != ~0)
 		printf("0x%lx/0x%lx ", mark, mask);
 	else
 		printf("0x%lx ", mark);
@@ -96,7 +116,7 @@ print(const struct ipt_ip *ip,
 	print_mark(info->mark, info->mask, numeric);
 }
 
-/* Saves the union ipt_matchinfo in parsable form to stdout. */
+/* Saves the matchinfo in parsable form to stdout. */
 static void
 save(const struct ipt_ip *ip, const struct ipt_entry_match *match)
 {
@@ -109,23 +129,21 @@ save(const struct ipt_ip *ip, const stru
 	print_mark(info->mark, info->mask, 0);
 }
 
-static
-struct iptables_match mark
-= { NULL,
-    "connmark",
-    IPTABLES_VERSION,
-    IPT_ALIGN(sizeof(struct ipt_connmark_info)),
-    IPT_ALIGN(sizeof(struct ipt_connmark_info)),
-    &help,
-    &init,
-    &parse,
-    &final_check,
-    &print,
-    &save,
-    opts
+static struct iptables_match connmark_match = {
+    .name          = "connmark",
+    .version       = IPTABLES_VERSION,
+    .size          = IPT_ALIGN(sizeof(struct ipt_connmark_info)),
+    .userspacesize = IPT_ALIGN(sizeof(struct ipt_connmark_info)),
+    .help          = &help,
+    .init          = &init,
+    .parse         = &parse,
+    .final_check   = &final_check,
+    .print         = &print,
+    .save          = &save,
+    .extra_opts    = opts
 };
 
 void _init(void)
 {
-	register_match(&mark);
+	register_match(&connmark_match);
 }
Index: userspace/extensions/libipt_connmark.man
===================================================================
RCS file: userspace/extensions/libipt_connmark.man
diff -N userspace/extensions/libipt_connmark.man
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ userspace/extensions/libipt_connmark.man	2 Feb 2004 23:27:20 -0000
@@ -0,0 +1,9 @@
+This module matches the netfilter mark field associated with a connection
+(which can be set using the
+.B CONNMARK
+target below).
+.TP
+.BI "--mark " "value[/mask]"
+Matches packets in connections with the given mark value (if a mask is
+specified, this is logically ANDed with the mark before the
+comparison).

[-- Attachment #4: Type: TEXT/PLAIN, Size: 11187 bytes --]

Index: patch-o-matic/extra/CONNMARK.patch
===================================================================
RCS file: /cvspublic/netfilter/patch-o-matic/extra/CONNMARK.patch,v
retrieving revision 1.4
diff -u -p -r1.4 CONNMARK.patch
--- patch-o-matic/extra/CONNMARK.patch	23 Aug 2003 22:17:08 -0000	1.4
+++ patch-o-matic/extra/CONNMARK.patch	2 Feb 2004 23:24:43 -0000
@@ -36,27 +36,60 @@
  		LIST_DELETE(&ip_conntrack_expect_list, expected);
  		expected->expectant->expecting--;
  		nf_conntrack_get(&master_ct(conntrack)->infos[0]);
-diff -uN linux-2.4.3-pre3/include/linux/netfilter_ipv4/ipt_connmark.h linux-2.4.3-pre3-uml/include/linux/netfilter_ipv4/ipt_connmark.h
---- linux-2.4.3-pre3/include/linux/netfilter_ipv4/ipt_connmark.h	Thu Jan  1 01:00:00 1970
-+++ linux-2.4.3-pre3-uml/include/linux/netfilter_ipv4/ipt_connmark.h	Wed Mar 21 11:38:46 2001
-@@ -0,0 +1,9 @@
+diff -ruN --exclude CVS linux-orig/include/linux/netfilter_ipv4/ipt_connmark.h linux/include/linux/netfilter_ipv4/ipt_connmark.h
+--- linux-orig/include/linux/netfilter_ipv4/ipt_connmark.h	Thu Jan  1 01:00:00 1970
++++ linux/include/linux/netfilter_ipv4/ipt_connmark.h	Sun Jan 18 17:33:42 2004
+@@ -0,0 +1,18 @@
 +#ifndef _IPT_CONNMARK_H
 +#define _IPT_CONNMARK_H
 +
++/* Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
++ * by Henrik Nordstrom <hno@marasystems.com>
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; either version 2 of the License, or
++ * (at your option) any later version.
++ */
++
 +struct ipt_connmark_info {
 +	unsigned long mark, mask;
 +	u_int8_t invert;
 +};
 +
 +#endif /*_IPT_CONNMARK_H*/
-diff -uN --exclude .* --exclude *.o linux-2.4.3-pre3/net/ipv4/netfilter/ipt_connmark.c linux-2.4.3-pre3-uml/net/ipv4/netfilter/ipt_connmark.c
---- linux-2.4.3-pre3/net/ipv4/netfilter/ipt_connmark.c	Thu Jan  1 01:00:00 1970
-+++ linux-2.4.3-pre3-uml/net/ipv4/netfilter/ipt_connmark.c	Wed Mar 21 13:23:33 2001
-@@ -0,0 +1,55 @@
-+/* Kernel module to match connection mark values. */
+diff -ruN --exclude CVS linux-orig/net/ipv4/netfilter/ipt_connmark.c linux/net/ipv4/netfilter/ipt_connmark.c
+--- linux-orig/net/ipv4/netfilter/ipt_connmark.c	Thu Jan  1 01:00:00 1970
++++ linux/net/ipv4/netfilter/ipt_connmark.c	Sun Jan 18 17:57:58 2004
+@@ -0,0 +1,83 @@
++/* This kernel module matches connection mark values set by the
++ * CONNMARK target
++ *
++ * Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
++ * by Henrik Nordstrom <hno@marasystems.com>
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; either version 2 of the License, or
++ * (at your option) any later version.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program; if not, write to the Free Software
++ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
++ */
++
 +#include <linux/module.h>
 +#include <linux/skbuff.h>
 +
++MODULE_AUTHOR("Henrik Nordstrom <hno@marasytems.com>");
++MODULE_DESCRIPTION("IP tables connmark match module");
++MODULE_LICENSE("GPL");
++
 +#include <linux/netfilter_ipv4/ip_tables.h>
 +#include <linux/netfilter_ipv4/ipt_connmark.h>
 +#include <linux/netfilter_ipv4/ip_conntrack.h>
@@ -82,10 +115,10 @@
 +
 +static int
 +checkentry(const char *tablename,
-+           const struct ipt_ip *ip,
-+           void *matchinfo,
-+           unsigned int matchsize,
-+           unsigned int hook_mask)
++	   const struct ipt_ip *ip,
++	   void *matchinfo,
++	   unsigned int matchsize,
++	   unsigned int hook_mask)
 +{
 +	if (matchsize != IPT_ALIGN(sizeof(struct ipt_connmark_info)))
 +		return 0;
@@ -93,8 +126,12 @@
 +	return 1;
 +}
 +
-+static struct ipt_match connmark_match
-+= { { NULL, NULL }, "connmark", &match, &checkentry, NULL, THIS_MODULE };
++static struct ipt_match connmark_match = {
++	.name = "connmark",
++	.match = &match,
++	.checkentry = &checkentry,
++	.me = THIS_MODULE
++};
 +
 +static int __init init(void)
 +{
@@ -108,37 +145,68 @@
 +
 +module_init(init);
 +module_exit(fini);
-diff -uN linux-2.4.3-pre3/include/linux/netfilter_ipv4/ipt_CONNMARK.h linux-2.4.3-pre3-uml/include/linux/netfilter_ipv4/ipt_CONNMARK.h
---- linux-2.4.3-pre3/include/linux/netfilter_ipv4/ipt_CONNMARK.h	Thu Jan  1 01:00:00 1970
-+++ linux-2.4.3-pre3-uml/include/linux/netfilter_ipv4/ipt_CONNMARK.h	Wed Mar 21 12:25:20 2001
-@@ -0,0 +1,15 @@
+diff -ruN --exclude CVS linux-orig/include/linux/netfilter_ipv4/ipt_CONNMARK.h linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h
+--- linux-orig/include/linux/netfilter_ipv4/ipt_CONNMARK.h	Thu Jan  1 01:00:00 1970
++++ linux/include/linux/netfilter_ipv4/ipt_CONNMARK.h	Sun Jan 18 17:58:43 2004
+@@ -0,0 +1,25 @@
 +#ifndef _IPT_CONNMARK_H_target
 +#define _IPT_CONNMARK_H_target
 +
++/* Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
++ * by Henrik Nordstrom <hno@marasystems.com>
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; either version 2 of the License, or
++ * (at your option) any later version.
++ */
++
 +enum {
-+    IPT_CONNMARK_SET = 0,
-+    IPT_CONNMARK_SAVE,
-+    IPT_CONNMARK_RESTORE
++	IPT_CONNMARK_SET = 0,
++	IPT_CONNMARK_SAVE,
++	IPT_CONNMARK_RESTORE
 +};
 +
 +struct ipt_connmark_target_info {
 +	unsigned long mark;
++	unsigned long mask;
 +	u_int8_t mode;
 +};
 +
 +#endif /*_IPT_CONNMARK_H_target*/
-diff -uN --exclude .* --exclude *.o linux-2.4.3-pre3/net/ipv4/netfilter/ipt_CONNMARK.c linux-2.4.3-pre3-uml/net/ipv4/netfilter/ipt_CONNMARK.c
---- linux-2.4.3-pre3/net/ipv4/netfilter/ipt_CONNMARK.c	Thu Jan  1 01:00:00 1970
-+++ linux-2.4.3-pre3-uml/net/ipv4/netfilter/ipt_CONNMARK.c	Wed May 20 17:49:10 2001
-@@ -0,0 +1,87 @@
-+/* This is a module which is used for setting/remembering the mark field of
-+ * an connection, or optionally restore it to the skb
+diff -ruN --exclude CVS linux-orig/net/ipv4/netfilter/ipt_CONNMARK.c linux/net/ipv4/netfilter/ipt_CONNMARK.c
+--- linux-orig/net/ipv4/netfilter/ipt_CONNMARK.c	Thu Jan  1 01:00:00 1970
++++ linux/net/ipv4/netfilter/ipt_CONNMARK.c	Sun Jan 18 17:58:15 2004
+@@ -0,0 +1,118 @@
++/* This kernel module is used to modify the connection mark values, or
++ * to optionally restore the skb nfmark from the connection mark
++ *
++ * Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
++ * by Henrik Nordstrom <hno@marasystems.com>
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; either version 2 of the License, or
++ * (at your option) any later version.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program; if not, write to the Free Software
++ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 + */
 +#include <linux/module.h>
 +#include <linux/skbuff.h>
 +#include <linux/ip.h>
 +#include <net/checksum.h>
 +
++MODULE_AUTHOR("Henrik Nordstrom <hno@marasytems.com>");
++MODULE_DESCRIPTION("IP tables CONNMARK matching module");
++MODULE_LICENSE("GPL");
++
 +#include <linux/netfilter_ipv4/ip_tables.h>
 +#include <linux/netfilter_ipv4/ipt_CONNMARK.h>
 +#include <linux/netfilter_ipv4/ip_conntrack.h>
@@ -152,20 +220,29 @@
 +       void *userinfo)
 +{
 +	const struct ipt_connmark_target_info *markinfo = targinfo;
++	unsigned long diff;
++	unsigned long nfmark;
++	unsigned long newmark;
 +
 +	enum ip_conntrack_info ctinfo;
 +	struct ip_conntrack *ct = ip_conntrack_get((*pskb), &ctinfo);
 +	if (ct) {
 +	    switch(markinfo->mode) {
 +	    case IPT_CONNMARK_SET:
-+		ct->mark = markinfo->mark;
++		newmark = (ct->mark & ~markinfo->mask) | markinfo->mark;
++		if (newmark != ct->mark)
++		    ct->mark = newmark;
 +		break;
 +	    case IPT_CONNMARK_SAVE:
-+		ct->mark = (*pskb)->nfmark;
++		newmark = (ct->mark & ~markinfo->mask) | ((*pskb)->nfmark & markinfo->mask);
++		if (ct->mark != newmark)
++		    ct->mark = newmark;
 +		break;
 +	    case IPT_CONNMARK_RESTORE:
-+		if (ct->mark != (*pskb)->nfmark) {
-+		    (*pskb)->nfmark = ct->mark;
++		nfmark = (*pskb)->nfmark;
++		diff = (ct->mark ^ nfmark & markinfo->mask);
++		if (diff != 0) {
++		    (*pskb)->nfmark = nfmark ^ diff;
 +		    (*pskb)->nfcache |= NFC_ALTERED;
 +		}
 +		break;
@@ -178,9 +255,9 @@
 +static int
 +checkentry(const char *tablename,
 +	   const struct ipt_entry *e,
-+           void *targinfo,
-+           unsigned int targinfosize,
-+           unsigned int hook_mask)
++	   void *targinfo,
++	   unsigned int targinfosize,
++	   unsigned int hook_mask)
 +{
 +	struct ipt_connmark_target_info *matchinfo = targinfo;
 +	if (targinfosize != IPT_ALIGN(sizeof(struct ipt_connmark_target_info))) {
@@ -200,15 +277,16 @@
 +	return 1;
 +}
 +
-+static struct ipt_target ipt_connmark_reg
-+= { { NULL, NULL }, "CONNMARK", target, checkentry, NULL, THIS_MODULE };
++static struct ipt_target ipt_connmark_reg = {
++	.name = "CONNMARK",
++	.target = &target,
++	.checkentry = &checkentry,
++	.me = THIS_MODULE
++};
 +
 +static int __init init(void)
 +{
-+	if (ipt_register_target(&ipt_connmark_reg))
-+		return -EINVAL;
-+
-+	return 0;
++	return ipt_register_target(&ipt_connmark_reg);
 +}
 +
 +static void __exit fini(void)
Index: patch-o-matic/extra/CONNMARK.patch.configure.help
===================================================================
RCS file: /cvspublic/netfilter/patch-o-matic/extra/CONNMARK.patch.configure.help,v
retrieving revision 1.1
diff -u -p -r1.1 CONNMARK.patch.configure.help
--- patch-o-matic/extra/CONNMARK.patch.configure.help	21 May 2002 17:49:14 -0000	1.1
+++ patch-o-matic/extra/CONNMARK.patch.configure.help	2 Feb 2004 23:24:43 -0000
@@ -20,3 +20,8 @@
 CONFIP_IP_NF_MATCH_CONNMARK
   This option adds a `connmark' match, which allows you to match the
   connection mark value previously set for the session by `CONNMARK'. 
+
+  If you want to compile it as a module, say M here and read
+  Documentation/modules.txt.  The module will be called
+  ipt_connmark.o.  If unsure, say `N'.
+

Re: [patch, resent] Updated CONNMARK

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

On Tue, Feb 03, 2004 at 12:40:27AM +0100, Henrik Nordstrom wrote:
> Attached you find a slightly updated version of the previously submitted 
> patches to bring CONNMARK up to the level discussed for kernel submission. 
> You acknowledged getting the patches but the CVS is still not updated it 
> seems.

Sorry, I might have applied it to some local tree but never committed it
to CVS.  Seems to happen quite often when I'm mostly offline due to
travel :(

Now it's applied.

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch, resent] Updated CONNMARK

[Henrik Nordstrom]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 219 bytes --]

On Tue, 3 Feb 2004, Harald Welte wrote:

> Now it's applied.

Thanks!

Unfortunately I forgot the userspace includes in the patch I sent it
seems. Needed in case the kernel is not patched. Please apply.

Regards
Henrik

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2102 bytes --]

Index: userspace/include/linux/netfilter_ipv4/ipt_CONNMARK.h
===================================================================
RCS file: /cvspublic/iptables/include/linux/netfilter_ipv4/ipt_CONNMARK.h,v
retrieving revision 1.1
diff -u -p -r1.1 ipt_CONNMARK.h
--- userspace/include/linux/netfilter_ipv4/ipt_CONNMARK.h	23 Aug 2003 22:10:11 -0000	1.1
+++ userspace/include/linux/netfilter_ipv4/ipt_CONNMARK.h	3 Feb 2004 08:54:16 -0000
@@ -1,6 +1,15 @@
 #ifndef _IPT_CONNMARK_H_target
 #define _IPT_CONNMARK_H_target
 
+/* Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
 enum {
 	IPT_CONNMARK_SET = 0,
 	IPT_CONNMARK_SAVE,
@@ -9,6 +18,7 @@ enum {
 
 struct ipt_connmark_target_info {
 	unsigned long mark;
+	unsigned long mask;
 	u_int8_t mode;
 };
 
Index: userspace/include/linux/netfilter_ipv4/ipt_connmark.h
===================================================================
RCS file: /cvspublic/iptables/include/linux/netfilter_ipv4/ipt_connmark.h,v
retrieving revision 1.1
diff -u -p -r1.1 ipt_connmark.h
--- userspace/include/linux/netfilter_ipv4/ipt_connmark.h	23 Aug 2003 22:10:11 -0000	1.1
+++ userspace/include/linux/netfilter_ipv4/ipt_connmark.h	3 Feb 2004 08:54:16 -0000
@@ -1,6 +1,15 @@
 #ifndef _IPT_CONNMARK_H
 #define _IPT_CONNMARK_H
 
+/* Copyright (C) 2002,2004 MARA Systems AB <http://www.marasystems.com>
+ * by Henrik Nordstrom <hno@marasystems.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
 struct ipt_connmark_info {
 	unsigned long mark, mask;
 	u_int8_t invert;

Re: [patch, resent] Updated CONNMARK

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

On Tue, Feb 03, 2004 at 10:03:23AM +0100, Henrik Nordstrom wrote:
> On Tue, 3 Feb 2004, Harald Welte wrote:
> 
> > Now it's applied.
> 
> Thanks!
> 
> Unfortunately I forgot the userspace includes in the patch I sent it
> seems. Needed in case the kernel is not patched. Please apply.

applied.

> Regards
> Henrik
-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: iptables extension manpages

[Henrik Nordstrom]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 759 bytes --]

On Sun, 18 Jan 2004, Harald Welte wrote:

> well, we could do some semi-automated mechanism, like splitting it up in
> one file per extension, and then concatenate the whole manpage from
> those snippets depending on PF_EXT_LIBS / PF6_EXT_LIBS during the
> iptables userspace make process.

Attached is a proposed patch for doing this together with a small script
that splits the exiting manpages into the required snippets (included in 
the patch)

The script splits the manpages into the following components
     iptables.8.in (replaces iptables.8)
     ip6tables.8.in (replaces ip6tables.8)
     extensions/libipt_<extension>.man
     extensions/libip6t_<extension>.man

And the Makefile changes brings it all together again at build time

Regards
Henrik

[-- Attachment #2: Type: TEXT/PLAIN, Size: 5678 bytes --]

Index: split_manpages.sh
===================================================================
RCS file: split_manpages.sh
diff -N split_manpages.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ split_manpages.sh	18 Jan 2004 19:12:10 -0000
@@ -0,0 +1,70 @@
+#!/bin/sh
+
+cat iptables.8 | awk '
+BEGIN {
+	ext=""
+	out=""
+}
+/^.SH / {
+	ext=""
+	out=""
+}
+
+/^.SH .*EXTENSIONS/ {
+	ext=$2
+}
+
+/^.SS / {
+	if (ext != "") {
+		if (out == "")
+			print ".\\\" @" ext "@";
+		out=$2
+	}
+}
+
+{
+	if (out != "")
+		print >>"extensions/libipt_" out ".man"
+	else
+		print
+}
+' >iptables.8.in
+
+cat ip6tables.8 | awk '
+BEGIN {
+	ext=""
+	out=""
+}
+/^.SH / {
+	ext=""
+	out=""
+}
+
+/^.SH .*EXTENSIONS/ {
+	ext=$2
+}
+
+/^.SS / {
+	if (ext != "") {
+		if (out == "")
+			print ".\\\" @" ext "@";
+		out=$2
+	}
+}
+
+/^\.\\" .SS / {
+	if (ext != "") {
+		if (out == "")
+			print ".\\\" @" ext "@";
+		out=$3
+	}
+}
+
+
+{
+	if (out != "")
+		print >>"extensions/libip6t_" out ".man"
+	else
+		print
+}
+' >ip6tables.8.in
Index: Makefile
===================================================================
RCS file: /cvspublic/iptables/Makefile,v
retrieving revision 1.76
diff -u -p -r1.76 Makefile
--- Makefile	2 Nov 2003 17:01:09 -0000	1.76
+++ Makefile	18 Jan 2004 19:12:10 -0000
@@ -53,7 +53,7 @@ LDFLAGS      = -static
 LDLIBS       =
 endif
 
-EXTRAS+=iptables iptables.o
+EXTRAS+=iptables iptables.o iptables.8
 EXTRA_INSTALLS+=$(DESTDIR)$(BINDIR)/iptables $(DESTDIR)$(MANDIR)/man8/iptables.8
 
 # No longer experimental.
@@ -61,7 +61,7 @@ EXTRAS+=iptables-save iptables-restore
 EXTRA_INSTALLS+=$(DESTDIR)$(BINDIR)/iptables-save $(DESTDIR)$(BINDIR)/iptables-restore $(DESTDIR)$(MANDIR)/man8/iptables-restore.8 $(DESTDIR)$(MANDIR)/man8/iptables-save.8
 
 ifeq ($(DO_IPV6), 1)
-EXTRAS+=ip6tables ip6tables.o
+EXTRAS+=ip6tables ip6tables.o ip6tables.8
 EXTRA_INSTALLS+=$(DESTDIR)$(BINDIR)/ip6tables $(DESTDIR)$(MANDIR)/man8/ip6tables.8
 EXTRAS_EXP+=ip6tables-save ip6tables-restore
 EXTRA_INSTALLS_EXP+=$(DESTDIR)$(BINDIR)/ip6tables-save $(DESTDIR)$(BINDIR)/ip6tables-restore # $(DESTDIR)$(MANDIR)/man8/iptables-restore.8 $(DESTDIR)$(MANDIR)/man8/iptables-save.8 $(DESTDIR)$(MANDIR)/man8/ip6tables-save.8 $(DESTDIR)$(MANDIR)/man8/ip6tables-restore.8
@@ -147,6 +147,11 @@ EXTRA_DEPENDS+=iptables-standalone.d ipt
 iptables-standalone.d iptables.d: %.d: %.c
 	@-$(CC) -M -MG $(CFLAGS) $< | sed -e 's@^.*\.o:@$*.d $*.o:@' > $@
 
+iptables.8: iptables.8.in extensions/libipt_matches.man extensions/libipt_targets.man
+	sed -e '/@MATCH@/ r extensions/libipt_matches.man' -e '/@TARGET@/ r extensions/libipt_targets.man' iptables.8.in >iptables.8
+
+ip6tables.8: ip6tables.8.in extensions/libip6t_matches.man extensions/libip6t_targets.man
+	sed -e '/@MATCH@/ r extensions/libip6t_matches.man' -e '/@TARGET@/ r extensions/libiptt_targets.man' ip6tables.8.in >ip6tables.8
 
 # Development Targets
 .PHONY: install-devel-man3
Index: extensions/Makefile
===================================================================
RCS file: /cvspublic/iptables/extensions/Makefile,v
retrieving revision 1.35
diff -u -p -r1.35 Makefile
--- extensions/Makefile	8 Oct 2003 10:17:05 -0000	1.35
+++ extensions/Makefile	18 Jan 2004 19:12:10 -0000
@@ -18,6 +18,12 @@ PF6_EXT_SLIB+=$(PF6_EXT_SLIB_OPTS)
 OPTIONALS+=$(patsubst %,IPv4:%,$(PF_EXT_SLIB_OPTS))
 OPTIONALS+=$(patsubst %,IPv6:%,$(PF6_EXT_SLIB_OPTS))
 
+PF_EXT_MAN_MATCHES:=$(foreach T,$(PF_EXT_SLIB),$(shell test -f extensions/libipt_$(T).man && grep -q register_match extensions/libipt_$(T).c  && echo $(T)))
+PF_EXT_MAN_TARGETS:=$(foreach T,$(PF_EXT_SLIB),$(shell test -f extensions/libipt_$(T).man && grep -q register_target extensions/libipt_$(T).c && echo $(T)))
+
+PF6_EXT_MAN_MATCHES:=$(foreach T,$(PF6_EXT_SLIB),$(shell test -f extensions/libip6t_$(T).man && grep -q register_match6 extensions/libip6t_$(T).c && echo $(T)))
+PF6_EXT_MAN_TARGETS:=$(foreach T,$(PF6_EXT_SLIB),$(shell test -f extensions/libip6t_$(T).man && grep -q register_target6 extensions/libip6t_$(T).c && echo $(T)))
+
 ifndef NO_SHARED_LIBS
 SHARED_LIBS+=$(foreach T,$(PF_EXT_SLIB),extensions/libipt_$(T).so)
 EXTRA_INSTALLS+=$(foreach T, $(PF_EXT_SLIB), $(DESTDIR)$(LIBDIR)/iptables/libipt_$(T).so)
@@ -79,6 +85,22 @@ extensions/lib%.o: extensions/lib%.c
 
 endif
  
+EXTRAS += extensions/libipt_targets.man
+extensions/libipt_targets.man: $(patsubst %,extensions/libipt_%.c,$(PF_EXT_MAN_TARGETS))
+	cat $(patsubst %,extensions/libipt_%.man,$(PF_EXT_MAN_TARGETS)) >$@
+
+EXTRAS += extensions/libipt_matches.man
+extensions/libipt_matches.man: $(patsubst %,extensions/libipt_%.c,$(PF_EXT_MAN_MATCHES))
+	cat $(patsubst %,extensions/libipt_%.man,$(PF_EXT_MAN_MATCHES)) >$@
+
+EXTRAS += extensions/libip6t_targets.man
+extensions/libip6t_targets.man: $(patsubst %,extensions/libip6t_%.c,$(PF6_EXT_MAN_TARGETS))
+	cat $(patsubst %,extensions/libip6t_%.man,$(PF6_EXT_MAN_TARGETS)) >$@
+
+EXTRAS += extensions/libip6t_matches.man
+extensions/libip6t_matches.man: $(patsubst %,extensions/libip6t_%.c,$(PF6_EXT_MAN_MATCHES))
+	cat $(patsubst %,extensions/libip6t_%.man,$(PF6_EXT_MAN_MATCHES)) >$@
+
 $(DESTDIR)$(LIBDIR)/iptables/libipt_%.so: extensions/libipt_%.so
 	@[ -d $(DESTDIR)$(LIBDIR)/iptables ] || mkdir -p $(DESTDIR)$(LIBDIR)/iptables
 	cp $< $@
@@ -86,3 +108,5 @@ $(DESTDIR)$(LIBDIR)/iptables/libipt_%.so
 $(DESTDIR)$(LIBDIR)/iptables/libip6t_%.so: extensions/libip6t_%.so
 	@[ -d $(DESTDIR)$(LIBDIR)/iptables ] || mkdir -p $(DESTDIR)$(LIBDIR)/iptables
 	cp $< $@
+
+extensions/libipt_matches.man: 

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Tom Eastep]

On Saturday 17 January 2004 03:09 am, Harald Welte wrote:

>
> We had that idea for a very long time, and didn't ever change it for the
> skb->nfmark field for the sake of compatibility.
>
> But now, when introducing a new mark field (the conntrack->mark field),
> I'd rather prefer implement matching/setting individual bitmasks from
> the beginning.  What do you think?  Would you be willing to add that
> feature, or alternatively don't mind if I'd add the respective changes
> myself?

Harald,

I am very much in favor of the change you propose. The ability to set 
individual bits would allow Netfilter configuration tools like Shorewall to 
make internal use of packet marking by reserving part of the mark field for 
use by the tool and the remainder of the field for use by the user.

Given that the current MARK target lacks this capability, I am not able to 
make effective use of that target in Shorewall.

Thanks,
-Tom
-- 
Tom Eastep    \ Nothing is foolproof to a sufficiently talented fool
Shoreline,     \ http://shorewall.net
Washington USA  \ teastep@shorewall.net

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

On Sat, 17 Jan 2004, Tom Eastep wrote:

> I am very much in favor of the change you propose. The ability to set 
> individual bits would allow Netfilter configuration tools like Shorewall to 
> make internal use of packet marking by reserving part of the mark field for 
> use by the tool and the remainder of the field for use by the user.
> 
> Given that the current MARK target lacks this capability, I am not able to 
> make effective use of that target in Shorewall.

Please note that we are talkning about the CONNMARK target which is quite 
different from MARK. The two operates on different values. The discussed 
change will NOT add mask operations to the standard MARK target.

If you need mask operations in the standard MARK target then nothing stops
you from writing an extended MARK target having mask operations. It is
just that it can not be done easily in the standard kernel due to binary
compatibility issues.

Why there is not a extra class p-o-m patch to add mask capability to MARK 
I do not know. I am pretty sure the netfilter team would not mind if such 
patch is submitted, but as indicated above it can not progress beyond 
"extra" due to the frozen nature of the existing MARK target, at least not 
unless a different target name is used.

It is a pity the iptables match/target interface does not have versioning 
support of the target/match structures. Maybe 2.7 will..

Regards
Henrik

Re: Future of CONNMARK (was Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Tom Eastep]

On Sun, 18 Jan 2004, Henrik Nordstrom wrote:

> On Sat, 17 Jan 2004, Tom Eastep wrote:
>
> > I am very much in favor of the change you propose. The ability to set
> > individual bits would allow Netfilter configuration tools like Shorewall to
> > make internal use of packet marking by reserving part of the mark field for
> > use by the tool and the remainder of the field for use by the user.
> >
> > Given that the current MARK target lacks this capability, I am not able to
> > make effective use of that target in Shorewall.
>
> Please note that we are talkning about the CONNMARK target which is quite
> different from MARK. The two operates on different values. The discussed
> change will NOT add mask operations to the standard MARK target.
>

I realize that. I was simply stating that any type of packet/connection
marking facility that doesn't support the setting of individual bits
is of limited use to higher-level tools.

> If you need mask operations in the standard MARK target then nothing stops
> you from writing an extended MARK target having mask operations. It is
> just that it can not be done easily in the standard kernel due to binary
> compatibility issues.
>
> Why there is not a extra class p-o-m patch to add mask capability to MARK
> I do not know. I am pretty sure the netfilter team would not mind if such
> patch is submitted, but as indicated above it can not progress beyond
> "extra" due to the frozen nature of the existing MARK target, at least not
> unless a different target name is used.
>

I have strongly resisted making anything in Shorewall dependent on p-o-m
features. Supporting Shorewall takes enough of my time without having to
hand-hold newbies through kernel and iptables builds and installs.

> It is a pity the iptables match/target interface does not have versioning
> support of the target/match structures.

All the more reason to avoid p-o-m dependencies.

> Maybe 2.7 will..

We can hope...

-Tom
--
Tom Eastep    \ Nothing is foolproof to a sufficiently talented fool
Shoreline,     \ http://shorewall.net
Washington USA  \ teastep@shorewall.net

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Patrick McHardy]

Julian Anastasov wrote:

>- provide oif learned from the input route (as before the discussed
>change). May be in 99% of the setups it selects the right route.
>I think, we should use this, at least for 2.4.
>
>  
>
Why should we do a route lookup at all ? MASQUERADE doesn't need the
dst_entry but only the interface address. Using ifa_list->ifa_local
of the outgoing in_device seems like the simplest solution to me.

Regards,
Patrick

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Rusty Russell]

In message <4001F33A.9000802@trash.net> you write:
> Julian Anastasov wrote:
> 
> >- provide oif learned from the input route (as before the discussed
> >change). May be in 99% of the setups it selects the right route.
> >I think, we should use this, at least for 2.4.
> >
> >  
> >
> Why should we do a route lookup at all ? MASQUERADE doesn't need the
> dst_entry but only the interface address. Using ifa_list->ifa_local
> of the outgoing in_device seems like the simplest solution to me.

You take all the fun out.

Yes, this is the best.  It always does *something*, and is
predictable.  People with really complex routing shouldn't really use
MASQUERADE, since it's designed for a specific, simple case.

Patch?

Cheers,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

Rusty Russell wrote:
> You take all the fun out.
> 
> Yes, this is the best.  It always does *something*, and is
> predictable.  People with really complex routing shouldn't really use
> MASQUERADE, since it's designed for a specific, simple case.

I'm not sure about this, as Julian mentioned it breaks some setups,
but that's your decision.

> 
> Patch?

Patches for 2.4+2.6 attached. Both patches compile, 2.6 version is also
tested.

Best regards,
Patrick

> 
> Cheers,
> Rusty.
> --
>   Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
> 


[-- Attachment #2: 2.4-masq-ifa_local.diff --]
[-- Type: text/plain, Size: 2543 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/01/13 05:05:13+01:00 kaber@trash.net 
#   Use indev->ifa_list->ifa_local for MASQUERADE
# 
# net/ipv4/netfilter/ipt_MASQUERADE.c
#   2004/01/13 05:05:11+01:00 kaber@trash.net +16 -24
#   Use indev->ifa_list->ifa_local for MASQUERADE
# 
diff -Nru a/net/ipv4/netfilter/ipt_MASQUERADE.c b/net/ipv4/netfilter/ipt_MASQUERADE.c
--- a/net/ipv4/netfilter/ipt_MASQUERADE.c	Tue Jan 13 05:09:32 2004
+++ b/net/ipv4/netfilter/ipt_MASQUERADE.c	Tue Jan 13 05:09:32 2004
@@ -1,10 +1,13 @@
-/* Masquerade.  Simple mapping which alters range to a local IP address
-   (depending on route). */
+/* Masquerade.  Simple mapping which alters range to a local IP address. */
+#include <linux/kernel.h>
 #include <linux/config.h>
 #include <linux/types.h>
 #include <linux/ip.h>
 #include <linux/timer.h>
 #include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+#include <linux/inetdevice.h>
 #include <linux/netfilter.h>
 #include <net/protocol.h>
 #include <net/checksum.h>
@@ -67,9 +70,8 @@
 	enum ip_conntrack_info ctinfo;
 	const struct ip_nat_multi_range *mr;
 	struct ip_nat_multi_range newrange;
+	struct in_device *indev;
 	u_int32_t newsrc;
-	struct rtable *rt;
-	struct rt_key key;
 
 	IP_NF_ASSERT(hooknum == NF_IP_POST_ROUTING);
 
@@ -84,30 +86,20 @@
 
 	mr = targinfo;
 
-	key.dst = (*pskb)->nh.iph->daddr;
-	key.src = 0; /* Unknown: that's what we're trying to establish */
-	key.tos = RT_TOS((*pskb)->nh.iph->tos)|RTO_CONN;
-	key.oif = 0;
-#ifdef CONFIG_IP_ROUTE_FWMARK
-	key.fwmark = (*pskb)->nfmark;
-#endif
-	if (ip_route_output_key(&rt, &key) != 0) {
-                /* Funky routing can do this. */
-                if (net_ratelimit())
-                        printk("MASQUERADE:"
-                               " No route: Rusty's brain broke!\n");
-                return NF_DROP;
-        }
-        if (rt->u.dst.dev != out) {
-                if (net_ratelimit())
-                        printk("MASQUERADE:"
-                               " Route sent us somewhere else.\n");
+	indev = in_dev_get(out);
+	if (indev == NULL)
+		return NF_DROP;
+	read_lock(&indev->lock);
+	if (indev->ifa_list == NULL) {
+		read_unlock(&indev->lock);
+		in_dev_put(indev);
 		return NF_DROP;
 	}
+	newsrc = indev->ifa_list->ifa_local;
+	read_unlock(&indev->lock);
+	in_dev_put(indev);
 
-	newsrc = rt->rt_src;
 	DEBUGP("newsrc = %u.%u.%u.%u\n", NIPQUAD(newsrc));
-	ip_rt_put(rt);
 
 	WRITE_LOCK(&masq_lock);
 	ct->nat.masq_index = out->ifindex;

[-- Attachment #3: 2.6-masq-ifa_local.diff --]
[-- Type: text/plain, Size: 2415 bytes --]

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/01/13 05:08:46+01:00 kaber@trash.net 
#   Use indev->ifa_list->ifa_local for MASQUERADE
# 
# net/ipv4/netfilter/ipt_MASQUERADE.c
#   2004/01/13 05:05:49+01:00 kaber@trash.net +18 -28
#   Use indev->ifa_list->ifa_local for MASQUERADE
# 
diff -Nru a/net/ipv4/netfilter/ipt_MASQUERADE.c b/net/ipv4/netfilter/ipt_MASQUERADE.c
--- a/net/ipv4/netfilter/ipt_MASQUERADE.c	Tue Jan 13 05:09:17 2004
+++ b/net/ipv4/netfilter/ipt_MASQUERADE.c	Tue Jan 13 05:09:17 2004
@@ -1,10 +1,13 @@
-/* Masquerade.  Simple mapping which alters range to a local IP address
-   (depending on route). */
+/* Masquerade.  Simple mapping which alters range to a local IP address. */
+#include <linux/kernel.h>
 #include <linux/config.h>
 #include <linux/types.h>
 #include <linux/ip.h>
 #include <linux/timer.h>
 #include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+#include <linux/inetdevice.h>
 #include <linux/netfilter.h>
 #include <net/protocol.h>
 #include <net/checksum.h>
@@ -71,8 +74,8 @@
 	enum ip_conntrack_info ctinfo;
 	const struct ip_nat_multi_range *mr;
 	struct ip_nat_multi_range newrange;
+	struct in_device *indev;
 	u_int32_t newsrc;
-	struct rtable *rt;
 
 	IP_NF_ASSERT(hooknum == NF_IP_POST_ROUTING);
 
@@ -87,33 +90,20 @@
 
 	mr = targinfo;
 
-	{
-		struct flowi fl = { .nl_u = { .ip4_u =
-					      { .daddr = (*pskb)->nh.iph->daddr,
-						.tos = (RT_TOS((*pskb)->nh.iph->tos) |
-							RTO_CONN),
-#ifdef CONFIG_IP_ROUTE_FWMARK
-						.fwmark = (*pskb)->nfmark
-#endif
-					      } } };
-		if (ip_route_output_key(&rt, &fl) != 0) {
-			/* Funky routing can do this. */
-			if (net_ratelimit())
-				printk("MASQUERADE:"
-				       " No route: Rusty's brain broke!\n");
-			return NF_DROP;
-		}
-		if (rt->u.dst.dev != out) {
-			if (net_ratelimit())
-				printk("MASQUERADE:"
-				       " Route sent us somewhere else.\n");
-			return NF_DROP;
-		}
-	}
+	indev = in_dev_get(out);
+	if (indev == NULL)
+		return NF_DROP;
+	read_lock(&indev->lock);
+	if (indev->ifa_list == NULL) {
+		read_unlock(&indev->lock);
+		in_dev_put(indev);
+		return NF_DROP;
+	}
+	newsrc = indev->ifa_list->ifa_local;
+	read_unlock(&indev->lock);
+	in_dev_put(indev);
 
-	newsrc = rt->rt_src;
 	DEBUGP("newsrc = %u.%u.%u.%u\n", NIPQUAD(newsrc));
-	ip_rt_put(rt);
 
 	WRITE_LOCK(&masq_lock);
 	ct->nat.masq_index = out->ifindex;

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Julian Anastasov]

	Hello,

On Mon, 12 Jan 2004, Rusty Russell wrote:

> > Why should we do a route lookup at all ? MASQUERADE doesn't need the
> > dst_entry but only the interface address. Using ifa_list->ifa_local
> > of the outgoing in_device seems like the simplest solution to me.
>
> You take all the fun out.

	Selecting link local addresses is not always a fun :)
The IP addresses are sorted by their scope.

> Yes, this is the best.  It always does *something*, and is
> predictable.  People with really complex routing shouldn't really use
> MASQUERADE, since it's designed for a specific, simple case.

	Don't do that for 2.4, the users will not be happy.
There are setups with non-default multipath routes, multipath
routes with nexthops sharing same outdev, WAN interfaces which
have their private link layer IPs at first position.

	MASQUERADE is better for some cases where SNAT can
not be used at all: using source address autoselection based
on the scope and the nexthop's GW IP.

	I vote for backing out the oif change and to return
to the previous state - the routing is still smarter than any
iptables rules.

> Cheers,
> Rusty.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1783 bytes --]

On Mon, Jan 12, 2004 at 03:30:14PM +1100, Rusty Russell wrote:
> In message <4001F33A.9000802@trash.net> you write:
> > Julian Anastasov wrote:
> > 
> > >- provide oif learned from the input route (as before the discussed
> > >change). May be in 99% of the setups it selects the right route.
> > >I think, we should use this, at least for 2.4.
> > >
> > >  
> > >
> > Why should we do a route lookup at all ? MASQUERADE doesn't need the
> > dst_entry but only the interface address. Using ifa_list->ifa_local
> > of the outgoing in_device seems like the simplest solution to me.
> 
> You take all the fun out.
> 
> Yes, this is the best.  It always does *something*, and is
> predictable.  People with really complex routing shouldn't really use
> MASQUERADE, since it's designed for a specific, simple case.

No, I really disagree with that.  I still don't understand what the
problem was with the old solution.  We haven't received any complaints,
at least not that I can remember.

And as long as we don't provide a more sophisticated MASQUERADE
replacement target, we shouldn't change the behaviour at all.

You cannot use SNAT for the dynamic IP address case, because it doesn't
flush the tables.

And there are lots of users that have multiple DSL-dynip links these
days, trying to statically or dynamically balance web requests between
them, etc.

> Cheers,
> Rusty.

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Rusty Russell]

In message <20040113115428.GO20206@sunbeam.de.gnumonks.org> you write:
> > Yes, this is the best.  It always does *something*, and is
> > predictable.  People with really complex routing shouldn't really use
> > MASQUERADE, since it's designed for a specific, simple case.
> 
> No, I really disagree with that.  I still don't understand what the
> problem was with the old solution.  We haven't received any complaints,
> at least not that I can remember.

I'm not so sure.  We know Patrick's solution will work.  Yes it might
break things.

> And as long as we don't provide a more sophisticated MASQUERADE
> replacement target, we shouldn't change the behaviour at all.
> 
> You cannot use SNAT for the dynamic IP address case, because it doesn't
> flush the tables.

We should probably do "-j SNAT --dynamic" for this case.

> And there are lots of users that have multiple DSL-dynip links these
> days, trying to statically or dynamically balance web requests between
> them, etc.

In that case, the interfaces are different, (ppp0 vs ppp1) so no
problem.  You need something more complex to trigger the problem
AFAICT.

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Julian Anastasov]

	Hello,

On Mon, 12 Jan 2004, Patrick McHardy wrote:

> Why should we do a route lookup at all ? MASQUERADE doesn't need the
> dst_entry but only the interface address. Using ifa_list->ifa_local
> of the outgoing in_device seems like the simplest solution to me.

	Because it is possible the output interface to be used for
many logical subnets. In such cases it is desired maddr to be
the preferred source address for the target. If the users do not
want to fill the routing cache with such entries they can add
SNAT rule for each path. The drawback is that SNAT has only -o
match, there is no GW match. As result, we can not properly assign
maddr in setups that have two GWs on same output interface.

> Regards,
> Patrick

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[kuznet]

Hello!

> This means that ip_route_output_key() returns a route with a different
> outgoing interface than the skb->dst->dev of our to-be-masqueraded
> packet. =20
>
> Why was it wrong to specify skb->dst->dev->ifindex of the previous
> 'real' routing decision as key to our current routing decision?

Because in the situtation when ip_route_output_key() with oif=0 returns result
different of previous ip_route_output_key() you get undefined result.

I expected that you do ip_route_output_key() on _rewritten_ address,
in this case you have to _drop_ packet if routing contradicts to firewalling
rules or to send the packet via that interface which routng requires.
You cannot just blindly send packet to some random interface, when
firewalling rules are illegal.

> As an example case where I would suspect problems: The packet could be
> coming from a local socket, and the socket be bound to a specific
> interface (sk->bound_dev_if).

This is right observation. F.e. you could use skb->sk ? skb->sk->bound_dev_if : 0.

Alexey

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Julian Anastasov]

	Hello,

On Wed, 14 Jan 2004 kuznet@ms2.inr.ac.ru wrote:

> > Why was it wrong to specify skb->dst->dev->ifindex of the previous
> > 'real' routing decision as key to our current routing decision?
>
> Because in the situtation when ip_route_output_key() with oif=0 returns result
> different of previous ip_route_output_key() you get undefined result.
>
> I expected that you do ip_route_output_key() on _rewritten_ address,

	Nobody does output route on maddr, it is present only in 2.2

> in this case you have to _drop_ packet if routing contradicts to firewalling
> rules or to send the packet via that interface which routng requires.
> You cannot just blindly send packet to some random interface, when
> firewalling rules are illegal.

	Ah, no, you explain the 2.2 algorithm. 2.4 uses output
route only for the first packet, just to select maddr. While
2.2 uses 2nd route call by maddr for all next packets, 2.4
relies only on the routing cache for them.

	IMO, the rules should be:

1. packet should not change its outdev after routing (firewalling
expectation)

	In the discussed case, oif is provided only for the
1st packet in connection, providing oif=0 easily breaks
the above rule when multipath route is used - the input
route can return oif1 while the output route can return oif2.
Note the main thing: the goal before now was that oif is
provided to keep one packet to its outdev, not two packets
to same outdev, because:

2. different packets from one NAT connection still can be routed
through alive path (possibly different outdev) after the initial
path fails. This looks valid as long as we route by maddr which is
true for 2.2 but not for 2.4. Currently, such routing by maddr for
all next packets is not implemented in 2.4 with the excuse of
saving one routing call. With the current tools it can be done
with the cost of 2nd route call (the 1st is the input route).

	Now I'll remind you about my lsrc routing argument which
allows to route properly NAT connections by providing lsrc=maddr
in a ROUTING hook, only with single input route - a combination
from the current input and output route logic. It can be used
exactly from NAT because relying on the routing cache for binding
long living connections to its path is wrong.

	If this sounds complex, returning to the previous way
of providing oif still works better than now.

> Alexey

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

---------- Forwarded message ----------
Date: Wed, 14 Jan 2004 23:50:44 +0100 (CET)
From: Henrik Nordstrom <hno@marasystems.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Harald Welte <laforge@netfilter.org>
Subject: Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's
    brain broke!) 

On Wed, 14 Jan 2004, Rusty Russell wrote:

> > And there are lots of users that have multiple DSL-dynip links these
> > days, trying to statically or dynamically balance web requests between
> > them, etc.
> 
> In that case, the interfaces are different, (ppp0 vs ppp1) so no
> problem.  You need something more complex to trigger the problem
> AFAICT.

Consider a firewall connected with ethernet to multiple DSL links not
using PPP-over-ethernet.

But i can not say I consider such setup sane in the DSL-dynip case, nor
how to overcome such trivial issues such as address assignment as such
setups generally use DHCP over the modem to assign the address and it is a
little hard to direct the DHCP request to the correct modem over a
broadcast ethernet.

A better case is the case of having multiple links from the same ethernet 
and wanting to SNAT based on the route taken. Masquerade with route lookup 
does this kind of magically by the route local source address field.

Hmm.. doesn't the SKB already have the route information by the time of
POSTROUTING? If so, why not simply use the route information already
there? MASQUERADE should never change the route taken, only the source
address of the packet so there should not be a need for an additional 
route lookup, and as the packet is already routed there should be no need 
to dig into the interface to find addressing informations..

I.e. why not use something like the following to find the source address 
in MASQUERADE:

    ((struct rtable*)skb->dst)->rt_src

Only worry is if there is POSTROUTING cases where skb->dst is not an
rtable (superset of dst_entry).

Regards
Henrik

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Patrick McHardy]

Henrik Nordstrom wrote:
> I.e. why not use something like the following to find the source address 
> in MASQUERADE:
> 
>     ((struct rtable*)skb->dst)->rt_src

AFAICS in case of a input routed packet rt_src is the source ip of the
packet.

Best regards,
Patrick


> 
> Only worry is if there is POSTROUTING cases where skb->dst is not an
> rtable (superset of dst_entry).
> 
> Regards
> Henrik
> 

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

On Thu, 15 Jan 2004, Patrick McHardy wrote:

> Henrik Nordstrom wrote:
> > I.e. why not use something like the following to find the source address 
> > in MASQUERADE:
> > 
> >     ((struct rtable*)skb->dst)->rt_src
> 
> AFAICS in case of a input routed packet rt_src is the source ip of the
> packet.

MASQUERADE is never called in INPUT.

Regards
Henrik

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Patrick McHardy]

Henrik Nordstrom wrote:
> 
> MASQUERADE is never called in INPUT.

That is true, but if you want to save the routing call in MASQUERADE all
you've got is the route that was determined at input routing (for
non-local packets).

Regards,
Patrick


> 
> Regards
> Henrik
> 

Re: MASQUERADE: Route sent us somewhere else (was Re: Fw: Rusty's brain broke!)

[Henrik Nordstrom]

On Thu, 15 Jan 2004, Patrick McHardy wrote:

> That is true, but if you want to save the routing call in MASQUERADE all
> you've got is the route that was determined at input routing (for
> non-local packets).

Right.. so that shortcut can not be taken.

Regards
Henrik