diff mbox

ipv4: remove all rt cache entries on UNREGISTER event

Message ID 1285751929.2615.30.camel@edumazet-laptop
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet Sept. 29, 2010, 9:18 a.m. UTC 
I found following patch was enough to avoid route being created if
device is down. This is still racy and needs more thinking.




--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Nicolas Dichtel Sept. 30, 2010, 11:49 a.m. UTC | #1 
Patch works well with my case.
In fact, it's more proper to returns an error to the daemon to let it 
knows that packet was not sent.

Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Regards,
Nicolas

Eric Dumazet wrote:
> I found following patch was enough to avoid route being created if
> device is down. This is still racy and needs more thinking.
> 
> 
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index ac6559c..1ee0b1a 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2586,9 +2586,10 @@ static int ip_route_output_slow(struct net *net, struct rtable **rp,
>  			goto out;
>  
>  		/* RACE: Check return value of inet_select_addr instead. */
> -		if (__in_dev_get_rtnl(dev_out) == NULL) {
> +		if (!(dev_out->flags & IFF_UP) || __in_dev_get_rtnl(dev_out) == NULL) {
>  			dev_put(dev_out);
> -			goto out;	/* Wrong error code */
> +			err = -ENETUNREACH;
> +			goto out;
>  		}
>  
>  		if (ipv4_is_local_multicast(oldflp->fl4_dst) ||
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Dichtel Dec. 22, 2010, 8:32 a.m. UTC | #2 
What is the status of this patch? The initial problem is still here in 2.6.37-rc5+


Regards,
Nicolas

Le 29.09.2010 11:18, Eric Dumazet a écrit :
> I found following patch was enough to avoid route being created if
> device is down. This is still racy and needs more thinking.
> 
> 
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index ac6559c..1ee0b1a 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2586,9 +2586,10 @@ static int ip_route_output_slow(struct net *net, struct rtable **rp,
>  			goto out;
>  
>  		/* RACE: Check return value of inet_select_addr instead. */
> -		if (__in_dev_get_rtnl(dev_out) == NULL) {
> +		if (!(dev_out->flags & IFF_UP) || __in_dev_get_rtnl(dev_out) == NULL) {
>  			dev_put(dev_out);
> -			goto out;	/* Wrong error code */
> +			err = -ENETUNREACH;
> +			goto out;
>  		}
>  
>  		if (ipv4_is_local_multicast(oldflp->fl4_dst) ||
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet Dec. 22, 2010, 9:55 a.m. UTC | #3 
Le mercredi 22 décembre 2010 à 09:32 +0100, Nicolas Dichtel a écrit :
> What is the status of this patch? The initial problem is still here in 2.6.37-rc5+
> 

I cannot reproduce the problem on net-next-2.6, are you sure we still
need a new patch ?

# ip tunnel add mode ipip remote 10.16.0.164 local 10.16.0.72 dev eth0
# ip link show dev tunl1
19: tunl1: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN 
    link/ipip 10.16.0.72 peer 10.16.0.164
# /root/vrrp tunl1
# ip tunnel del tunl1



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet Dec. 22, 2010, 10:07 a.m. UTC | #4 
Le mercredi 22 décembre 2010 à 10:55 +0100, Eric Dumazet a écrit :
> Le mercredi 22 décembre 2010 à 09:32 +0100, Nicolas Dichtel a écrit :
> > What is the status of this patch? The initial problem is still here in 2.6.37-rc5+
> > 
> 
> I cannot reproduce the problem on net-next-2.6, are you sure we still
> need a new patch ?
> 
> # ip tunnel add mode ipip remote 10.16.0.164 local 10.16.0.72 dev eth0
> # ip link show dev tunl1
> 19: tunl1: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN 
>     link/ipip 10.16.0.72 peer 10.16.0.164
> # /root/vrrp tunl1
> # ip tunnel del tunl1
> 
> 

(I thought commit 332dd96f7ac15e fixed the problem for you, as it did
for me)

net/dst: dst_dev_event() called after other notifiers



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Dichtel Dec. 22, 2010, 1:43 p.m. UTC | #5 
Yes, I saw this commit, but I still got the problem:

shelby:/home/root/src# uname -a
Linux shelby 2.6.37-rc5+ #10 SMP Wed Dec 22 05:02:53 EST 2010 i686 GNU/Linux
shelby:/home/root/src# ip tunnel add mode ipip remote 10.16.0.164 local 
10.16.0.72 dev eth0
shelby:/home/root/src# ./a.out tunl1
shelby:/home/root/src# ip tunnel del tunl1

Message from syslogd@shelby at Dec 22 10:12:08 ...
  kernel:[18459.828011] unregister_netdevice: waiting for tunl1 to become free. 
Usage count = 3

Message from syslogd@shelby at Dec 22 10:12:19 ...
  kernel:[18470.072017] unregister_netdevice: waiting for tunl1 to become free. 
Usage count = 3

Message from syslogd@shelby at Dec 22 10:12:29 ...
  kernel:[18480.316011] unregister_netdevice: waiting for tunl1 to become free. 
Usage count = 3

Message from syslogd@shelby at Dec 22 10:12:39 ...
  kernel:[18490.560010] unregister_netdevice: waiting for tunl1 to become free. 
Usage count = 3
shelby:/home/root/src#

I don't know if I will have time to investigate more before next year.

Regards,
Nicolas


Le 22.12.2010 11:07, Eric Dumazet a écrit :
> Le mercredi 22 décembre 2010 à 10:55 +0100, Eric Dumazet a écrit :
>> Le mercredi 22 décembre 2010 à 09:32 +0100, Nicolas Dichtel a écrit :
>>> What is the status of this patch? The initial problem is still here in 2.6.37-rc5+
>>>
>> I cannot reproduce the problem on net-next-2.6, are you sure we still
>> need a new patch ?
>>
>> # ip tunnel add mode ipip remote 10.16.0.164 local 10.16.0.72 dev eth0
>> # ip link show dev tunl1
>> 19: tunl1: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN 
>>     link/ipip 10.16.0.72 peer 10.16.0.164
>> # /root/vrrp tunl1
>> # ip tunnel del tunl1
>>
>>
> 
> (I thought commit 332dd96f7ac15e fixed the problem for you, as it did
> for me)
> 
> net/dst: dst_dev_event() called after other notifiers
> 
> 
>
diff mbox

Patch

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index ac6559c..1ee0b1a 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2586,9 +2586,10 @@  static int ip_route_output_slow(struct net *net, struct rtable **rp,
 			goto out;
 
 		/* RACE: Check return value of inet_select_addr instead. */
-		if (__in_dev_get_rtnl(dev_out) == NULL) {
+		if (!(dev_out->flags & IFF_UP) || __in_dev_get_rtnl(dev_out) == NULL) {
 			dev_put(dev_out);
-			goto out;	/* Wrong error code */
+			err = -ENETUNREACH;
+			goto out;
 		}
 
 		if (ipv4_is_local_multicast(oldflp->fl4_dst) ||