xt_gateway 20070605 (kernel)

xt_gateway 20070605 (kernel)

[Jan Engelhardt]

Originally from Amin Azez <azez@ufomechanic.net>,
http://lists.netfilter.org/pipermail/netfilter-devel/2007-June/027954.html

This adds a gateway match to iptables that lets you match against the
routed ipv4 gateway, it's very useful for SNAT if you want to avoid
replicating your routing in your SNAT table.

e.g.

	iptables -t nat -A POSTROUTING -m gateway --nexthop 172.16.1.1 \
		-j SNAT --to-address 172.16.1.5
	iptables -t nat -A POSTROUTING -m gateway --nexthop 192.168.1.1 \
		-j SNAT --to-address 192.168.1.25

to help you choose the right SNAT address.

It works by comparing the to-be-matched gateway IP with the key in the
neighbor table of the next-hop (the key is the layer 3 address).

	--gateway 1.2.3.4

only matches if the packet is destined to 1.2.3.4 as a ROUTE, i.e.
1.2.3.4 is not also the target address.

	--nexthop 1.2.3.4

matches if the next hop is specified as 1.2.3.4 either as a gateway or
as a final destination.

It cannot do magic, and match on non-routed aliases of routers, it only
matches the targeted IP address from which the layer 2 address has been
(or will be) actually derived.


Signed-off-by: Jan Engelhardt <jengelh@gmx.de>
[Posted to LKML/NF-DEV on 2007-06-02, 2007-06-05]

---
 include/linux/netfilter/xt_gateway.h |   13 +++++
 net/netfilter/Kconfig                |    9 +++
 net/netfilter/Makefile               |    1 
 net/netfilter/xt_gateway.c           |   86 +++++++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+)

Index: linux-2.6.22-rc4/include/linux/netfilter/xt_gateway.h
===================================================================
--- /dev/null
+++ linux-2.6.22-rc4/include/linux/netfilter/xt_gateway.h
@@ -0,0 +1,13 @@
+#ifndef _XT_GATEWAY_H
+#define _XT_GATEWAY_H
+
+#define XT_GATEWAY_INV		0x1	/* Negate the condition */
+#define XT_GATEWAY_ROUTE	0x2	/* ...and the gateway is not the final hop */
+
+struct xt_gateway_info {
+	/* Inclusive: network order. */
+	uint32_t gateway;
+	uint8_t flags;
+};
+
+#endif /* _XT_GATEWAY_H */
Index: linux-2.6.22-rc4/net/netfilter/Kconfig
===================================================================
--- linux-2.6.22-rc4.orig/net/netfilter/Kconfig
+++ linux-2.6.22-rc4/net/netfilter/Kconfig
@@ -475,6 +475,15 @@ config NETFILTER_XT_MATCH_ESP
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config NETFILTER_XT_MATCH_GATEWAY
+	tristate '"gateway" match support'
+	depends on NETFILTER_XTABLES
+	---help---
+	  This option makes possible to match the IP address of the
+	  routed gateway for routed packets.
+
+	  To compile it as a module, choose M here. If unsure, say N.
+
 config NETFILTER_XT_MATCH_HELPER
 	tristate '"helper" match support'
 	depends on NETFILTER_XTABLES
Index: linux-2.6.22-rc4/net/netfilter/Makefile
===================================================================
--- linux-2.6.22-rc4.orig/net/netfilter/Makefile
+++ linux-2.6.22-rc4/net/netfilter/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_NETFILTER_XT_MATCH_CONNTRAC
 obj-$(CONFIG_NETFILTER_XT_MATCH_DCCP) += xt_dccp.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_DSCP) += xt_dscp.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_ESP) += xt_esp.o
