| Message ID | 1271797043.7895.320.camel@edumazet-laptop |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 20 Apr 2010 22:57:23 +0200 Eric Dumazet <eric.dumazet@gmail.com> wrote: > Le mardi 20 avril 2010 à 22:49 +0200, Jiri Bohac a écrit : > > On Tue, Apr 20, 2010 at 07:57:27PM +0200, Eric Dumazet wrote: > > > Le mardi 20 avril 2010 à 19:44 +0200, Jiri Bohac a écrit : > > > > --- a/net/ipv6/addrconf.c 2010-04-17 00:12:32.000000000 +0200 > > > > +++ b/net/ipv6/addrconf.c 2010-04-20 19:07:35.000000000 +0200 > > > > @@ -3974,8 +3974,7 @@ static void __ipv6_ifa_notify(int event, > > > > addrconf_leave_anycast(ifp); > > > > addrconf_leave_solict(ifp->idev, &ifp->addr); > > > > dst_hold(&ifp->rt->u.dst); > > > > - if (ip6_del_rt(ifp->rt)) > > > > - dst_free(&ifp->rt->u.dst); > > > > + ip6_del_rt(ifp->rt); > > > > break; > > > > } > > > > } > > > > > > > > > > > > > I dont understand the problem Jiri. > > > > > > We just did dst_hold(&ifp->rt->u.dst), so if ip6_del_rt() fails we must > > > dst_free(), or we leak a refcount. > > > > Well, no ... dst_free() does not decrement the refcount. > > The "opposite" of dst_hold() is dst_release(). And it does not > > automatically call dst_free() when the refcount drops to 0. > > dst_free() needs to be called explicitly and it apparently > > expects the caller to ensure that two dst_free()s won't be called > > simultaneously. But my bonding example shows this is not the > > case. > > > > > > Ah yes you're right > > Maybe ask Stephen ? > > commit 93fa159abe50d3c55c7f83622d3f5c09b6e06f4b > Author: stephen hemminger <shemminger@vyatta.com> > Date: Mon Apr 12 05:41:31 2010 +0000 > > IPv6: keep route for tentative address > > Recent changes preserve IPv6 address when link goes down (good). > But would cause address to point to dead dst entry (bad). > The simplest fix is to just not delete route if address is > being held for later use. > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c > index 1b00bfe..a9913d2 100644 > --- a/net/ipv6/addrconf.c > +++ b/net/ipv6/addrconf.c > @@ -4047,7 +4047,8 @@ static void __ipv6_ifa_notify(int event, struct > inet6_ifaddr *ifp) > addrconf_leave_anycast(ifp); > addrconf_leave_solict(ifp->idev, &ifp->addr); > dst_hold(&ifp->rt->u.dst); > - if (ip6_del_rt(ifp->rt)) > + > + if (ifp->dead && ip6_del_rt(ifp->rt)) > dst_free(&ifp->rt->u.dst); > break; Is this problem new to net-next where we hold onto addresses, or was the issue there before? -- 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
On Tue, Apr 20, 2010 at 02:16:55PM -0700, Stephen Hemminger wrote: > Is this problem new to net-next where we hold onto addresses, or was > the issue there before? Before. I am seeing it on 2.6.32.
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 1b00bfe..a9913d2 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -4047,7 +4047,8 @@ static void __ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp) addrconf_leave_anycast(ifp); addrconf_leave_solict(ifp->idev, &ifp->addr); dst_hold(&ifp->rt->u.dst); - if (ip6_del_rt(ifp->rt)) + + if (ifp->dead && ip6_del_rt(ifp->rt)) dst_free(&ifp->rt->u.dst); break;