Skip to contentSkip to conversation

Fix MASQUERADE

PATCH In review Rusty Russell 3 participants 4 messages Sep 7, 2004

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix MASQUERADE

Please test. Also resets assured bit.

Name: Change MASQUERADE to Use Device Address Directly
Status: Untested

Instead of doing a dubious route lookup, just use the first IP address of the (dynamic) interface. Also, reset assured bit so after masq connections can be cleaned up if memory pressure.

net/ipv4/netfilter/ipt_MASQUERADE.c +18 -29
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .2156-linux-2.6.9-rc1-bk12/net/ipv4/netfilter/ipt_MASQUERADE.c .2156-linux-2.6.9-rc1-bk12.updated/net/ipv4/netfilter/ipt_MASQUERADE.c
@@ -82,7 +82,6 @@ masquerade_target(struct sk_buff **pskb,
 	const struct ip_nat_multi_range *mr;
 	struct ip_nat_multi_range newrange;
 	u_int32_t newsrc;
-	struct rtable *rt;
 
 	IP_NF_ASSERT(hooknum == NF_IP_POST_ROUTING);
 
@@ -96,36 +95,12 @@ masquerade_target(struct sk_buff **pskb,
 	                    || ctinfo == IP_CT_RELATED + IP_CT_IS_REPLY));
 
 	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");
-			ip_rt_put(rt);
-			return NF_DROP;
-		}
+	newsrc = inet_select_addr(out, 0, RT_SCOPE_UNIVERSE);
+	if (!newsrc) {
+		printk("MASQUERADE: %s ate my IP address\n", out->name);
+		return NF_DROP;
 	}
 
-	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;
 	WRITE_UNLOCK(&masq_lock);
@@ -157,6 +132,18 @@ device_cmp(const struct ip_conntrack *i,
 	return ret;
 }
 
+static inline int
+connect_unassure(const struct ip_conntrack *i, void *_ina)
+{
+	struct in_ifaddr *ina = _ina;
+
+	/* We reset the ASSURED bit on all connections, so they will
+	 * get reaped under memory pressure. */
+	if (i->nat.masq_index == ina->ifa_dev->dev->ifindex)
+		clear_bit(IPS_ASSURED_BIT, (unsigned long *)&i->status);
+	return 0;
+}
+
 static int masq_inet_event(struct notifier_block *this,
 			   unsigned long event,
 			   void *ptr)
@@ -166,6 +153,8 @@ static int masq_inet_event(struct notifi
 	 * entries. */
 	if (event == NETDEV_UP)
 		ip_ct_selective_cleanup(device_cmp, ptr);
+	else if (event == NETDEV_DOWN)
+		ip_ct_selective_cleanup(connect_unassure, ptr);
 
 	return NOTIFY_DONE;
 }
Signature · 1 line
Anyone who quotes me in their signature is an idiot -- Rusty Russell
maintainer
#1 lore raw
Quoted 9 lines · On Wed, 08 Sep 2004 00:20:09 +1000 Rusty Russell <[email protected]> wrote:

Please test. Also resets assured bit.

Name: Change MASQUERADE to Use Device Address Directly
Status: Untested

Instead of doing a dubious route lookup, just use the first IP address of the (dynamic) interface. Also, reset assured bit so after masq connections can be cleaned up if memory pressure.

Didn't Herbert Xu et al. come to the conclusion that this isn't the way to fix this and that using inet_select_addr() had some problems especially in policy routing situations?

#2 lore raw
Quoted 4 lines · David S. Miller

Didn't Herbert Xu et al. come to the conclusion that this isn't the way to fix this and that using inet_select_addr() had some problems especially in policy routing situations?

No that was Julian Anastasov IIRC.

I agree with Rusty and Harald that MASQUERADE should be as simple as possible. So if you need complex routing to determine the correct source address, then you should use SNAT.

Now I haven't actually seen Rusty's patch but if it just uses inet_select_addr then it's definitely the way to go.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#3 lore raw
Quoted 6 lines · On Wed, 8 Sep 2004 07:30:36 +1000 Herbert Xu <[email protected]>…
Quoted 4 lines · David S. Miller

Didn't Herbert Xu et al. come to the conclusion that this isn't the way to fix this and that using inet_select_addr() had some problems especially in policy routing situations?

Quoted 8 lines

No that was Julian Anastasov IIRC.

I agree with Rusty and Harald that MASQUERADE should be as simple as possible. So if you need complex routing to determine the correct source address, then you should use SNAT.

Now I haven't actually seen Rusty's patch but if it just uses inet_select_addr then it's definitely the way to go.

Hmmm, ok. Rusty, please resend to me under private cover.