Re: [PATCH] ipv4: remove all rt cache entries on UNREGISTER event

[Eric Dumazet <eric.dumazet@gmail.com>]

Le mardi 28 septembre 2010 à 18:45 +0200, Nicolas Dichtel a écrit :
> Eric Dumazet wrote:
> > Le mardi 28 septembre 2010 à 17:24 +0200, Nicolas Dichtel a écrit :
> >> Hi,
> >>
> >> I face a problem when I try to remove an interface, 
> >> netdev_wait_allrefs() complains about refcount.
> >>
> >> Here is a trivial scenario to reproduce the problem:
> >> # ip tunnel add mode ipip remote 10.16.0.164 local 10.16.0.72 dev eth0
> >> # ./a.out tunl1
> >> # ip tunnel del tunl1
> >>
> >> Note: a.out binary create an IPv4 raw socket, attach it to tunl1 
> >> (SO_BINDTODEVICE), set it as multicast (IP_MULTICAST_LOOP), set the 
> >> multicast interface to tunl1 (IP_MULTICAST_IF), build the IP header 
> >> (IP_HDRINCL) and then send a single packet (192.168.6.1 -> 224.0.0.18).
> >>
> >> Note2: when a.out is executed, tunl1 has no ip address and is down.
> >>
> > 
> > CC Octavian Purdila, the patch author.
> > 
> > I am just wondering why this route is created in the first place.
> At first, I asked myself the same question, but it seems that this is 
> allowed to send a packet through this kind of socket, even if interface 
> is down. Packet will be destroyed by the noop qdisk.
> But I agree that it is strange to perform route lookup and everything to 
>    destroy the packet at the end ...
> Maybe raw_sendmsg() can delete it directly ;-) ... or maybe 
> ip_route_output_flow().
> 
> Any suggestions welcome.
> 

Hmm...

One way to track this kind of problem would be to add a WARN_ON() in
dev_hold()

-> Check that when a reference on dev is taken, we are in a known state.

Something like this ?

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 83de0eb..54bef78 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1773,6 +1774,7 @@ static inline void dev_put(struct net_device *dev)
  */
 static inline void dev_hold(struct net_device *dev)
 {
+	WARN_ON(dev->reg_state != NETREG_REGISTERED);
 	atomic_inc(&dev->refcnt);
 }