+obj-$(CONFIG_NETFILTER_XT_MATCH_GATEWAY) += xt_gateway.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_HELPER) += xt_helper.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_LENGTH) += xt_length.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_LIMIT) += xt_limit.o
Index: linux-2.6.22-rc4/net/netfilter/xt_gateway.c
===================================================================
--- /dev/null
+++ linux-2.6.22-rc4/net/netfilter/xt_gateway.c
@@ -0,0 +1,86 @@
+/*
+ * netfilter module to match nexthop router by IP address
+ * (C) 2007 UFO Mechanic <azez@ufomechanic.net>
+ * © Jan Engelhardt <jengelh@gmx.de>, 2007
+ *   to save time and bugs, based on ip_range by
+ *   (C) 2003 Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/ip.h>
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/types.h>
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter/xt_gateway.h>
+#include <net/dst.h>
+#include <net/neighbour.h>
+
+static bool xt_gateway_match_it(const struct sk_buff *skb,
+				const struct xt_gateway_info *info)
+{
+        const struct iphdr *iph;
+	const struct dst_entry *dst;
+	const struct neighbour *neigh;
+	const struct neigh_table *tbl;
+
+	if (skb == NULL) /* necessary? */
+		return false;
+	if ((dst = skb->dst) == NULL)
+		return false;
+	if ((neigh = dst->neighbour) == NULL)
+		return false;
+	if ((tbl = neigh->tbl) == NULL)
+		return false;
+	if (tbl->family != AF_INET)
+		return false;
+	if (memcmp(&info->gateway, &neigh->primary_key, tbl->key_len) != 0)
+		return false;
+	if (!(info->flags & XT_GATEWAY_ROUTE))
+		return true;
+	iph = ip_hdr(skb);
+	if (iph->daddr != info->gateway)
+		return true;
+
+	return false;
+}
+
+static bool xt_gateway_match(const struct sk_buff *skb,
+			     const struct net_device *in,
+			     const struct net_device *out,
+			     const struct xt_match *match,
+			     const void *matchinfo, int offset,
+			     unsigned int protoff, bool *hotdrop)
+{
+	const struct xt_gateway_info *info = matchinfo;
+	return !!(info->flags & XT_GATEWAY_INV) ^
+	       xt_gateway_match_it(skb, info);
+}
+
+static struct xt_match xt_gateway_reg = {
+	.name      = "gateway",
+	.family    = AF_INET,
+	.match     = xt_gateway_match,
+	.matchsize = sizeof(struct xt_gateway_info),
+	.me        = THIS_MODULE,
+};
+
+static int __init xt_gateway_init(void)
+{
+	return xt_register_match(&xt_gateway_reg);
+}
+
+static void __exit xt_gateway_exit(void)
+{
+	xt_unregister_match(&xt_gateway_reg);
+	return;
+}
+
+module_init(xt_gateway_init);
+module_exit(xt_gateway_exit);
+MODULE_AUTHOR("Sam Liddicott <azez@ufomechanic.net>");
+MODULE_DESCRIPTION("netfilter nexthop/gateway match module");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("ipt_gateway");

Re: xt_gateway 20070605 (kernel)

[Jan Engelhardt]

Hi Sam,



(Style changes the way Patrick prefers it.)

Signed-off-by: Jan Engelhardt <jengelh@gmx.de>

---
 include/linux/netfilter/xt_gateway.h |   13 +++++
 net/netfilter/Kconfig                |    9 +++
 net/netfilter/Makefile               |    1 
 net/netfilter/xt_gateway.c           |   85 +++++++++++++++++++++++++++++++++++
 4 files changed, 108 insertions(+)

Index: linux-2.6.22-rc4/include/linux/netfilter/xt_gateway.h
===================================================================
--- /dev/null
+++ linux-2.6.22-rc4/include/linux/netfilter/xt_gateway.h
@@ -0,0 +1,13 @@
+#ifndef _XT_GATEWAY_H
+#define _XT_GATEWAY_H
+
+#define XT_GATEWAY_INV		0x1	/* Negate the condition */
+#define XT_GATEWAY_ROUTE	0x2	/* ...and the gateway is not the final hop */
+
+struct xt_gateway_info {
+	/* Inclusive: network order. */
+	u_int32_t gateway;
+	u_int8_t flags;
+};
+
+#endif /* _XT_GATEWAY_H */
Index: linux-2.6.22-rc4/net/netfilter/Kconfig
===================================================================
--- linux-2.6.22-rc4.orig/net/netfilter/Kconfig
+++ linux-2.6.22-rc4/net/netfilter/Kconfig
@@ -475,6 +475,15 @@ config NETFILTER_XT_MATCH_ESP
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config NETFILTER_XT_MATCH_GATEWAY
+	tristate '"gateway" match support'
+	depends on NETFILTER_XTABLES
+	---help---
+	  This option makes possible to match the IP address of the
+	  routed gateway for routed packets.
+
+	  To compile it as a module, choose M here. If unsure, say N.
+
 config NETFILTER_XT_MATCH_HELPER
 	tristate '"helper" match support'
 	depends on NETFILTER_XTABLES
