Re: Troubles with MARK target in 2.6.28

Re: Troubles with MARK target in 2.6.28

[Patrick McHardy]

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

Nikolay S. Rybalov wrote:
> Hi all,
> 
> I have troubles with "-j MARK --set-xmark" combination in mangle table. 
> Particulary, I want set one bit in nf mark by means, say, --set-xmark 
> 0x10/0x10, and it worked in kernels from at least 2.6.24 up to the last 
> in 2.6.27.
> 
> I use: iptables -t mangle -A POSTROUTING -o eth1 -d 192.168.37.0/24 -j 
> MARK --set-xmark 0x10/0x10
> 
> and get:
> 
> iptables v1.4.2: Unknown arg `(null)'
> Try `iptables -h' or 'iptables --help' for more information.
> 
> When I try --or-mark, iptables says that my "kernel too old for --or-mark"
> 
> Same thing happens with -j CONNMARK.
> 
> Can someone advise what to do, I really need to set only one bit without 
> altering the rest of the mark.

Does this patch fix it?


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 412 bytes --]

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 89837a4..36cb63b 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -289,6 +289,10 @@ static int target_revfn(u8 af, const char *name, u8 revision, int *bestp)
 				have_rev = 1;
 		}
 	}
+
+	if (af != NFPROTO_UNSPEC && !have_rev)
+		return target_revfn(NFPROTO_UNSPEC, name, revision, bestp);
+
 	return have_rev;
 }
 

Re: Troubles with MARK target in 2.6.28

[Patrick McHardy]

Patrick McHardy wrote:
> Does this patch fix it?
> 

Jan, display of NFPROTO_UNSPEC targets and matches in
/proc/net/ip_tables_targets etc. is also broken. Please
have a look at this.

Re: Troubles with MARK target in 2.6.28

[Jan Engelhardt]

On Monday 2009-01-12 08:08, Patrick McHardy wrote:
> Patrick McHardy wrote:
>> Does this patch fix it?
>
> Jan, display of NFPROTO_UNSPEC targets and matches in
> /proc/net/ip_tables_targets etc. is also broken.

I can tell it is not (more like a missing feature if you will),
because /proc/net/ip_t* is for IPv4 only, and /proc/net/ip6_t*
is for IPv6 only. I had a patch somewhere that added a better
overview, let's see where in my git realms that disappeared...

Re: Troubles with MARK target in 2.6.28

[Patrick McHardy]

Jan Engelhardt wrote:
> On Monday 2009-01-12 08:08, Patrick McHardy wrote:
>> Patrick McHardy wrote:
>>> Does this patch fix it?
>> Jan, display of NFPROTO_UNSPEC targets and matches in
>> /proc/net/ip_tables_targets etc. is also broken.
> 
> I can tell it is not (more like a missing feature if you will),
> because /proc/net/ip_t* is for IPv4 only, and /proc/net/ip6_t*
> is for IPv6 only. I had a patch somewhere that added a better
> overview, let's see where in my git realms that disappeared...

Its supposed to show the targets and matches *available* for
a family. This includes NFPROTO_UNSPEC. This is the way people
are using it, so please fix this up.

Re: Troubles with MARK target in 2.6.28

[Jan Engelhardt]

On Monday 2009-01-12 08:18, Patrick McHardy wrote:
> Jan Engelhardt wrote:
>> On Monday 2009-01-12 08:08, Patrick McHardy wrote:
>>> Patrick McHardy wrote:
>>>> Does this patch fix it?
>>> Jan, display of NFPROTO_UNSPEC targets and matches in
>>> /proc/net/ip_tables_targets etc. is also broken.
>>
>> I can tell it is not (more like a missing feature if you will),
>> because /proc/net/ip_t* is for IPv4 only, and /proc/net/ip6_t*
>> is for IPv6 only. I had a patch somewhere that added a better
>> overview, let's see where in my git realms that disappeared...
>
> Its supposed to show the targets and matches *available* for
> a family.

Possibly - I found no comment in the source.
But the spartanic output of those proc files barely helps (especially
in light of multiple revisions), so here is the patch I spoke about,
refreshed and rebased, that solves the worries.

parent cc46eb3e855b7c1f628e934e01b97f4f2642973e (v2.6.29-rc1-22-gcc46eb3)
commit 11d60b1b555097e613ad6548b9b695a19735dda1
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Wed Jan 14 06:36:10 2009 +0100

netfilter: proc file for extension overview

Add a procfs file that dumps out the full Xtables target, match and
table info for all known nfprotos, superseding the IPv4/v6-only
/proc/net/ip{,6}_tables* which only provide names.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/netfilter/x_tables.c |  267 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 267 insertions(+), 0 deletions(-)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 89837a4..8d0dac7 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -26,6 +26,7 @@
 
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter_arp.h>
+#include <linux/netfilter_bridge.h>
 
 
 MODULE_LICENSE("GPL");
