[2.6 PATCH] ipchains masquerade must select maddr correctly

[2.6 PATCH] ipchains masquerade must select maddr correctly

[Julian Anastasov]

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


	Hello,

	The attached patch fixes ipchains masquerade to use
correctly the routing. This bug-to-bug compatibility with 2.2
is not valid from long time. Also, a missing unlock is added.

Regards

--
Julian Anastasov <ja@ssi.bg>

[-- Attachment #2: ipchains maddr --]
[-- Type: TEXT/PLAIN, Size: 1776 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1352  -> 1.1353 
#	net/ipv4/netfilter/ip_fw_compat_masq.c	1.11    -> 1.12   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/12/03	ja@ssi.bg	1.1353
# [IPCHAINS]: masquerade must select maddr correctly
# 
# - It is fixed in Linux 2.2 from ages
# - Add missing WRITE_UNLOCK on route error
# --------------------------------------------
#
diff -Nru a/net/ipv4/netfilter/ip_fw_compat_masq.c b/net/ipv4/netfilter/ip_fw_compat_masq.c
--- a/net/ipv4/netfilter/ip_fw_compat_masq.c	Wed Dec  3 00:32:05 2003
+++ b/net/ipv4/netfilter/ip_fw_compat_masq.c	Wed Dec  3 00:32:05 2003
@@ -67,18 +67,23 @@
 	/* Setup the masquerade, if not already */
 	if (!info->initialized) {
 		u_int32_t newsrc;
-		struct flowi fl = { .nl_u = { .ip4_u = { .daddr = (*pskb)->nh.iph->daddr } } };
+		struct flowi fl = { .nl_u = {
+					.ip4_u = {
+						.daddr = (*pskb)->nh.iph->daddr,
+						.tos = RT_TOS((*pskb)->nh.iph->tos),
+					} },
+				    .oif = (*pskb)->dst->dev->ifindex };
 		struct rtable *rt;
 		struct ip_nat_multi_range range;
 
 		/* Pass 0 instead of saddr, since it's going to be changed
 		   anyway. */
 		if (ip_route_output_key(&rt, &fl) != 0) {
+			WRITE_UNLOCK(&ip_nat_lock);
 			DEBUGP("ipnat_rule_masquerade: Can't reroute.\n");
 			return NF_DROP;
 		}
-		newsrc = inet_select_addr(rt->u.dst.dev, rt->rt_gateway,
-					  RT_SCOPE_UNIVERSE);
+		newsrc = rt->rt_src;
 		ip_rt_put(rt);
 		range = ((struct ip_nat_multi_range)
 			 { 1,

Re: [2.6 PATCH] ipchains masquerade must select maddr correctly

[David S. Miller]

On Wed, 3 Dec 2003 01:55:28 +0200 (EET)
Julian Anastasov <ja@ssi.bg> wrote:

> 	The attached patch fixes ipchains masquerade to use
> correctly the routing. This bug-to-bug compatibility with 2.2
> is not valid from long time. Also, a missing unlock is added.

Slow down.

I don't think it's always desirable to specify a specific TOS when
we're working with an input packet.  In fact, what you're doing all
over the tree is going to cause the routing cache size to explode in
some very real usage.

Re: [2.6 PATCH] ipchains masquerade must select maddr correctly

[Julian Anastasov]

	Hello,

On Tue, 2 Dec 2003, David S. Miller wrote:

> > 	The attached patch fixes ipchains masquerade to use
> > correctly the routing. This bug-to-bug compatibility with 2.2
> > is not valid from long time. Also, a missing unlock is added.
>
> Slow down.
>
> I don't think it's always desirable to specify a specific TOS when
> we're working with an input packet.  In fact, what you're doing all
> over the tree is going to cause the routing cache size to explode in
> some very real usage.

	Yes, it can grow up to 8 times (IPTOS_RT_MASK is 3 bits) if
we detect different rt tos values. In fact, ipchains is the only
case where tos is not provided :) For some users may be this is
not only a maddr selection, may be they have real routes by tos
for this public IP. Perhaps, TOS matching and hash key should be
a sysctl/compile time option? Then a site that does not use tos
for routing can safely run PMTUD without problems. I think, it is
a common case not to route by tos.

	The good news is that for ipchains this is in->out traffic
and may be there is only one tos value per path.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [2.6 PATCH] ipchains masquerade must select maddr correctly

[David S. Miller]

On Wed, 3 Dec 2003 02:50:56 +0200 (EET)
Julian Anastasov <ja@ssi.bg> wrote:

> TOS matching and hash key should be
> a sysctl/compile time option?

It already is, CONFIG_IP_ROUTE_TOS.  But the issue is that
all dist vendors enable that.

Also note that what you propose could potentially break
some setups.