Index: linux-2.6.22-rc4/net/netfilter/Makefile
===================================================================
--- linux-2.6.22-rc4.orig/net/netfilter/Makefile
+++ linux-2.6.22-rc4/net/netfilter/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_NETFILTER_XT_MATCH_CONNTRAC
 obj-$(CONFIG_NETFILTER_XT_MATCH_DCCP) += xt_dccp.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_DSCP) += xt_dscp.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_ESP) += xt_esp.o
+obj-$(CONFIG_NETFILTER_XT_MATCH_GATEWAY) += xt_gateway.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_HELPER) += xt_helper.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_LENGTH) += xt_length.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_LIMIT) += xt_limit.o
Index: linux-2.6.22-rc4/net/netfilter/xt_gateway.c
===================================================================
--- /dev/null
+++ linux-2.6.22-rc4/net/netfilter/xt_gateway.c
@@ -0,0 +1,85 @@
+/*
+ * netfilter module to match nexthop router by IP address
+ * (C) 2007 UFO Mechanic <azez@ufomechanic.net>
+ * © Jan Engelhardt <jengelh@gmx.de>, 2007
+ *   to save time and bugs, based on ip_range by
+ *   (C) 2003 Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/ip.h>
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/types.h>
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter/xt_gateway.h>
+#include <net/dst.h>
+#include <net/neighbour.h>
+
+static bool xt_gateway_match_it(const struct sk_buff *skb,
+				const struct xt_gateway_info *info)
+{
+	const struct iphdr *iph;
+	const struct dst_entry *dst;
+	const struct neighbour *neigh;
+	const struct neigh_table *tbl;
+
+	if (skb == NULL) /* necessary? */
+		return false;
+	if ((dst = skb->dst) == NULL)
+		return false;
+	if ((neigh = dst->neighbour) == NULL)
+		return false;
+	if ((tbl = neigh->tbl) == NULL)
+		return false;
+	if (tbl->family != AF_INET)
+		return false;
+	if (memcmp(&info->gateway, &neigh->primary_key, tbl->key_len) != 0)
+		return false;
+	if (!(info->flags & XT_GATEWAY_ROUTE))
+		return true;
+	iph = ip_hdr(skb);
+	if (iph->daddr != info->gateway)
+		return true;
+
+	return false;
+}
+
+static bool xt_gateway_match(const struct sk_buff *skb,
+			     const struct net_device *in,
+			     const struct net_device *out,
+			     const struct xt_match *match,
+			     const void *matchinfo, int offset,
+			     unsigned int protoff, bool *hotdrop)
+{
+	const struct xt_gateway_info *info = matchinfo;
+	return !!(info->flags & XT_GATEWAY_INV) ^
+	       xt_gateway_match_it(skb, info);
+}
+
+static struct xt_match xt_gateway_reg = {
+	.name      = "gateway",
+	.family    = AF_INET,
+	.match     = xt_gateway_match,
+	.matchsize = sizeof(struct xt_gateway_info),
+	.me        = THIS_MODULE,
+};
+
+static int __init xt_gateway_init(void)
+{
+	return xt_register_match(&xt_gateway_reg);
+}
+
+static void __exit xt_gateway_exit(void)
+{
+	xt_unregister_match(&xt_gateway_reg);
+}
+
+module_init(xt_gateway_init);
+module_exit(xt_gateway_exit);
+MODULE_AUTHOR("Sam Liddicott <azez@ufomechanic.net>");
+MODULE_DESCRIPTION("netfilter nexthop/gateway match module");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("ipt_gateway");

Re: xt_gateway 20070605 (kernel)