@@ -949,6 +950,260 @@ static const struct file_operations xt_target_ops = {
 #define	FORMAT_MATCHES	"_tables_matches"
 #define FORMAT_TARGETS 	"_tables_targets"
 
+struct xt_extinfo_trav {
+	struct seq_net_private seqnet;
+	struct list_head *head, *curr;
+	u_int8_t class, nfproto;
+};
+
+enum {
+	EXTINFO_TABLE_HDR,
+	EXTINFO_TABLE,
+	EXTINFO_TARGET_HDR,
+	EXTINFO_TARGET,
+	EXTINFO_MATCH_HDR,
+	EXTINFO_MATCH,
+	EXTINFO_LEAD_IN,
+	EXTINFO_DONE,
+};
+
+static void *xt_extinfo_seq_next(struct seq_file *seq, void *v, loff_t *ppos)
+{
+	static const u_int8_t next[] = {
+		[EXTINFO_TABLE_HDR]  = EXTINFO_TABLE,
+		[EXTINFO_TABLE]      = EXTINFO_TARGET_HDR,
+		[EXTINFO_TARGET_HDR] = EXTINFO_TARGET,
+		[EXTINFO_TARGET]     = EXTINFO_MATCH_HDR,
+		[EXTINFO_MATCH_HDR]  = EXTINFO_MATCH,
+		[EXTINFO_MATCH]      = EXTINFO_DONE,
+	};
+	struct xt_extinfo_trav *trav = seq->private;
+
+	switch (trav->class) {
+	case EXTINFO_LEAD_IN:
+		trav->class = EXTINFO_TABLE_HDR;
+		break;
+	case EXTINFO_TABLE_HDR:
+	case EXTINFO_TARGET_HDR:
+	case EXTINFO_MATCH_HDR:
+		trav->nfproto = 0;
+		mutex_lock(&xt[trav->nfproto].mutex);
+		switch (trav->class) {
+		case EXTINFO_TABLE_HDR:
+			trav->head = trav->curr =
+				&seq_file_net(seq)->xt.tables[trav->nfproto];
+			break;
+		case EXTINFO_TARGET_HDR:
+			trav->head = trav->curr = &xt[trav->nfproto].target;
+			break;
+		case EXTINFO_MATCH_HDR:
+			trav->head = trav->curr = &xt[trav->nfproto].match;
+			break;
+		}
+		trav->class = next[trav->class];
+		break;
+	case EXTINFO_TABLE:
+	case EXTINFO_TARGET:
+	case EXTINFO_MATCH:
+		trav->curr = trav->curr->next;
+		if (trav->curr != trav->head)
+			break; /* still in the game */
+
+		mutex_unlock(&xt[trav->nfproto].mutex);
+		if (++trav->nfproto >= NFPROTO_NUMPROTO) {
+			/* Done with category */
+			trav->class = next[trav->class];
+			break;
+		}
+
+		/* nfproto switchover */
+		mutex_lock(&xt[trav->nfproto].mutex);
+		switch (trav->class) {
+		case EXTINFO_TABLE:
+			trav->head = trav->curr =
+				&seq_file_net(seq)->xt.tables[trav->nfproto];
+			break;
+		case EXTINFO_TARGET:
+			trav->head = trav->curr = &xt[trav->nfproto].target;
+			break;
+		case EXTINFO_MATCH:
+			trav->head = trav->curr = &xt[trav->nfproto].match;
+			break;
+		}
+		break;
+	default:
+		return NULL;
+	}
+
+	if (ppos != NULL)
+		++*ppos;
+	return trav;
+}
+
+static void *xt_extinfo_seq_start(struct seq_file *seq, loff_t *pos)
+{
+	struct xt_extinfo_trav *trav = seq->private;
+	void *ret;
+	loff_t i;
+
+	trav->class = EXTINFO_LEAD_IN;
+	for (i = 0; i < *pos; ++i)
+		if ((ret = xt_extinfo_seq_next(seq, NULL, NULL)) == NULL)
+			return NULL;
+	return trav;
+}
+
+static const char *xt_extinfo_hooks(char *buf, unsigned int hook_mask)
+{
+	if (hook_mask == 0)
+		return "*";
+
+	buf[0] = (hook_mask & (1 << NF_INET_PRE_ROUTING))  ? 'R' : '-';
+	buf[1] = (hook_mask & (1 << NF_INET_LOCAL_IN))     ? 'I' : '-';
+	buf[2] = (hook_mask & (1 << NF_INET_FORWARD))      ? 'F' : '-';
+	buf[3] = (hook_mask & (1 << NF_INET_LOCAL_OUT))    ? 'O' : '-';
+	buf[4] = (hook_mask & (1 << NF_INET_POST_ROUTING)) ? 'S' : '-';
+	buf[5] = (hook_mask & (1 << NF_BR_BROUTING))       ? 'B' : '-';
+	buf[6] = '\0';
+	return buf;
+}
+
+static const char *xt_extinfo_nfproto(char *buf, u_int8_t p)
+{
+	static const char *const names[] = {
+		[NFPROTO_UNSPEC] = "*",
+		[NFPROTO_IPV4]   = "IPv4",
+		[NFPROTO_ARP]    = "ARP",
+		[NFPROTO_BRIDGE] = "bridge",
+		[NFPROTO_IPV6]   = "IPv6",
+		[NFPROTO_DECNET] = "DECnet",
+	};
+	if (p < ARRAY_SIZE(names))
+		return names[p];
+	/* buf is at least [4] big; and p's range is 0..255 */
+	sprintf(buf, "%u", p);
+	return buf;
+}
+
+static const char *xt_extinfo_l4proto(char *buf, u_int8_t p)
+{
+	/* IPPROTO_* can be a large value; not using an array here */
+	switch (p) {
+	case 0:			return "*";
+	case IPPROTO_AH:	return "ah";
+	case IPPROTO_DCCP:	return "dccp";
+	case IPPROTO_ESP:	return "esp";
+	case IPPROTO_ICMP:	return "icmp";
+	case IPPROTO_ICMPV6:	return "icmpv6";
+	case IPPROTO_SCTP:	return "sctp";
+	case IPPROTO_TCP:	return "tcp";
+	case IPPROTO_UDP:	return "udp";
+	case IPPROTO_UDPLITE:	return "udplite";
+	case IPPROTO_MH:	return "mh";
+	default:
+		sprintf(buf, "%u", p);
+		return buf;
+	}
+}
+
+static int xt_extinfo_seq_show(struct seq_file *seq, void *v)
+{
+	struct xt_extinfo_trav *trav = seq->private;
+	const struct xt_table *table;
+	const struct xt_target *target;
+	const struct xt_match *match;
+	char hooks[7], nfproto[4], l4proto[4];
+
+	switch (trav->class) {
+	case EXTINFO_LEAD_IN:
+		return seq_printf(seq, "# format version 1\n");
+	case EXTINFO_TABLE_HDR:
+		return seq_printf(seq, "# -- Tables --\n"
+		       "# type\t" "%-13s\t\t\t" "nfproto\t\t"
+		       "hooks\n", "Name");
+	case EXTINFO_TABLE:
+		if (trav->curr == trav->head)
+			return 0;
+		table = list_entry(trav->curr, struct xt_table, list);
+		/*
+		 * The leading type identifier is so that sorting the output
+		 * produces a meaningful result, i.e. that targets remain
+		 * distinguishable from matches even when mixed.
+		 */
+		return seq_printf(seq, "table\t%-15s\t\t\t%s\t\t%s\n",
+		       table->name, xt_extinfo_nfproto(nfproto, table->af),
+		       xt_extinfo_hooks(hooks, table->valid_hooks));
+	case EXTINFO_TARGET_HDR:
+		return seq_printf(seq, "# -- Targets --\n"
+		       "# type\t" "%-13s\t" "rev\t" "table\t" "nfproto\t"
+		       "l4proto\t" "hooks\t" "tgsize\n", "Name");
+	case EXTINFO_TARGET:
+		if (trav->curr == trav->head)
+			return 0;
+		target = list_entry(trav->curr, struct xt_target, list);
+		/*
+		 * Last field's %d is correct despite it being unsigned -
+		 * "-1" might be used in exceptions.
+		 */
+		return seq_printf(seq, "target\t%-15s\t%u\t%s\t%s\t"
+		       "%s\t%s\t%6d\n",
+		       (*target->name == '\0') ? "(standard)" : target->name,
+		       target->revision, target->table ? : "*",
+		       xt_extinfo_nfproto(nfproto, target->family),
+		       xt_extinfo_l4proto(l4proto, target->proto),
+		       xt_extinfo_hooks(hooks, target->hooks),
+		       target->targetsize);
+	case EXTINFO_MATCH_HDR:
+		return seq_printf(seq, "# -- Matches --\n"
+		       "# type\t" "%-13s\t" "rev\t" "table\t" "nfproto\t"
+		       "l4proto\t" "hooks\t" "mtsize\n", "Name");
+	case EXTINFO_MATCH:
+		if (trav->curr == trav->head)
+			return 0;
+		match = list_entry(trav->curr, struct xt_match, list);
+		return seq_printf(seq, "match\t%-15s\t%u\t%s\t%s\t"
+		       "%s\t%s\t%6d\n",
+		       match->name, match->revision, match->table ? : "*",
+		       xt_extinfo_nfproto(nfproto, match->family),
+		       xt_extinfo_l4proto(l4proto, match->proto),
+		       xt_extinfo_hooks(hooks, match->hooks), match->matchsize);
+	}
+	return 0;
+}
+
+static void xt_extinfo_seq_stop(struct seq_file *seq, void *v)
+{
+	struct xt_extinfo_trav *trav = seq->private;
+
+	switch (trav->class) {
+	case EXTINFO_TABLE:
+	case EXTINFO_TARGET:
+	case EXTINFO_MATCH:
+		mutex_unlock(&xt[trav->nfproto].mutex);
+	}
+	/* in all other cases, no lock is currently held */
+}
+
+static const struct seq_operations xt_extinfo_seq_ops = {
+	.start	= xt_extinfo_seq_start,
+	.next	= xt_extinfo_seq_next,
+	.stop	= xt_extinfo_seq_stop,
+	.show	= xt_extinfo_seq_show,
+};
+
+static int xt_extinfo_open(struct inode *inode, struct file *file)
+{
+	return seq_open_net(inode, file, &xt_extinfo_seq_ops,
+	       sizeof(struct xt_extinfo_trav));
+}
+
+static const struct file_operations xt_extinfo_fops = {
+	.open    = xt_extinfo_open,
+	.read    = seq_read,
+	.llseek  = seq_lseek,
+	.release = seq_release_net,
+};
+
 #endif /* CONFIG_PROC_FS */
 
 int xt_proto_init(struct net *net, u_int8_t af)
@@ -1025,15 +1280,27 @@ EXPORT_SYMBOL_GPL(xt_proto_fini);
 
 static int __net_init xt_net_init(struct net *net)
 {
+	struct proc_dir_entry *proc;
 	int i;
 
+	proc = proc_create_data("xtables_info", S_IRUGO, net->proc_net,
+	       &xt_extinfo_fops, NULL);
+	if (proc == NULL)
+		return -EINVAL;
+
 	for (i = 0; i < NFPROTO_NUMPROTO; i++)
 		INIT_LIST_HEAD(&net->xt.tables[i]);
 	return 0;
 }
 
+static void __net_exit xt_net_exit(struct net *net)
+{
+	remove_proc_entry("xtables_info", net->proc_net);
+}
+
 static struct pernet_operations xt_net_ops = {
 	.init = xt_net_init,
+	.exit = xt_net_exit,
 };
 
 static int __init xt_init(void)
-- 
# Created with git-export-patch

Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[Jan Engelhardt]

On Wednesday 2009-01-14 06:39, Jan Engelhardt wrote:
>parent cc46eb3e855b7c1f628e934e01b97f4f2642973e (v2.6.29-rc1-22-gcc46eb3)
>commit 11d60b1b555097e613ad6548b9b695a19735dda1
>Author: Jan Engelhardt <jengelh@medozas.de>
>Date:   Wed Jan 14 06:36:10 2009 +0100
>
>netfilter: proc file for extension overview
>
>Add a procfs file that dumps out the full Xtables target, match and
>table info for all known nfprotos, superseding the IPv4/v6-only
>/proc/net/ip{,6}_tables* which only provide names.

With this patch of mine I was able to identify a small discrepancy
(/proc/net/ip* would not have told me as it just prints names):

># -- Targets --
># type	Name         	rev	table	nfproto	l4proto	hooks	tgsize
>target	MARK           	2	*	*	*	*	     8
>target	MARK           	1	mangle	*	*	*	     8
>target	MARK           	0	mangle	*	*	*	     4

Namely that MARK.2 is available for all tables. It looks like an error,
given that the previous ones were all limited to the mangle table.
But, I would have to ask - what do we gain from limiting it to mangle?
All other *MARK targets are available for all tables too, so what was
the original reason for the table limit?

I could imagine it having to do with routing (nfmark can be used as
a routing key, as can TOS/DSCP):

>target	TOS            	1	mangle	IPv4	*	*	     2
>target	TOS            	0	mangle	IPv4	*	*	     1
>target	DSCP           	0	mangle	IPv4	*	*	     1

then again, MARK has more uses than just for routing; it can, for example,
serve as a way to reduce the number of rules by remembering some previous
result.
What do others think?

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[James King]

On Thu, Jan 15, 2009 at 12:06 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
> Namely that MARK.2 is available for all tables. It looks like an error,
> given that the previous ones were all limited to the mangle table.
> But, I would have to ask - what do we gain from limiting it to mangle?
> All other *MARK targets are available for all tables too, so what was
> the original reason for the table limit?
>
> I could imagine it having to do with routing (nfmark can be used as
> a routing key, as can TOS/DSCP):
>
>>target TOS             1       mangle  IPv4    *       *            2
>>target TOS             0       mangle  IPv4    *       *            1
>>target DSCP            0       mangle  IPv4    *       *            1
>
> then again, MARK has more uses than just for routing; it can, for example,
> serve as a way to reduce the number of rules by remembering some previous
> result.
> What do others think?

The only place I can see in the mangle table where nfmark has any
special consideration is in ipt_local_hook(), in case the local output
packet needs to be rerouted, but it seems a bit heavy handed to
restrict MARK there based on that one edge case.  It might be useful
to have it available elsewhere, for example to be able to refine the
mark as it passes through the different tables, especially now that
the mark is maskable.

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[Patrick McHardy]

James King wrote:
> On Thu, Jan 15, 2009 at 12:06 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
>> Namely that MARK.2 is available for all tables. It looks like an error,
>> given that the previous ones were all limited to the mangle table.
>> But, I would have to ask - what do we gain from limiting it to mangle?
>> All other *MARK targets are available for all tables too, so what was
>> the original reason for the table limit?
>>
>> I could imagine it having to do with routing (nfmark can be used as
>> a routing key, as can TOS/DSCP):
>>
>>> target TOS             1       mangle  IPv4    *       *            2
>>> target TOS             0       mangle  IPv4    *       *            1
>>> target DSCP            0       mangle  IPv4    *       *            1
>> then again, MARK has more uses than just for routing; it can, for example,
>> serve as a way to reduce the number of rules by remembering some previous
>> result.
>> What do others think?
> 
> The only place I can see in the mangle table where nfmark has any
> special consideration is in ipt_local_hook(), in case the local output
> packet needs to be rerouted, but it seems a bit heavy handed to
> restrict MARK there based on that one edge case.  It might be useful
> to have it available elsewhere, for example to be able to refine the
> mark as it passes through the different tables, especially now that
> the mark is maskable.

Agreed, it doesn't make sense to restrict it to mangle only.

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[Jan Engelhardt]

On Thursday 2009-01-15 14:47, Patrick McHardy wrote:
>>> Namely that MARK.2 is available for all tables. It looks like an error,
>>> given that the previous ones were all limited to the mangle table.
>>> But, I would have to ask - what do we gain from limiting it to mangle?
>>>[...]
>>> I could imagine it having to do with routing (nfmark can be used as
>>> a routing key, as can TOS/DSCP):
>>>[...]
>>> What do others think?
>
> Agreed, it doesn't make sense to restrict it to mangle only.
>
Are there perhaps other targets besides MARK whose table restriction
should be relaxed?

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[James King]

On Thu, Jan 15, 2009 at 8:44 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
>
> On Thursday 2009-01-15 14:47, Patrick McHardy wrote:
>>>> Namely that MARK.2 is available for all tables. It looks like an error,
>>>> given that the previous ones were all limited to the mangle table.
>>>> But, I would have to ask - what do we gain from limiting it to mangle?
>>>>[...]
>>>> I could imagine it having to do with routing (nfmark can be used as
>>>> a routing key, as can TOS/DSCP):
>>>>[...]
>>>> What do others think?
>>
>> Agreed, it doesn't make sense to restrict it to mangle only.
>>
> Are there perhaps other targets besides MARK whose table restriction
> should be relaxed?

Could TOS/DSCP just call ip_route_me_harder() directly when necessary
instead of relying on the mangle hook to do so?  This would allow it
to be used everywhere.

Also, I would have thought TTL to be a considered field when
determining if a reroute is needed, but it looks like only mark, iph,
saddr, daddr, and tos are checked currently, so there doesn't seem to
be an obvious benefit to restricting that target to mangle.

With those targets available everywhere, it might allow for future
collapse of the tables, since mangle would become somewhat redundant.

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[Patrick McHardy]

James King wrote:
> On Thu, Jan 15, 2009 at 8:44 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
>> On Thursday 2009-01-15 14:47, Patrick McHardy wrote:
>>>>> Namely that MARK.2 is available for all tables. It looks like an error,
>>>>> given that the previous ones were all limited to the mangle table.
>>>>> But, I would have to ask - what do we gain from limiting it to mangle?
>>>>> [...]
>>>>> I could imagine it having to do with routing (nfmark can be used as
>>>>> a routing key, as can TOS/DSCP):
>>>>> [...]
>>>>> What do others think?
>>> Agreed, it doesn't make sense to restrict it to mangle only.
>>>
>> Are there perhaps other targets besides MARK whose table restriction
>> should be relaxed?
> 
> Could TOS/DSCP just call ip_route_me_harder() directly when necessary
> instead of relying on the mangle hook to do so?  This would allow it
> to be used everywhere.

That doesn't seem like a good idea. Rerouting should be an optional
feature, available in the (misnamed) mangle table. There might be
completely different reasons for changing DSCP. So making them available
in other tables yes, making them responsible for rerouting no.

> 
> Also, I would have thought TTL to be a considered field when
> determining if a reroute is needed, but it looks like only mark, iph,
> saddr, daddr, and tos are checked currently, so there doesn't seem to
> be an obvious benefit to restricting that target to mangle.
> 
> With those targets available everywhere, it might allow for future
> collapse of the tables, since mangle would become somewhat redundant.

Yes, lets just remove all those silly restrictions and just keep
the few ones that actually have a technical reason.

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[James King]

On Fri, Jan 16, 2009 at 12:04 AM, Patrick McHardy <kaber@trash.net> wrote:
>>> Are there perhaps other targets besides MARK whose table restriction
>>> should be relaxed?
>>
>> Could TOS/DSCP just call ip_route_me_harder() directly when necessary
>> instead of relying on the mangle hook to do so?  This would allow it
>> to be used everywhere.
>
> That doesn't seem like a good idea. Rerouting should be an optional
> feature, available in the (misnamed) mangle table. There might be
> completely different reasons for changing DSCP. So making them available
> in other tables yes, making them responsible for rerouting no.

True, however, I was mainly thinking about the confusion that might
result if it becomes available in all tables.  For instance, someone
who relies on reroute after DSCP change (which is implicit on output)
accidentally omits the table specifier, causing the rule to default to
filter.  Or if mangle can be made entirely superfluous by
unrestricting all other targets, is there any sense in keeping it
around for a single target?  Maybe adding a new revision that allows
for explicit rerouting after the target action (such as
--set-dscp-and-reroute) might address this?

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[Patrick McHardy]

Jan Engelhardt wrote:
> On Thursday 2009-01-15 14:47, Patrick McHardy wrote:
>>>> Namely that MARK.2 is available for all tables. It looks like an error,
>>>> given that the previous ones were all limited to the mangle table.
>>>> But, I would have to ask - what do we gain from limiting it to mangle?
>>>> [...]
>>>> I could imagine it having to do with routing (nfmark can be used as
>>>> a routing key, as can TOS/DSCP):
>>>> [...]
>>>> What do others think?
>> Agreed, it doesn't make sense to restrict it to mangle only.
>>
> Are there perhaps other targets besides MARK whose table restriction
> should be relaxed?

I can think of CONNMARK, CLASSIFY, TCPOPTSTRIP for consistency with
TCPMSS and possibly CONNSECMARK (after consulting with James Morris).

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[Jan Engelhardt]

On Friday 2009-01-16 08:33, Patrick McHardy wrote:
>> On Thursday 2009-01-15 14:47, Patrick McHardy wrote:
>>>>> Namely that MARK.2 is available for all tables. It looks like an error,
>>>>> given that the previous ones were all limited to the mangle table.
>>>>> But, I would have to ask - what do we gain from limiting it to mangle?
>>>>> [...]
>>>>> I could imagine it having to do with routing (nfmark can be used as
>>>>> a routing key, as can TOS/DSCP):
>>>>> [...]
>>>>> What do others think?
>>> Agreed, it doesn't make sense to restrict it to mangle only.
>>>
>> Are there perhaps other targets besides MARK whose table restriction
>> should be relaxed?
>
> I can think of CONNMARK, CLASSIFY, TCPOPTSTRIP for consistency with
> TCPMSS and possibly CONNSECMARK (after consulting with James Morris).
>
connmark is already relaxed, as is connsecmark.

And so I wonder what purpose the mangle table has, other than being 
before routing.

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[Patrick McHardy]

Jan Engelhardt wrote:
> On Friday 2009-01-16 08:33, Patrick McHardy wrote:
>>> On Thursday 2009-01-15 14:47, Patrick McHardy wrote:
>>>>>> Namely that MARK.2 is available for all tables. It looks like an error,
>>>>>> given that the previous ones were all limited to the mangle table.
>>>>>> But, I would have to ask - what do we gain from limiting it to mangle?
>>>>>> [...]
>>>>>> I could imagine it having to do with routing (nfmark can be used as
>>>>>> a routing key, as can TOS/DSCP):
>>>>>> [...]
>>>>>> What do others think?
>>>> Agreed, it doesn't make sense to restrict it to mangle only.
>>>>
>>> Are there perhaps other targets besides MARK whose table restriction
>>> should be relaxed?
>> I can think of CONNMARK, CLASSIFY, TCPOPTSTRIP for consistency with
>> TCPMSS and possibly CONNSECMARK (after consulting with James Morris).
>>
> connmark is already relaxed, as is connsecmark.
> 
> And so I wonder what purpose the mangle table has, other than being 
> before routing.

It does explicit rerouting in LOCAL_OUT. It should have been named
"route" in my opinion.

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[James King]

On Fri, Jan 16, 2009 at 12:15 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
>>> Are there perhaps other targets besides MARK whose table restriction
>>> should be relaxed?
>>
>> I can think of CONNMARK, CLASSIFY, TCPOPTSTRIP for consistency with
>> TCPMSS and possibly CONNSECMARK (after consulting with James Morris).
>>
> connmark is already relaxed, as is connsecmark.

secmark and connsecmark don't specifiy the table in their xt_target
structure, but they do restrict to either the mangle or security table
in their tg_check functions.  connmark appears to work in other
tables.

A quick grep shows that the following targets are restricted to the
mangle table in some fashion:

ipt_TTL
ipt_ECN
ip6t_HL (maybe we could merge this together with TTL?)
CLASSIFY
DSCP
SECMARK
CONNSECMARK
MARK
CONNMARK
TPROXY
TCPOPTSTRIP

Also, REJECT is restricted to the filter table, although I'm not sure
it would be useful elsewhere.

Re: Targets with "mangle" table limiting (Was: Re: Troubles with MARK target in 2.6.28)

[Patrick McHardy]

James King wrote:
> On Fri, Jan 16, 2009 at 12:15 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
>>>> Are there perhaps other targets besides MARK whose table restriction
>>>> should be relaxed?
>>> I can think of CONNMARK, CLASSIFY, TCPOPTSTRIP for consistency with
>>> TCPMSS and possibly CONNSECMARK (after consulting with James Morris).
>>>
>> connmark is already relaxed, as is connsecmark.
> 
> secmark and connsecmark don't specifiy the table in their xt_target
> structure, but they do restrict to either the mangle or security table
> in their tg_check functions.  connmark appears to work in other
> tables.
> 
> A quick grep shows that the following targets are restricted to the
> mangle table in some fashion:
> 
> ipt_TTL
> ipt_ECN
> ip6t_HL (maybe we could merge this together with TTL?)
> CLASSIFY
> DSCP
> SECMARK
> CONNSECMARK
> MARK
> CONNMARK
> TPROXY
> TCPOPTSTRIP
> 
> Also, REJECT is restricted to the filter table, although I'm not sure
> it would be useful elsewhere.

We currently only support "restricted to a specific table" or
"not restricted at all". To avoid people misusing it in the
NAT table I'd rather keep that restriction.

Just lifting the mangle restrictions seems OK for now.

Re: Troubles with MARK target in 2.6.28

[Patrick McHardy]

Jan Engelhardt wrote:
> On Monday 2009-01-12 08:18, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>> I can tell it is not (more like a missing feature if you will),
>>> because /proc/net/ip_t* is for IPv4 only, and /proc/net/ip6_t*
>>> is for IPv6 only. I had a patch somewhere that added a better
>>> overview, let's see where in my git realms that disappeared...
 >>
>> Its supposed to show the targets and matches *available* for
>> a family.
> 
> Possibly - I found no comment in the source.

It is older than xtables and has always worked that way. It doesn't
make sense to have the contents dependant on implementation details.
I also know that some UIs rely on having available modules shown,
IIRC one of them was shorewall. This needs to be fixed.

> But the spartanic output of those proc files barely helps (especially
> in light of multiple revisions), so here is the patch I spoke about,
> refreshed and rebased, that solves the worries.

That seems a bit overkill. Text-based representation also doesn't seem
ideal, iptables already has this information and a cmdline user will
simply use "iptables ... -h" to get the supported option. For other
programs a binary representation would probably be easier to handle.

Re: Troubles with MARK target in 2.6.28

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Jan Engelhardt wrote:
>> But the spartanic output of those proc files barely helps (especially
>> in light of multiple revisions), so here is the patch I spoke about,
>> refreshed and rebased, that solves the worries.
> 
> That seems a bit overkill. Text-based representation also doesn't seem
> ideal, iptables already has this information and a cmdline user will
> simply use "iptables ... -h" to get the supported option. For other
> programs a binary representation would probably be easier to handle.

Agreed. The text-based interface seems to me like yet-another-interface
that users may (ab)use (like /proc/net/nf_conntrack, I have seen people
polling from it to display information, this was OK in the early days).
Following this basis, one may come and implement another text-based
interface to display the rule-set in the kernel and so on.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: Troubles with MARK target in 2.6.28

[Jan Engelhardt]

On Thursday 2009-01-15 15:06, Pablo Neira Ayuso wrote:
>Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>> But the spartanic output of those proc files barely helps (especially
>>> in light of multiple revisions), so here is the patch I spoke about,
>>> refreshed and rebased, that solves the worries.
>> 
>> That seems a bit overkill. Text-based representation also doesn't seem
>> ideal, iptables already has this information and a cmdline user will
>> simply use "iptables ... -h" to get the supported option. For other
>> programs a binary representation would probably be easier to handle.
>
>Agreed. The text-based interface seems to me like yet-another-interface
>that users may (ab)use (like /proc/net/nf_conntrack, I have seen people
>polling from it to display information, this was OK in the early days).
>Following this basis, one may come and implement another text-based
>interface to display the rule-set in the kernel and so on.

But is there an equivalent user tool and interface at this time
to obtain the same information?

Re: Troubles with MARK target in 2.6.28

[Patrick McHardy]

Jan Engelhardt wrote:
> On Thursday 2009-01-15 15:06, Pablo Neira Ayuso wrote:
>> Patrick McHardy wrote:
>>> Jan Engelhardt wrote:
>>>> But the spartanic output of those proc files barely helps (especially
>>>> in light of multiple revisions), so here is the patch I spoke about,
>>>> refreshed and rebased, that solves the worries.
>>> That seems a bit overkill. Text-based representation also doesn't seem
>>> ideal, iptables already has this information and a cmdline user will
>>> simply use "iptables ... -h" to get the supported option. For other
>>> programs a binary representation would probably be easier to handle.
>> Agreed. The text-based interface seems to me like yet-another-interface
>> that users may (ab)use (like /proc/net/nf_conntrack, I have seen people
>> polling from it to display information, this was OK in the early days).
>> Following this basis, one may come and implement another text-based
>> interface to display the rule-set in the kernel and so on.
> 
> But is there an equivalent user tool and interface at this time
> to obtain the same information?

No that I know of, but that doesn't matter as long as nobody has a
need for it.

Re: Troubles with MARK target in 2.6.28

[Jan Engelhardt]

On Thursday 2009-01-15 16:54, Patrick McHardy wrote:
>>> Agreed. The text-based interface seems to me like yet-another-interface
>>> that users may (ab)use (like /proc/net/nf_conntrack, I have seen people
>>> polling from it to display information, this was OK in the early days).
>>> Following this basis, one may come and implement another text-based
>>> interface to display the rule-set in the kernel and so on.
>>
>> But is there an equivalent user tool and interface at this time
>> to obtain the same information?
>
> No that I know of, but that doesn't matter as long as nobody has a
> need for it.
>
As a developer of Xtables modules I have a need  ;-)

