Re: [PATCH] MASQUERADE not flushing conntracks on ip change

[Phil Oester <kernel@linuxace.com>]

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

On Fri, Nov 05, 2004 at 08:15:09PM +0100, Patrick McHardy wrote:
> We only know is someone adds a true secondary address, not multiple
> primaries, otherwise we could just ignore it. Anyway, I agree we
> don't need to be overly friendly, I just don't see a case where this
> optimization does something useful. On ethernet devices, why delete
> the IP (if it didn't change) or set the interface down in the first
> place ? On ppp-interfaces, it doesn't work. Phil mentioned powercycling
> his dsl-/cablemodem would set his eth-interface down. I find that hard
> to believe, so I assume he didn't literally meant "my", but picked a
> bad example.
> 
> So, can anyone think of a setup where this optimization does work ?

Yes, I picked a bad example...I'll put down the crackpipe before typing
next time.

Anyway...below is what I'm thinking about now, which will handle both
the ppp case and the 'ip addr add' case.  Unfortunately, it's not working
on the ppp case due to some (IMO) unexpected behaviour from inet_confirm_addr.

Comments?

Phil



[-- Attachment #2: patch-test --]
[-- Type: text/plain, Size: 1467 bytes --]

--- linux-orig/net/ipv4/netfilter/ipt_MASQUERADE.c	2004-11-04 17:32:05.669856144 -0500
+++ linux-diff/net/ipv4/netfilter/ipt_MASQUERADE.c	2004-11-05 14:22:19.595596960 -0500
@@ -118,16 +118,15 @@
 }
 
 static inline int
-device_cmp(const struct ip_conntrack *i, void *_ina)
+device_cmp(const struct ip_conntrack *i, void *junk)
 {
 	int ret = 0;
-	struct in_ifaddr *ina = _ina;
 
 	READ_LOCK(&masq_lock);
-	/* If it's masquerading out this interface with a different address,
-	   or we don't know the new address of this interface. */
-	if (i->nat.masq_index == ina->ifa_dev->dev->ifindex
-	    && i->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip != ina->ifa_address)
+	/* If masquerading this conntrack but the masquerading ip
+	   no longer exists locally, drop conntrack. */
+	if (i->nat.masq_index && !(inet_confirm_addr(NULL, 0,
+	    i->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip, RT_SCOPE_HOST)))
 		ret = 1;
 	READ_UNLOCK(&masq_lock);
 
@@ -150,11 +149,10 @@
 			   unsigned long event,
 			   void *ptr)
 {
-	/* For some configurations, interfaces often come back with
-	 * the same address.  If not, clean up old conntrack
-	 * entries. */
+	/* In some configurations, interfaces come back with the
+	 * same address.  If not, clean up old conntrack entries. */
 	if (event == NETDEV_UP)
-		ip_ct_selective_cleanup(device_cmp, ptr);
+		ip_ct_selective_cleanup(device_cmp, NULL);
 	else if (event == NETDEV_DOWN)
 		ip_ct_selective_cleanup(connect_unassure, ptr);