[Patrick McHardy]

Jan Engelhardt wrote:
> (Style changes the way Patrick prefers it.)
> 
> +	if ((dst = skb->dst) == NULL)
> +		return false;
> +	if ((neigh = dst->neighbour) == NULL)
> +		return false;


Actually I very much prefer to have assignments seperated
from comparisons, there just were no instances of this in
the patches I commented on :)

Most of the checks here are unnecessary anyway (we always
have a skb with skb->dst != NULL, neigh->tbl is always != NULL
and the neighbour table family is always AF_INET since the
match is only registered for AF_INET.

Re: xt_gateway 20070605 (kernel)

[Jan Engelhardt]

On Jun 5 2007 17:15, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> (Style changes the way Patrick prefers it.)
>> 
>> +	if ((dst = skb->dst) == NULL)
>> +		return false;
>> +	if ((neigh = dst->neighbour) == NULL)
>> +		return false;
>
>
>Actually I very much prefer to have assignments seperated
>from comparisons, there just were no instances of this in
>the patches I commented on :)

Well I amsure you can make an exception here, it's just so really simple.

>Most of the checks here are unnecessary anyway (we always
>have a skb with skb->dst != NULL, neigh->tbl is always != NULL
>and the neighbour table family is always AF_INET since the
>match is only registered for AF_INET.

Thanks for letting us now. Now, prime question: Is skb always != NULL?
(Otherwise it would not really make sense calling the match if
there was nothing to match.)


	Jan
-- 

Re: xt_gateway 20070605 (kernel)

[Phil Oester]

On Tue, Jun 05, 2007 at 01:17:57PM +0200, Jan Engelhardt wrote:
> 
> Originally from Amin Azez <azez@ufomechanic.net>,
> http://lists.netfilter.org/pipermail/netfilter-devel/2007-June/027954.html
> 
> This adds a gateway match to iptables that lets you match against the
> routed ipv4 gateway, it's very useful for SNAT if you want to avoid
> replicating your routing in your SNAT table.

Just a suggestion...for a while I've been needing the ability to match
on the routing table (but not just the gateway).  Could we perhaps name
this match 'route' instead (similar to the ROUTE target)?

Some of the things I'd like to be able to do is match on the length
of a route.  For instance we use lots of 10.x.x.x/30 nets internally
and I'd like to be able to match on them.  I haven't quite gotten around
to figuring out how to do this given the route cache doesn't include
prefix length, but I do think it would be useful.  I could see how
it could be combined with this gateway match.

Thoughts?

Phil

RE: xt_gateway 20070605 (kernel)

[Amin Azez]

I apologise again for top-posting and all the sins of pocket outlook quoting.

As best as I can recall without the source in front of me a route cache item attached to the skb on routing is cast to a neighbour table entry. (they are the same waist up). 

There is no way at forward or postrouting time to tell if the skb's attached neighbour table member is (was once) a route cache entry or just a plain neighbour table entry.

This means it is not safe (hard to know when it is safe) to cast it back to a route cache item to match what you need.

Perhaps yours is a case to use realm match as Patrick has mentioned.

What you want requires a change to the routing code so that it leaves evidence that the next hop is a route entry.

BTW the ROUTE target doesn't route so much as transmit. I hope to alter it so that in PREROUTING (or later) it actually adds a route to the skb, so that postrouting will work.

Sam

-----Original Message-----
From: "Phil Oester" <kernel@linuxace.com>
To: "Jan Engelhardt" <jengelh@linux01.gwdg.de>
Cc: "Amin Azez" <azez@ufomechanic.net>; "Netfilter Developer Mailing List" <netfilter-devel@lists.netfilter.org>
Sent: 05/06/07 23:24
Subject: Re: xt_gateway 20070605 (kernel)

On Tue, Jun 05, 2007 at 01:17:57PM +0200, Jan Engelhardt wrote:
> 
> Originally from Amin Azez <azez@ufomechanic.net>,
> http://lists.netfilter.org/pipermail/netfilter-devel/2007-June/027954.html
> 
> This adds a gateway match to iptables that lets you match against the
> routed ipv4 gateway, it's very useful for SNAT if you want to avoid
> replicating your routing in your SNAT table.

Just a suggestion...for a while I've been needing the ability to match
on the routing table (but not just the gateway).  Could we perhaps name
this match 'route' instead (similar to the ROUTE target)?

Some of the things I'd like to be able to do is match on the length
of a route.  For instance we use lots of 10.x.x.x/30 nets internally
and I'd like to be able to match on them.  I haven't quite gotten around
to figuring out how to do this given the route cache doesn't include
prefix length, but I do think it would be useful.  I could see how
it could be combined with this gateway match.

Thoughts?

Phil

Re: xt_gateway 20070605 (kernel)

[Patrick McHardy]

Jan Engelhardt wrote:
> On Jun 5 2007 17:15, Patrick McHardy wrote:
> 
>>Most of the checks here are unnecessary anyway (we always
>>have a skb with skb->dst != NULL, neigh->tbl is always != NULL
>>and the neighbour table family is always AF_INET since the
>>match is only registered for AF_INET.
> 
> 
> Thanks for letting us now. Now, prime question: Is skb always != NULL?
> (Otherwise it would not really make sense calling the match if
> there was nothing to match.)


Yes.

RE: xt_gateway 20070605 (kernel)

[Amin Azez]

So is it necessary to resubmit the kernel patch with the un-necessary matches turned into explanatory comments?

Sam

-----Original Message-----
From: "Patrick McHardy" <kaber@trash.net>
To: "Jan Engelhardt" <jengelh@linux01.gwdg.de>
Cc: "Netfilter Developer Mailing List" <netfilter-devel@lists.netfilter.org>; "Amin Azez" <azez@ufomechanic.net>
Sent: 06/06/07 12:31
Subject: Re: xt_gateway 20070605 (kernel)

Jan Engelhardt wrote:
> On Jun 5 2007 17:15, Patrick McHardy wrote:
> 
>>Most of the checks here are unnecessary anyway (we always
>>have a skb with skb->dst != NULL, neigh->tbl is always != NULL
>>and the neighbour table family is always AF_INET since the
>>match is only registered for AF_INET.
> 
> 
> Thanks for letting us now. Now, prime question: Is skb always != NULL?
> (Otherwise it would not really make sense calling the match if
> there was nothing to match.)


Yes.

Re: xt_gateway 20070605 (kernel)

[Patrick McHardy]

Amin Azez wrote:
> So is it necessary to resubmit the kernel patch with the un-necessary matches turned into explanatory comments?


I'd call them useless comments, so please no :) No, seriously,
I don't want to start that, people copy too much useless comments
around already. I don't want every match to state that skb is
never NULL and that its only called for AF_INET packets since
it only registered for AF_INET.

Please just remove them.

Re: xt_gateway 20070605 (kernel)

[Jan Engelhardt]

On Jun 15 2007 12:30, Amin Azez wrote:
>This didn't seem to turn up on the mailing list, so I'm re-sending:

It did, at least for me. (And I truly mean the copy that netfilter.org 
mailing list software sent me, since I still have that.)

>>> +	if ((dst = skb->dst) == NULL)
>>> +		return false;
>>> +	if ((neigh = dst->neighbour) == NULL)
>>> +		return false;    
>>
>> Actually I very much prefer to have assignments seperated
>> from comparisons, there just were no instances of this in
>> the patches I commented on :)
>>
>> Most of the checks here are unnecessary anyway (we always
>> have a skb with skb->dst != NULL, 
>maybe skb->dst = NULL on pre-routing?
>> neigh->tbl is always != NULL  
>same?
>> and the neighbour table family is always AF_INET since the
>> match is only registered for AF_INET.
>>   
>Maybe if I take out these checks, I should restrict the match to FORWARD
>and POSTROUTING?

Leaving them in does not cost too much.

>I'm being careful because while I am confident that what I submitted
>(and Jan modified for current kernels) is safe and works, I'm not
>confident that it will be safe with these changes.
>

	Jan
-- 

Re: xt_gateway 20070605 (kernel)

[Patrick McHardy]

Jan Engelhardt wrote:
> On Jun 15 2007 12:30, Amin Azez wrote:
> 
>>>and the neighbour table family is always AF_INET since the
>>>match is only registered for AF_INET.
>>>  
>>
>>Maybe if I take out these checks, I should restrict the match to FORWARD
>>and POSTROUTING?
> 
> 
> Leaving them in does not cost too much.


Thats no reason to keep pointless checks around.

RE: xt_gateway 20070605 (kernel)

[Amin Azez]

I agree that pointless checks are not needed at runtime.

I am not sure what other changes I should make to stop people crashing the kernel by using this match in places I have not forseen.

I am not certain that my anticpated crashes are actually possible.

Patrick, If I merely remove these unnecessary tests, will you be satisfied with the result?

Sam

-----Original Message-----
From: "Patrick McHardy" <kaber@trash.net>
To: "Jan Engelhardt" <jengelh@computergmbh.de>
Cc: "Amin Azez" <azez@ufomechanic.net>; "Netfilter Developer Mailing List" <netfilter-devel@lists.netfilter.org>
Sent: 15/06/07 17:15
Subject: Re: xt_gateway 20070605 (kernel)

Jan Engelhardt wrote:
> On Jun 15 2007 12:30, Amin Azez wrote:
> 
>>>and the neighbour table family is always AF_INET since the
>>>match is only registered for AF_INET.
>>>  
>>
>>Maybe if I take out these checks, I should restrict the match to FORWARD
>>and POSTROUTING?
> 
> 
> Leaving them in does not cost too much.


Thats no reason to keep pointless checks around.

Re: xt_gateway 20070605 (kernel)

[Patrick McHardy]

Amin Azez wrote:
> I agree that pointless checks are not needed at runtime.
> 
> I am not sure what other changes I should make to stop people crashing the kernel by using this match in places I have not forseen.


I can look over it again to make sure thats not possible.

> I am not certain that my anticpated crashes are actually possible.
> 
> Patrick, If I merely remove these unnecessary tests, will you be satisfied with the result?


Probably, I don't have the patch handy to look at it right now.
So far I don't intend to merge it, but I'm also not very opposed
to it (only reason against it is that its another file to take
care of when changing say function signatures and it doesn't
offer anything that we can't already do). Its up to you to
convince me :)

Re: xt_gateway 20070605 (kernel)

[Jan Engelhardt]

On Jun 15 2007 19:27, Patrick McHardy wrote:
>> I am not certain that my anticpated crashes are actually possible.
>> 
>> Patrick, If I merely remove these unnecessary tests, will you be satisfied with the result?
>
>
>Probably, I don't have the patch handy to look at it right now.

https://dev.computergmbh.de/svn/misc_kernel/xt_gateway/trunk/
You can choose either bare files (kernel/) or patch-based (patches/),
depending on your review taste :-)

>So far I don't intend to merge it, but I'm also not very opposed
>to it (only reason against it is that its another file to take
>care of when changing say function signatures and it doesn't
>offer anything that we can't already do). Its up to you to
>convince me :)



	Jan
-- 

RE: xt_gateway 20070605 (kernel)

[Amin Azez]

From: "Patrick McHardy" <kaber@trash.net>
Sent: 15/06/07 18:27
Subject: Re: xt_gateway 20070605 (kernel)

=So far I don't intend to merge it, but I'm also not very opposed
=to it (only reason against it is that its another file to take
=care of when changing say function signatures and it doesn't
=offer anything that we can't already do). Its up to you to
convince me :)

It does one thing well and simply.
For most iptables users, realms is more complicated than replicating route information in the nat table.

Realms are perhaps suitable with a large routing infrastructure under one administration.

Xt_gateway removes the need for the more advanced explanations of the SNAT target that are required from time to time.

Xt_gateway is simple to manage snat when the gateways are out of administrative control.

Xt_gateway is persisted solely with iptables-save.
There is no iproute-save (actually there is, I posted it to the relevant list a few months back but no-one noticed).

I'm not going to try to convince you harder than this. Some directors and shareholders (if they were aware) would probably prefer that you did NOT merge it to the mainline kernel and I also have a duty to them.

However I thank you (and more particularly Jan) for the time you have spent on this.

Sam

RE: xt_gateway 20070605 (kernel)

[Jan Engelhardt]

On Jun 16 2007 08:34, Amin Azez wrote:
>>So far I don't intend to merge it, but I'm also not very opposed
>>to it (only reason against it is that its another file to take
>>care of when changing say function signatures and it doesn't
>>offer anything that we can't already do). Its up to you to
>>convince me :)
>
>It does one thing well and simply. For most iptables users, realms is more
>complicated than replicating route information in the nat table.

Well, it is as complicated as policy routing
(e.g. using -j MARK and iproute2's fwmark match)

ip route add 1.3.3.7/32 dev leet0 realm 666
iptables -t nat -A POSTROUTING -m realm --realm 666 -j SNAT --to whatever.

>Realms are perhaps suitable with a large routing infrastructure under one
>administration.
>
>Xt_gateway removes the need for the more advanced explanations of the SNAT
>target that are required from time to time.
>
>Xt_gateway is simple to manage snat when the gateways are out of
>administrative control.
>
>Xt_gateway is persisted solely with iptables-save. There is no iproute-save
>(actually there is, I posted it to the relevant list a few months back but
>no-one noticed).

iproute-save would have a problem: it may interfere with
 (a) 'proto kernel' rules

 (b) rules from the distribution (e.g. /etc/sysconfig/network/ifroutes)
     [lesser a problem]

>I'm not going to try to convince you harder than this. Some directors and
>shareholders (if they were aware) would probably prefer that you did NOT merge
>it to the mainline kernel and I also have a duty to them.

What would they care about how it's done?



	Jan
-- 

RE: xt_gateway 20070605 (kernel)

[Amin Azez]

From: "Jan Engelhardt" <jengelh@computergmbh.de>
Sent: 16/06/07 08:59
Subject: RE: xt_gateway 20070605 (kernel)

=Well, it is as complicated as policy routing
=(e.g. using -j MARK and iproute2's fwmark match)
=
=ip route add 1.3.3.7/32 dev leet0 realm 666
=iptables -t nat -A POSTROUTING -m realm --realm 666 -j 
=SNAT --to whatever.

Yeah, you have to have one realm per snat-ip or better, which is more complicated than just using the gateway ip.

It's true that you can use realm to make an association between gateway and snat address, but why bother if you can use gateway to make an association between gateway and snat address.

To use realm requires fiddling with routing tables (which I can do pleasantly as iproute-save supports xml) but gateway match lets users leave routing to be managed by their network scripts instead of managing it by hand. 

=>Xt_gateway is persisted solely with iptables-save. There is no iproute-save
=>(actually there is, I posted it to the relevant list a few months back but
=>no-one noticed).

=iproute-save would have a problem: it may interfere with
= (a) 'proto kernel' rules

My iproute-restore did not restore proto kernel routes, or flush them prior to a restore.

= (b) rules from the distribution (e.g. =/etc/sysconfig/network/ifroutes)
=    [lesser a problem]

Likewise, scope helped recognize these.

=>I'm not going to try to convince you harder than this. Some directors and
=>shareholders (if they were aware) would probably prefer that you did NOT merge
=>it to the mainline kernel and I also have a duty to them.

=What would they care about how it's done?

They might prefer that anything that made things easier were not available outside of the source CD that we ship with the box. There is no official company policy on this yet, and I prefer it that way.

The gateway match makes things easier for me. All our customers will be using it. I'm happy to share it.

If Patrick judges it to be unneccessary then I don't feel very inclined to argue the point. I agree that authors cannot insist that their contributions be recommended upstream and linux is not so scarce of features that this is remotely necessary.

Like my iptables vlan match and conntrack direction match, account + rate limiting, and probably the same way as my next connroute target all of which are useful to me, I'm disapointed that they won't have wider use, but not frantic.

Sam

RE: xt_gateway 20070605 (kernel)

[Jan Engelhardt]

On Jun 16 2007 10:29, Amin Azez wrote:
>
>= (b) rules from the distribution (e.g. =/etc/sysconfig/network/ifroutes)
>=    [lesser a problem]
>
>Likewise, scope helped recognize these.

ip r: (created by)

10.0.254.1 via 10.0.254.5 dev tun0 (openvpn)
10.0.254.5 dev tun0  proto kernel  scope link  src 10.0.254.6 (openvpn)
192.168.254.0/24 via 10.0.254.5 dev tun0 (openvpn)
192.168.222.0/24 dev eth1  proto kernel  scope link  src 192.168.222.1 (distro)
192.168.60.0/22 dev vmnet1  proto kernel  scope link  src 192.168.60.1 (vmware)
10.10.96.0/20 dev eth0  proto kernel  scope link  src 10.10.106.161 (distro)
169.254.0.0/16 dev eth0  scope link (distro)
127.0.0.0/8 dev lo  scope link  (distro)
default via 10.10.96.1 dev eth0  (distro)

Hm! Not much to save :)

>>What would they care about how it's done?
>
>They might prefer that anything that made things easier were not available
>outside of the source CD that we ship with the box. There is no official
>company policy on this yet, and I prefer it that way.

One could obtain the CD and then redistribute the GPL components, and we'd
be where we are now. It does not buy the corp.s anything by limiting
distribution. In fact, spread it, and let others do the work ^_^.


	Jan
-- 

Re: xt_gateway 20070605 (kernel)

[Amin Azex]

Jan Engelhardt wrote:
> On Jun 16 2007 10:29, Amin Azez wrote:
>   
>> = (b) rules from the distribution (e.g. =/etc/sysconfig/network/ifroutes)
>> =    [lesser a problem]
>>
>> Likewise, scope helped recognize these.
>>     
>
> ip r: (created by)
>
> 10.0.254.1 via 10.0.254.5 dev tun0 (openvpn)
> 10.0.254.5 dev tun0  proto kernel  scope link  src 10.0.254.6 (openvpn)
> 192.168.254.0/24 via 10.0.254.5 dev tun0 (openvpn)
> 192.168.222.0/24 dev eth1  proto kernel  scope link  src 192.168.222.1 (distro)
> 192.168.60.0/22 dev vmnet1  proto kernel  scope link  src 192.168.60.1 (vmware)
> 10.10.96.0/20 dev eth0  proto kernel  scope link  src 10.10.106.161 (distro)
> 169.254.0.0/16 dev eth0  scope link (distro)
> 127.0.0.0/8 dev lo  scope link  (distro)
> default via 10.10.96.1 dev eth0  (distro)
>
> Hm! Not much to save :)
>   
And not much to save it with.
I didn't notice any realm attributes to be saved there :-) either.

(However, my routing has multiple rules and tables. I only use 
iproute-save for quick restore if application of new routes (xml) fails).

>>> What would they care about how it's done?
>>>       
>> They might prefer that anything that made things easier were not available
>> outside of the source CD that we ship with the box. There is no official
>> company policy on this yet, and I prefer it that way.
>>     
>
> One could obtain the CD and then redistribute the GPL components, and we'd
> be where we are now. It does not buy the corp.s anything by limiting
> distribution. 
Well it does.
Users have to buy the product to get _any_ of the advantages.

It also maintains the differential between the products and and 
joe-linux standard distribution, as well as between products and 
competitors linux-based products. It maximizes the value of the company 
and its products.

However, as I say, there is no actual policy on this (yet), hence my 
posts of code.
> In fact, spread it, 
that's what I'm trying to do.
> and let others do the work ^_^.
>   
With any luck I'll be one of those "others", and paid for by my employer.
We'll see...

Sam

Re: xt_gateway 20070605 (kernel)

[Amin Azex]

Amin Azex wrote:
> Jan Engelhardt wrote:
>
>> One could obtain the CD and then redistribute the GPL components, and 
>> we'd
>> be where we are now. It does not buy the corp.s anything by limiting
>> distribution. 
> Well it does.
> Users have to buy the product to get _any_ of the advantages.
>
Sorry, I wasn't too clear here. Of course one _could_ do that, but until 
one does it AND merges it with the main linux tree, the differential 
remains, and end-users have to buy the product to get any of the 
advantages (if they perceive there to be any).

Sam