Re: Troubles with MARK target in 2.6.28

[Patrick McHardy]

Jan Engelhardt wrote:
> On Thursday 2009-01-15 16:54, Patrick McHardy wrote:
>>>> Agreed. The text-based interface seems to me like yet-another-interface
>>>> that users may (ab)use (like /proc/net/nf_conntrack, I have seen people
>>>> polling from it to display information, this was OK in the early days).
>>>> Following this basis, one may come and implement another text-based
>>>> interface to display the rule-set in the kernel and so on.
>>> But is there an equivalent user tool and interface at this time
>>> to obtain the same information?
>> No that I know of, but that doesn't matter as long as nobody has a
>> need for it.
>>
> As a developer of Xtables modules I have a need  ;-)

That was one of my first thoughts, it looks mainly useful for
developers. And I'm not very fond of the idea of adding almost
300 lines of segfile beauty for that.

Re: Troubles with MARK target in 2.6.28

[Jan Engelhardt]

On Thursday 2009-01-15 17:03, Patrick McHardy wrote:
> Jan Engelhardt wrote:
>> On Thursday 2009-01-15 16:54, Patrick McHardy wrote:
>>>>> Agreed. The text-based interface seems to me like yet-another-interface
>>>>> that users may (ab)use (like /proc/net/nf_conntrack, I have seen people
>>>>> polling from it to display information, this was OK in the early days).
>>>>> Following this basis, one may come and implement another text-based
>>>>> interface to display the rule-set in the kernel and so on.
>>>> But is there an equivalent user tool and interface at this time
>>>> to obtain the same information?
>>> No that I know of, but that doesn't matter as long as nobody has a
>>> need for it.
>>>
>> As a developer of Xtables modules I have a need  ;-)
>
> That was one of my first thoughts, it looks mainly useful for
> developers. And I'm not very fond of the idea of adding almost
> 300 lines of segfile beauty for that.
>
Then you probably won't like it if I say that the same logic is
needed if I shall make the existing proc-files multi-nfproto aware.

