| Message ID | 1392567954-25752-1-git-send-email-cyrus@openwrt.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sun, Feb 16, 2014 at 05:25:54PM +0100, Steven Barth wrote: > - if (ifp->flags & IFA_F_PERMANENT && !(ifp->flags & IFA_F_NOPREFIXROUTE)) > + if (!(ifp->flags & IFA_F_NOPREFIXROUTE)) > action = check_cleanup_prefix_route(ifp, &expires); This could too easily clean up valid on-link information if the address just happens to be in the same subnet. Would (ifp-flags & (IFA_F_PERMANENET|IFA_F_TEMPORARY) && !(...)) solve the problem, too? Greetings, Hannes -- 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
Hannes Frederic Sowa wrote: > This could too easily clean up valid on-link information if the address > just happens to be in the same subnet. Would (ifp-flags & > (IFA_F_PERMANENET|IFA_F_TEMPORARY) && !(...)) solve the problem, too? No, that doesn't do the trick unfortunately. Seems that the (non-permanent) address created by "ip" don't have IFA_F_TEMPORARY set. Any other good ideas? Cheers, Steven -- 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 Mon, Feb 17, 2014 at 07:41:42AM +0100, Steven Barth wrote: > Hannes Frederic Sowa wrote: > >This could too easily clean up valid on-link information if the address > >just happens to be in the same subnet. Would (ifp-flags & > >(IFA_F_PERMANENET|IFA_F_TEMPORARY) && !(...)) solve the problem, too? > > No, that doesn't do the trick unfortunately. Seems that the > (non-permanent) address created by "ip" don't have IFA_F_TEMPORARY set. > Any other good ideas? We are talking about managed temp addresses, no? What ip command do you use? Bye, Hannes -- 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
Ah sorry, maybe I was a bit unclear about this. With temporary addresses I meant addresses created like this: ip address add 2001:db8::1/64 dev eth0 valid_lft 1234 preferred_lft 1001 or the equivalent netlink-calls and with permanent addresses i was referring to addresses created similarly but with an infinite lifetime (e.g. through ip without giving valid_lft or preferred_lft arguments). Hope that makes it clear. Cheers, Steven On 17.02.2014 14:05, Hannes Frederic Sowa wrote: > On Mon, Feb 17, 2014 at 07:41:42AM +0100, Steven Barth wrote: >> Hannes Frederic Sowa wrote: >>> This could too easily clean up valid on-link information if the address >>> just happens to be in the same subnet. Would (ifp-flags & >>> (IFA_F_PERMANENET|IFA_F_TEMPORARY) && !(...)) solve the problem, too? >> No, that doesn't do the trick unfortunately. Seems that the >> (non-permanent) address created by "ip" don't have IFA_F_TEMPORARY set. >> Any other good ideas? > We are talking about managed temp addresses, no? What ip command do you use? > > Bye, > > Hannes > > -- > 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 -- 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 Mon, Feb 17, 2014 at 02:13:27PM +0100, Steven Barth wrote: > Ah sorry, maybe I was a bit unclear about this. With temporary addresses > I meant addresses created like this: > > ip address add 2001:db8::1/64 dev eth0 valid_lft 1234 preferred_lft 1001 > > or the equivalent netlink-calls and with permanent addresses i was > referring to addresses created similarly but with an infinite lifetime > (e.g. through ip without giving valid_lft or preferred_lft arguments). > > Hope that makes it clear. Yes, makes sense, thanks! I fear there is currently no combination of ifp->flags which can test for that. Your first patch seems to be too dangerous in removing neighbour discovery brought in on-link state, I fear. It may even be a problem for permanent ones, but that's how it is done, currently. Best option would be to fully decouple prefix routes from address handling, but that would break current userspace. Current iproute has support for no-prefix-route address addition (see iproute2 commit 58c69b226fb3a ("add support for IFA_F_NOPREFIXROUTE")), so you could manage those address and route combination directly from user space. So if we want to clean up those routes we either must set IFA_F_PERMANENT to the ifp flags of the address in inet6_addr_add and really make sure nothing breaks because of this. Or introduce a new flag, like IFA_F_USERINSTALLED e.g. Current ifp flags handling is really complex and we had some bad bugs in recent changes, so I guess, it is not a trivial task (I reviewed the last patches changing code in that for hours and they still had bugs :/), so I am unsure what is the right approach. Both ways (adding PERMANENT) and adding a new IFP_P_USERINSTALLED would need to be considered and reviewed. Thanks, Hannes -- 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 17.02.2014 15:08, Hannes Frederic Sowa wrote: > > I fear there is currently no combination of ifp->flags which can test for > that. Your first patch seems to be too dangerous in removing neighbour > discovery brought in on-link state, I fear. It may even be a problem for > permanent ones, but that's how it is done, currently. Yeah the kernel neighbor discovery support is a bit tricky in itself and I found myself working around it more often then actually using it (at least when doing router-stuff). > > Best option would be to fully decouple prefix routes from address handling, > but that would break current userspace. > > Current iproute has support for no-prefix-route address addition (see > iproute2 commit 58c69b226fb3a ("add support for IFA_F_NOPREFIXROUTE")), so you > could manage those address and route combination directly from user space. OK, I will use a workaround for now that removes and readds addresses when they should be deprecated instead of just converting permanents to non-permanents and maybe later use that new NOPREFIXROUTE feature which should come in handy. > > So if we want to clean up those routes we either must set IFA_F_PERMANENT to > the ifp flags of the address in inet6_addr_add and really make sure nothing > breaks because of this. Or introduce a new flag, like IFA_F_USERINSTALLED e.g. OK, I will try to think about this when I have some more time to spare on this. Thanks for your help so far. Steven -- 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
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index ad23569..55c35d5 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -1013,7 +1013,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp) __in6_ifa_put(ifp); } - if (ifp->flags & IFA_F_PERMANENT && !(ifp->flags & IFA_F_NOPREFIXROUTE)) + if (!(ifp->flags & IFA_F_NOPREFIXROUTE)) action = check_cleanup_prefix_route(ifp, &expires); list_del_init(&ifp->if_list);
When temporary addresses are created in userspace an accompanying on-link prefix-route is created alongside. However when said address is afterwards removed in userspace the respective prefix route remains until it expires. Similarly when userspace turns a permanent address into a temporary one the prefix route remains and is not removed when the address expires and is thus dangling until removed manually. This behavior is inconsistent with the addition and removal of permanent addresses and can potentially cause lots of dangling routes on downstream interfaces on an IPv6-router with changing prefixes. This patch runs the on-link prefix removal check also when a temporary address is removed and thus unifies the prefix route handling. Signed-off-by: Steven Barth <cyrus@openwrt.org> --- net/ipv6/addrconf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)