Message ID | 1399040150-11863-2-git-send-email-chris.j.arges@canonical.com |
---|---|
State | New |
Headers | show |
On Fri, May 02, 2014 at 09:15:50AM -0500, Chris J Arges wrote: > From: Stefan Bader <stefan.bader@canonical.com> > > When doing (for example) a connect to an unused port on localhost > in a new net namespace, it was observed that lo could not be > removed immediately because there were still references on it. > To release those references could take a very long time (up to > five minutes). > > This was tracked down to an element in the route cache which was > supposed to be dropped by a notify call with sending a > NETDEVICE_UNREGISTER_BATCH message, but as in_dev is already unset > then, the case statement never was executed. > > Move the handling of that notification up to be done regardless of > the state of in_dev. > > BugLink: http://bugs.launchpad.net/bugs/1021471 > > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > Signed-off-by: Chris J Arges <chris.j.arges@canonical.com> > --- > net/ipv4/fib_frontend.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c > index 92fc5f6..5085d48 100644 > --- a/net/ipv4/fib_frontend.c > +++ b/net/ipv4/fib_frontend.c > @@ -999,6 +999,14 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo > fib_disable_ip(dev, 2, -1); > return NOTIFY_DONE; > } > + if (event == NETDEV_UNREGISTER_BATCH) { > + /* The batch unregister is only called on the first > + * device in the list of devices being unregistered. > + * Therefore we should not pass dev_net(dev) in here. > + */ > + rt_cache_flush_batch(NULL); > + return NOTIFY_DONE; > + } > > if (!in_dev) > return NOTIFY_DONE; > @@ -1021,13 +1029,6 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo > case NETDEV_CHANGE: > rt_cache_flush(dev_net(dev), 0); > break; > - case NETDEV_UNREGISTER_BATCH: > - /* The batch unregister is only called on the first > - * device in the list of devices being unregistered. > - * Therefore we should not pass dev_net(dev) in here. > - */ > - rt_cache_flush_batch(NULL); > - break; > } > return NOTIFY_DONE; > } Looks to do what is claimed and has a reproducer to confirm. I assume this already on its way upstream. On that basis: Acked-by: Andy Whitcroft <apw@canonical.com> -apw
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c index 92fc5f6..5085d48 100644 --- a/net/ipv4/fib_frontend.c +++ b/net/ipv4/fib_frontend.c @@ -999,6 +999,14 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo fib_disable_ip(dev, 2, -1); return NOTIFY_DONE; } + if (event == NETDEV_UNREGISTER_BATCH) { + /* The batch unregister is only called on the first + * device in the list of devices being unregistered. + * Therefore we should not pass dev_net(dev) in here. + */ + rt_cache_flush_batch(NULL); + return NOTIFY_DONE; + } if (!in_dev) return NOTIFY_DONE; @@ -1021,13 +1029,6 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo case NETDEV_CHANGE: rt_cache_flush(dev_net(dev), 0); break; - case NETDEV_UNREGISTER_BATCH: - /* The batch unregister is only called on the first - * device in the list of devices being unregistered. - * Therefore we should not pass dev_net(dev) in here. - */ - rt_cache_flush_batch(NULL); - break; } return NOTIFY_DONE; }