Re: Troubles with MARK target in 2.6.28

[Patrick McHardy]

Jan Engelhardt wrote:
> On Thursday 2009-01-15 17:03, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>> On Thursday 2009-01-15 16:54, Patrick McHardy wrote:
>>>>>> Agreed. The text-based interface seems to me like yet-another-interface
>>>>>> that users may (ab)use (like /proc/net/nf_conntrack, I have seen people
>>>>>> polling from it to display information, this was OK in the early days).
>>>>>> Following this basis, one may come and implement another text-based
>>>>>> interface to display the rule-set in the kernel and so on.
>>>>> But is there an equivalent user tool and interface at this time
>>>>> to obtain the same information?
>>>> No that I know of, but that doesn't matter as long as nobody has a
>>>> need for it.
>>>>
>>> As a developer of Xtables modules I have a need  ;-)
>> That was one of my first thoughts, it looks mainly useful for
>> developers. And I'm not very fond of the idea of adding almost
>> 300 lines of segfile beauty for that.
>>
> Then you probably won't like it if I say that the same logic is
> needed if I shall make the existing proc-files multi-nfproto aware.

What does "multi-nfproto aware" mean exactly?

Re: Troubles with MARK target in 2.6.28

[Jan Engelhardt]

On Friday 2009-01-16 08:33, Patrick McHardy wrote:
>>>> As a developer of Xtables modules I have a need  ;-)
>>> That was one of my first thoughts, it looks mainly useful for
>>> developers. And I'm not very fond of the idea of adding almost
>>> 300 lines of segfile beauty for that.
>>>
>> Then you probably won't like it if I say that the same logic is
>> needed if I shall make the existing proc-files multi-nfproto aware.
>
> What does "multi-nfproto aware" mean exactly?

