| Message ID | 1389029375-17698-1-git-send-email-thaller@redhat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Mon, Jan 06, 2014 at 06:29:35PM CET, thaller@redhat.com wrote: >When adding/modifying an IPv6 address, the userspace application needs >a way to suppress adding a prefix route. This is for example relevant >together with IFA_F_MANAGERTEMPADDR, where userspace creates autoconf >generated addresses, but depending on on-link, no route should for the >prefix should be added. > >This flag will not be set as ifa_flags of the address, it is only >considered as parameter while adding/modifying an address. > >Signed-off-by: Thomas Haller <thaller@redhat.com> >--- > Hi, how about this? > > The flag is only a parameter for the netlink command. > This might be unexpected, because when adding an address, > you won't see the flag in `ip -6 addr`. > Still, I think, it is better to do it this way, because having > an address with the NOPREFIXROUTE flag, does not mean, that > there is no route for this prefix. It only means, that at the > moment of setting the address, no route was added. > > The alternative would be, not to add a prefix route, when > IFA_F_MANAGERTEMPADDR is set. > > Thomas > > include/uapi/linux/if_addr.h | 1 + > net/ipv6/addrconf.c | 19 +++++++++++++------ > 2 files changed, 14 insertions(+), 6 deletions(-) > >diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h >index cfed10b..dea10a8 100644 >--- a/include/uapi/linux/if_addr.h >+++ b/include/uapi/linux/if_addr.h >@@ -49,6 +49,7 @@ enum { > #define IFA_F_TENTATIVE 0x40 > #define IFA_F_PERMANENT 0x80 > #define IFA_F_MANAGETEMPADDR 0x100 >+#define IFA_F_NOPREFIXROUTE 0x200 > > struct ifa_cacheinfo { > __u32 ifa_prefered; >diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c >index 6c16345..51bd757 100644 >--- a/net/ipv6/addrconf.c >+++ b/net/ipv6/addrconf.c >@@ -2429,12 +2429,16 @@ static int inet6_addr_add(struct net *net, int ifindex, > prefered_lft = timeout; > } > >- ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags, >+ ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, >+ ifa_flags & ~IFA_F_NOPREFIXROUTE, > valid_lft, prefered_lft); > > if (!IS_ERR(ifp)) { >- addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev, >- expires, flags); >+ if (ifa_flags & IFA_F_NOPREFIXROUTE == 0) { if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) { >+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev, >+ expires, flags); >+ } >+ > /* > * Note that section 3.1 of RFC 4429 indicates > * that the Optimistic flag should not be set for >@@ -3662,8 +3666,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags, > if (!(ifp->flags&IFA_F_TENTATIVE)) > ipv6_ifa_notify(0, ifp); > >- addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev, >- expires, flags); >+ if (ifa_flags & IFA_F_NOPREFIXROUTE == 0) { >+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev, >+ expires, flags); >+ } > > if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) { > if (was_managetempaddr && !(ifp->flags & IFA_F_MANAGETEMPADDR)) >@@ -3717,7 +3723,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh) > ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags; > > /* We ignore other flags so far. */ >- ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR; >+ ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR | >+ IFA_F_NOPREFIXROUTE; > > ifa = ipv6_get_ifaddr(net, pfx, dev, 1); > if (ifa == NULL) { >-- >1.8.4.2 > -- 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, Jan 06, 2014 at 06:38:37PM +0100, Jiri Pirko wrote: > Mon, Jan 06, 2014 at 06:29:35PM CET, thaller@redhat.com wrote: > >When adding/modifying an IPv6 address, the userspace application needs > >a way to suppress adding a prefix route. This is for example relevant > >together with IFA_F_MANAGERTEMPADDR, where userspace creates autoconf > >generated addresses, but depending on on-link, no route should for the > >prefix should be added. > > > >This flag will not be set as ifa_flags of the address, it is only > >considered as parameter while adding/modifying an address. > > > >Signed-off-by: Thomas Haller <thaller@redhat.com> > >--- > > Hi, how about this? > > > > The flag is only a parameter for the netlink command. > > This might be unexpected, because when adding an address, > > you won't see the flag in `ip -6 addr`. > > Still, I think, it is better to do it this way, because having > > an address with the NOPREFIXROUTE flag, does not mean, that > > there is no route for this prefix. It only means, that at the > > moment of setting the address, no route was added. > > > > The alternative would be, not to add a prefix route, when > > IFA_F_MANAGERTEMPADDR is set. Looks good, maybe you could apply Jiri's feedback. Otherwise we could skip trying to find a corresponding prefix route in ipv6_del_addr. You would need to add the IFA_F_NOPREFIXROUTE test just before the addrconf_get_prefix_route and ip6_del_rt. 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
On Mon, Jan 06, 2014 at 06:29:35PM +0100, Thomas Haller wrote: > @@ -3662,8 +3666,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags, > if (!(ifp->flags&IFA_F_TENTATIVE)) > ipv6_ifa_notify(0, ifp); > > - addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev, > - expires, flags); > + if (ifa_flags & IFA_F_NOPREFIXROUTE == 0) { > + addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev, > + expires, flags); > + } Actually, if we switch from IFA_F_NOPREFIXROUTE to !IFA_F_NOPREFIXROUTE we have to remove the prefix route, no? 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
On Tue, 2014-01-07 at 13:01 +0100, Hannes Frederic Sowa wrote: > On Mon, Jan 06, 2014 at 06:29:35PM +0100, Thomas Haller wrote: > > @@ -3662,8 +3666,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags, > > if (!(ifp->flags&IFA_F_TENTATIVE)) > > ipv6_ifa_notify(0, ifp); > > > > - addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev, > > - expires, flags); > > + if (ifa_flags & IFA_F_NOPREFIXROUTE == 0) { > > + addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev, > > + expires, flags); > > + } > > Actually, if we switch from IFA_F_NOPREFIXROUTE to !IFA_F_NOPREFIXROUTE we > have to remove the prefix route, no? > > Greetings, > > Hannes > hi Hannes, I am about to resent the patch, so that IFA_F_NOPREFIXROUTE is saved in the flags of the address. Later, when deleting such address, this is used to indicate ~not~ to delete any prefix route... just as you suggest in your earlier email (if I understood you right). About this suggestion now, I tend to "no". Yes, it could be sensible, on the other hand, if user space already controls the routes (as indicated by it's use of IFA_F_NOPREFIXROUTES), I would just leave it to the user to clean up the wrong prefix route. What do you think? Thomas
Hi Thomas! On Tue, Jan 07, 2014 at 01:14:34PM +0100, Thomas Haller wrote: > I am about to resent the patch, so that IFA_F_NOPREFIXROUTE is saved in > the flags of the address. Later, when deleting such address, this is > used to indicate ~not~ to delete any prefix route... just as you suggest > in your earlier email (if I understood you right). Ok, good. > About this suggestion now, I tend to "no". Yes, it could be sensible, on > the other hand, if user space already controls the routes (as indicated > by it's use of IFA_F_NOPREFIXROUTES), I would just leave it to the user > to clean up the wrong prefix route. > > What do you think? It is a matter of usability. As prefix routes are added to main routing table and are visible via simple ip -6 r l (and not ip -6 r l table something), I am fine with that, 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
Now, the IFA_F_NOPREFIXROUTE flag is saved in ifp->flags.
The second patch reworks the deletion of addresses/cleanup of prefix
routes and considers IFA_F_NOPREFIXROUTE.
Thomas Haller (2):
ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
IP6 routes
ipv6 addrconf: don't cleanup route prefix for IFA_F_NOPREFIXROUTE
include/uapi/linux/if_addr.h | 1 +
net/ipv6/addrconf.c | 206 ++++++++++++++++++++++++++-----------------
2 files changed, 125 insertions(+), 82 deletions(-)
On Tue, Jan 07, 2014 at 03:39:11PM +0100, Thomas Haller wrote: > Now, the IFA_F_NOPREFIXROUTE flag is saved in ifp->flags. > The second patch reworks the deletion of addresses/cleanup of prefix > routes and considers IFA_F_NOPREFIXROUTE. > > Thomas Haller (2): > ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of > IP6 routes > ipv6 addrconf: don't cleanup route prefix for IFA_F_NOPREFIXROUTE Maybe you could look into if a small patch implementing noprefixroute would be doable. That would help testing these changes a lot. ;) 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
On Tue, 2014-01-07 at 17:03 +0100, Hannes Frederic Sowa wrote: > On Tue, Jan 07, 2014 at 03:39:11PM +0100, Thomas Haller wrote: > > Now, the IFA_F_NOPREFIXROUTE flag is saved in ifp->flags. > > The second patch reworks the deletion of addresses/cleanup of prefix > > routes and considers IFA_F_NOPREFIXROUTE. > > > > Thomas Haller (2): > > ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of > > IP6 routes > > ipv6 addrconf: don't cleanup route prefix for IFA_F_NOPREFIXROUTE > > Maybe you could look into if a small patch implementing noprefixroute would be > doable. That would help testing these changes a lot. ;) > > Greetings, > > Hannes > Just for reference: I sent an email "[patch iproute2 v3 3/3] add support for IFA_F_NOPREFIXROUTE" with a patch for iproute2. Thomas
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h index cfed10b..dea10a8 100644 --- a/include/uapi/linux/if_addr.h +++ b/include/uapi/linux/if_addr.h @@ -49,6 +49,7 @@ enum { #define IFA_F_TENTATIVE 0x40 #define IFA_F_PERMANENT 0x80 #define IFA_F_MANAGETEMPADDR 0x100 +#define IFA_F_NOPREFIXROUTE 0x200 struct ifa_cacheinfo { __u32 ifa_prefered; diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 6c16345..51bd757 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -2429,12 +2429,16 @@ static int inet6_addr_add(struct net *net, int ifindex, prefered_lft = timeout; } - ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags, + ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, + ifa_flags & ~IFA_F_NOPREFIXROUTE, valid_lft, prefered_lft); if (!IS_ERR(ifp)) { - addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev, - expires, flags); + if (ifa_flags & IFA_F_NOPREFIXROUTE == 0) { + addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev, + expires, flags); + } + /* * Note that section 3.1 of RFC 4429 indicates * that the Optimistic flag should not be set for @@ -3662,8 +3666,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags, if (!(ifp->flags&IFA_F_TENTATIVE)) ipv6_ifa_notify(0, ifp); - addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev, - expires, flags); + if (ifa_flags & IFA_F_NOPREFIXROUTE == 0) { + addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev, + expires, flags); + } if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) { if (was_managetempaddr && !(ifp->flags & IFA_F_MANAGETEMPADDR)) @@ -3717,7 +3723,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh) ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags; /* We ignore other flags so far. */ - ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR; + ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR | + IFA_F_NOPREFIXROUTE; ifa = ipv6_get_ifaddr(net, pfx, dev, 1); if (ifa == NULL) {
When adding/modifying an IPv6 address, the userspace application needs a way to suppress adding a prefix route. This is for example relevant together with IFA_F_MANAGERTEMPADDR, where userspace creates autoconf generated addresses, but depending on on-link, no route should for the prefix should be added. This flag will not be set as ifa_flags of the address, it is only considered as parameter while adding/modifying an address. Signed-off-by: Thomas Haller <thaller@redhat.com> --- Hi, how about this? The flag is only a parameter for the netlink command. This might be unexpected, because when adding an address, you won't see the flag in `ip -6 addr`. Still, I think, it is better to do it this way, because having an address with the NOPREFIXROUTE flag, does not mean, that there is no route for this prefix. It only means, that at the moment of setting the address, no route was added. The alternative would be, not to add a prefix route, when IFA_F_MANAGERTEMPADDR is set. Thomas include/uapi/linux/if_addr.h | 1 + net/ipv6/addrconf.c | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-)