The thing is, /proc/net/ip_tables_matches's seq_start function takes
the lock, lets it iterate, and drops the lock on seq_stop. That is
easy for a single list (xt[af].match), but when you want to make
matches from more than just one nfproto visible in that file, you
need to drop the lock during seq_next, and acquire the next one too.
Not only that, but you also have to switch the list you are
traversing, which is currently conveniently passed through the "v"
variable. And you need to keep track of the nfproto whose list you
are iterating through, because seq_stop can be called with either one
currently being traversed.

Re: Troubles with MARK target in 2.6.28

[Nikolay S. Rybaloff]

That did the trick. Both MARK and CONNMARK are working like expected.

--------------------------------------------------
From: "Patrick McHardy" <kaber@trash.net>
Sent: Monday, January 12, 2009 9:19 AM
To: "Nikolay S. Rybalov" <nowhere@hakkenden.ath.cx>
Cc: <netfilter@vger.kernel.org>; "Netfilter Development Mailinglist" 
<netfilter-devel@vger.kernel.org>; "Jan Engelhardt" <jengelh@medozas.de>
Subject: Re: Troubles with MARK target in 2.6.28

> Nikolay S. Rybalov wrote:
>> Hi all,
>>
>> I have troubles with "-j MARK --set-xmark" combination in mangle table.
>> Particulary, I want set one bit in nf mark by means, say, --set-xmark
>> 0x10/0x10, and it worked in kernels from at least 2.6.24 up to the last
>> in 2.6.27.
>>
>> I use: iptables -t mangle -A POSTROUTING -o eth1 -d 192.168.37.0/24 -j
>> MARK --set-xmark 0x10/0x10
>>
>> and get:
>>
>> iptables v1.4.2: Unknown arg `(null)'
>> Try `iptables -h' or 'iptables --help' for more information.
>>
>> When I try --or-mark, iptables says that my "kernel too old 
>> for --or-mark"
>>
>> Same thing happens with -j CONNMARK.
>>
>> Can someone advise what to do, I really need to set only one bit without
>> altering the rest of the mark.
>
> Does this patch fix it?
>
>



> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index 89837a4..36cb63b 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -289,6 +289,10 @@ static int target_revfn(u8 af, const char *name, u8 
> revision, int *bestp)
>  have_rev = 1;
>  }
>  }
> +
> + if (af != NFPROTO_UNSPEC && !have_rev)
> + return target_revfn(NFPROTO_UNSPEC, name, revision, bestp);
> +
>  return have_rev;
> }
>
>