[PATCH v3 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[PATCH v3 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Thomas Haller]

New version, obsoletes the two previous attempts.

v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
v2 -> v3: reword commit messages, code comments and some refactoring.

Thomas Haller (2):
  ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
    IP6 routes
  ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE

 include/uapi/linux/if_addr.h |   1 +
 net/ipv6/addrconf.c          | 204 ++++++++++++++++++++++++++-----------------
 2 files changed, 123 insertions(+), 82 deletions(-)

-- 
1.8.4.2

[PATCH v3 1/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Thomas Haller]

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 for the
prefix should be added.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 include/uapi/linux/if_addr.h |  1 +
 net/ipv6/addrconf.c          | 18 ++++++++++++------
 2 files changed, 13 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 31f75ea..1bc575f 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2433,8 +2433,11 @@ static int inet6_addr_add(struct net *net, int ifindex,
 			    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)) {
+			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
@@ -3658,7 +3661,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	spin_lock_bh(&ifp->lock);
 	was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
 	ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
-			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR);
+			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE);
 	ifp->flags |= ifa_flags;
 	ifp->tstamp = jiffies;
 	ifp->valid_lft = valid_lft;
@@ -3668,8 +3671,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)) {
+		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))
@@ -3723,7 +3728,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

[PATCH v3 2/2] ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE

[Thomas Haller]

Refactor the deletion/update of prefix routes when removing an
address. Now also consider IFA_F_NOPREFIXROUTE and if there is an address
present with this flag, to not cleanup the route. Instead, assume
that userspace is taking care of this route.

Also perform the same cleanup, when userspace changes an existing address
to add NOPREFIXROUTE (to an address that didn't have this flag). This is
done because when the address was added, a prefix route was created for it.
Since the user now wants to handle this route by himself, we cleanup this
route.

This cleanup of the route is not totally robust. There is no guarantee,
that the route we are about to delete was really the one added by the
kernel. This behavior does not change by the patch, and in practice it
should work just fine.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 net/ipv6/addrconf.c | 186 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 110 insertions(+), 76 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 1bc575f..b3fcf9f 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -900,15 +900,104 @@ out:
 	goto out2;
 }
 
+/*
+ * Check, whether the prefix for ifp would still need a prefix route
+ * after deleting ifp. The function returns:
+ *    -1 route valid, only update lifetimes (outputs expires).
+ *     0 route invalid, delete/purge
+ *     1 route valid, don't update lifetimes.
+ *
+ * 1) we don't purge prefix if address was not permanent.
+ *    prefix is managed by its own lifetime.
+ * 2) we also don't purge, if the address was IFA_F_NOPREFIXROUTE.
+ * 3) if there're no addresses, delete prefix.
+ * 4) if there're still other permanent address(es),
+ *    corresponding prefix is still permanent.
+ * 5) if there are still other addresses with IFA_F_NOPREFIXROUTE,
+ *    don't purge the prefix, assume user space is managing it.
+ * 6) otherwise, update prefix lifetime to the
+ *    longest valid lifetime among the corresponding
+ *    addresses on the device.
+ *    Note: subsequent RA will update lifetime.
+ **/
+static int
+check_cleanup_prefix_routes(struct inet6_ifaddr *ifp, u32 ifa_flags, unsigned long *expires)
+{
+	struct inet6_ifaddr *ifa;
+	struct inet6_dev *idev = ifp->idev;
+	unsigned long lifetime;
+	int onlink = 0;
+
+	*expires = jiffies;
+
+
+	if (!(ifa_flags & IFA_F_PERMANENT))
+		return 1;
+	if (ifa_flags & IFA_F_NOPREFIXROUTE)
+		return 1;
+
+	list_for_each_entry(ifa, &idev->addr_list, if_list) {
+		if (ifa == ifp)
+			continue;
+		if (!ipv6_prefix_equal(&ifa->addr, &ifp->addr,
+				       ifp->prefix_len))
+			continue;
+		if (ifa->flags & IFA_F_PERMANENT)
+			return 1;
+		if (ifa->flags & IFA_F_NOPREFIXROUTE)
+			return 1;
+
+		onlink = -1;
+
+		spin_lock(&ifa->lock);
+
+		lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
+		/*
+		 * Note: Because this address is
+		 * not permanent, lifetime <
+		 * LONG_MAX / HZ here.
+		 */
+		if (time_before(*expires, ifa->tstamp + lifetime * HZ))
+			*expires = ifa->tstamp + lifetime * HZ;
+		spin_unlock(&ifa->lock);
+	}
+
+	return onlink;
+}
+
+static void
+cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long expires, int onlink)
+{
+	struct rt6_info *rt;
+
+	if (onlink >= 1)
+		return;
+
+	rt = addrconf_get_prefix_route(&ifp->addr,
+				       ifp->prefix_len,
+				       ifp->idev->dev,
+				       0, RTF_GATEWAY | RTF_DEFAULT);
+	if (!rt)
+		return;
+
+	if (onlink == 0) {
+		ip6_del_rt(rt);
+		return;
+	}
+
+	if (!(rt->rt6i_flags & RTF_EXPIRES))
+		rt6_set_expires(rt, expires);
+	ip6_rt_put(rt);
+}
+
+
 /* This function wants to get referenced ifp and releases it before return */
 
 static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 {
-	struct inet6_ifaddr *ifa, *ifn;
-	struct inet6_dev *idev = ifp->idev;
 	int state;
-	int deleted = 0, onlink = 0;
-	unsigned long expires = jiffies;
+	int onlink;
+	unsigned long expires;
 
 	spin_lock_bh(&ifp->state_lock);
 	state = ifp->state;
@@ -922,7 +1011,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 	hlist_del_init_rcu(&ifp->addr_lst);
 	spin_unlock_bh(&addrconf_hash_lock);
 
-	write_lock_bh(&idev->lock);
+	write_lock_bh(&ifp->idev->lock);
 
 	if (ifp->flags&IFA_F_TEMPORARY) {
 		list_del(&ifp->tmp_list);
@@ -933,45 +1022,11 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 		__in6_ifa_put(ifp);
 	}
 
-	list_for_each_entry_safe(ifa, ifn, &idev->addr_list, if_list) {
-		if (ifa == ifp) {
-			list_del_init(&ifp->if_list);
-			__in6_ifa_put(ifp);
+	onlink = check_cleanup_prefix_routes(ifp, ifp->flags, &expires);
+	list_del_init(&ifp->if_list);
+	__in6_ifa_put(ifp);
 
-			if (!(ifp->flags & IFA_F_PERMANENT) || onlink > 0)
-				break;
-			deleted = 1;
-			continue;
-		} else if (ifp->flags & IFA_F_PERMANENT) {
-			if (ipv6_prefix_equal(&ifa->addr, &ifp->addr,
-					      ifp->prefix_len)) {
-				if (ifa->flags & IFA_F_PERMANENT) {
-					onlink = 1;
-					if (deleted)
-						break;
-				} else {
-					unsigned long lifetime;
-
-					if (!onlink)
-						onlink = -1;
-
-					spin_lock(&ifa->lock);
-
-					lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
-					/*
-					 * Note: Because this address is
-					 * not permanent, lifetime <
-					 * LONG_MAX / HZ here.
-					 */
-					if (time_before(expires,
-							ifa->tstamp + lifetime * HZ))
-						expires = ifa->tstamp + lifetime * HZ;
-					spin_unlock(&ifa->lock);
-				}
-			}
-		}
-	}
-	write_unlock_bh(&idev->lock);
+	write_unlock_bh(&ifp->idev->lock);
 
 	addrconf_del_dad_timer(ifp);
 
@@ -979,39 +1034,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 
 	inet6addr_notifier_call_chain(NETDEV_DOWN, ifp);
 
-	/*
-	 * Purge or update corresponding prefix
-	 *
-	 * 1) we don't purge prefix here if address was not permanent.
-	 *    prefix is managed by its own lifetime.
-	 * 2) if there're no addresses, delete prefix.
-	 * 3) if there're still other permanent address(es),
-	 *    corresponding prefix is still permanent.
-	 * 4) otherwise, update prefix lifetime to the
-	 *    longest valid lifetime among the corresponding
-	 *    addresses on the device.
-	 *    Note: subsequent RA will update lifetime.
-	 *
-	 * --yoshfuji
-	 */
-	if ((ifp->flags & IFA_F_PERMANENT) && onlink < 1) {
-		struct rt6_info *rt;
-
-		rt = addrconf_get_prefix_route(&ifp->addr,
-					       ifp->prefix_len,
-					       ifp->idev->dev,
-					       0, RTF_GATEWAY | RTF_DEFAULT);
-
-		if (rt) {
-			if (onlink == 0) {
-				ip6_del_rt(rt);
-				rt = NULL;
-			} else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-				rt6_set_expires(rt, expires);
-			}
-		}
-		ip6_rt_put(rt);
-	}
+	cleanup_prefix_route(ifp, expires, onlink);
 
 	/* clean up prefsrc entries */
 	rt6_remove_prefsrc(ifp);
@@ -3632,6 +3655,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	clock_t expires;
 	unsigned long timeout;
 	bool was_managetempaddr;
+	bool was_noprefixroute;
 
 	if (!valid_lft || (prefered_lft > valid_lft))
 		return -EINVAL;
@@ -3660,6 +3684,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 
 	spin_lock_bh(&ifp->lock);
 	was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
+	was_noprefixroute = ifp->flags & IFA_F_NOPREFIXROUTE;
 	ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
 			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE);
 	ifp->flags |= ifa_flags;
@@ -3674,6 +3699,15 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
 		addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
 				      expires, flags);
+	} else if (was_noprefixroute) {
+		int onlink;
+		unsigned long rt_expires;
+
+		write_lock_bh(&ifp->idev->lock);
+		onlink = check_cleanup_prefix_routes(ifp, ifp->flags & ~IFA_F_NOPREFIXROUTE, &rt_expires);
+		write_unlock_bh(&ifp->idev->lock);
+
+		cleanup_prefix_route(ifp, rt_expires, onlink);
 	}
 
 	if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
-- 
1.8.4.2

Re: [PATCH v3 2/2] ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE

[Hannes Frederic Sowa]

On Wed, Jan 08, 2014 at 01:41:28AM +0100, Thomas Haller wrote:
> Refactor the deletion/update of prefix routes when removing an
> address. Now also consider IFA_F_NOPREFIXROUTE and if there is an address
> present with this flag, to not cleanup the route. Instead, assume
> that userspace is taking care of this route.
> 
> Also perform the same cleanup, when userspace changes an existing address
> to add NOPREFIXROUTE (to an address that didn't have this flag). This is
> done because when the address was added, a prefix route was created for it.
> Since the user now wants to handle this route by himself, we cleanup this
> route.
> 
> This cleanup of the route is not totally robust. There is no guarantee,
> that the route we are about to delete was really the one added by the
> kernel. This behavior does not change by the patch, and in practice it
> should work just fine.
> 
> Signed-off-by: Thomas Haller <thaller@redhat.com>
> ---
>  net/ipv6/addrconf.c | 186 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 110 insertions(+), 76 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 1bc575f..b3fcf9f 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -900,15 +900,104 @@ out:
>  	goto out2;
>  }
>  
> +/*
> + * Check, whether the prefix for ifp would still need a prefix route
> + * after deleting ifp. The function returns:
> + *    -1 route valid, only update lifetimes (outputs expires).
> + *     0 route invalid, delete/purge
> + *     1 route valid, don't update lifetimes.

IMHO an enum would be nice for those.

> + *
> + * 1) we don't purge prefix if address was not permanent.
> + *    prefix is managed by its own lifetime.
> + * 2) we also don't purge, if the address was IFA_F_NOPREFIXROUTE.
> + * 3) if there're no addresses, delete prefix.
> + * 4) if there're still other permanent address(es),
> + *    corresponding prefix is still permanent.
> + * 5) if there are still other addresses with IFA_F_NOPREFIXROUTE,
> + *    don't purge the prefix, assume user space is managing it.
> + * 6) otherwise, update prefix lifetime to the
> + *    longest valid lifetime among the corresponding
> + *    addresses on the device.
> + *    Note: subsequent RA will update lifetime.
> + **/
> +static int
> +check_cleanup_prefix_routes(struct inet6_ifaddr *ifp, u32 ifa_flags, unsigned long *expires)

This line is over clearly over 80 characters.

> +{
> +	struct inet6_ifaddr *ifa;
> +	struct inet6_dev *idev = ifp->idev;
> +	unsigned long lifetime;
> +	int onlink = 0;
> +
> +	*expires = jiffies;
> +
> +
> +	if (!(ifa_flags & IFA_F_PERMANENT))
> +		return 1;
> +	if (ifa_flags & IFA_F_NOPREFIXROUTE)
> +		return 1;

Maybe if we factor out these tests to the caller we can give this function a
better name and don't need to pass ifa_flags?

For me it is easier to folow code like

	if ((foo & blah) || !(foo & blah2))
		do_something();

as when we return special value and pass it into another function.

> +
> +	list_for_each_entry(ifa, &idev->addr_list, if_list) {
> +		if (ifa == ifp)
> +			continue;
> +		if (!ipv6_prefix_equal(&ifa->addr, &ifp->addr,
> +				       ifp->prefix_len))
> +			continue;
> +		if (ifa->flags & IFA_F_PERMANENT)
> +			return 1;
> +		if (ifa->flags & IFA_F_NOPREFIXROUTE)
> +			return 1;
> +
> +		onlink = -1;
> +
> +		spin_lock(&ifa->lock);
> +
> +		lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
> +		/*
> +		 * Note: Because this address is
> +		 * not permanent, lifetime <
> +		 * LONG_MAX / HZ here.
> +		 */
> +		if (time_before(*expires, ifa->tstamp + lifetime * HZ))
> +			*expires = ifa->tstamp + lifetime * HZ;
> +		spin_unlock(&ifa->lock);
> +	}
> +
> +	return onlink;
> +}
> +
> +static void
> +cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long expires, int onlink)
> +{
> +	struct rt6_info *rt;
> +
> +	if (onlink >= 1)
> +		return;

Same here.

> +
> +	rt = addrconf_get_prefix_route(&ifp->addr,
> +				       ifp->prefix_len,
> +				       ifp->idev->dev,
> +				       0, RTF_GATEWAY | RTF_DEFAULT);
> +	if (!rt)
> +		return;
> +
> +	if (onlink == 0) {
> +		ip6_del_rt(rt);
> +		return;
> +	}

Maybe the onlink == 0 could be a bool then with a proper name (or an enum).

> +
> +	if (!(rt->rt6i_flags & RTF_EXPIRES))
> +		rt6_set_expires(rt, expires);
> +	ip6_rt_put(rt);
> +}
> +
> +
>  /* This function wants to get referenced ifp and releases it before return */
>  
>  static void ipv6_del_addr(struct inet6_ifaddr *ifp)
>  {
> -	struct inet6_ifaddr *ifa, *ifn;
> -	struct inet6_dev *idev = ifp->idev;
>  	int state;
> -	int deleted = 0, onlink = 0;
> -	unsigned long expires = jiffies;
> +	int onlink;
> +	unsigned long expires;
>  
>  	spin_lock_bh(&ifp->state_lock);
>  	state = ifp->state;
> @@ -922,7 +1011,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
>  	hlist_del_init_rcu(&ifp->addr_lst);
>  	spin_unlock_bh(&addrconf_hash_lock);
>  
> -	write_lock_bh(&idev->lock);
> +	write_lock_bh(&ifp->idev->lock);
>  
>  	if (ifp->flags&IFA_F_TEMPORARY) {
>  		list_del(&ifp->tmp_list);
> @@ -933,45 +1022,11 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
>  		__in6_ifa_put(ifp);
>  	}
>  
> -	list_for_each_entry_safe(ifa, ifn, &idev->addr_list, if_list) {
> -		if (ifa == ifp) {
> -			list_del_init(&ifp->if_list);
> -			__in6_ifa_put(ifp);
> +	onlink = check_cleanup_prefix_routes(ifp, ifp->flags, &expires);
> +	list_del_init(&ifp->if_list);
> +	__in6_ifa_put(ifp);
>  
> -			if (!(ifp->flags & IFA_F_PERMANENT) || onlink > 0)
> -				break;
> -			deleted = 1;
> -			continue;
> -		} else if (ifp->flags & IFA_F_PERMANENT) {
> -			if (ipv6_prefix_equal(&ifa->addr, &ifp->addr,
> -					      ifp->prefix_len)) {
> -				if (ifa->flags & IFA_F_PERMANENT) {
> -					onlink = 1;
> -					if (deleted)
> -						break;
> -				} else {
> -					unsigned long lifetime;
> -
> -					if (!onlink)
> -						onlink = -1;
> -
> -					spin_lock(&ifa->lock);
> -
> -					lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
> -					/*
> -					 * Note: Because this address is
> -					 * not permanent, lifetime <
> -					 * LONG_MAX / HZ here.
> -					 */
> -					if (time_before(expires,
> -							ifa->tstamp + lifetime * HZ))
> -						expires = ifa->tstamp + lifetime * HZ;
> -					spin_unlock(&ifa->lock);
> -				}
> -			}
> -		}
> -	}
> -	write_unlock_bh(&idev->lock);
> +	write_unlock_bh(&ifp->idev->lock);
>  
>  	addrconf_del_dad_timer(ifp);
>  
> @@ -979,39 +1034,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
>  
>  	inet6addr_notifier_call_chain(NETDEV_DOWN, ifp);
>  
> -	/*
> -	 * Purge or update corresponding prefix
> -	 *
> -	 * 1) we don't purge prefix here if address was not permanent.
> -	 *    prefix is managed by its own lifetime.
> -	 * 2) if there're no addresses, delete prefix.
> -	 * 3) if there're still other permanent address(es),
> -	 *    corresponding prefix is still permanent.
> -	 * 4) otherwise, update prefix lifetime to the
> -	 *    longest valid lifetime among the corresponding
> -	 *    addresses on the device.
> -	 *    Note: subsequent RA will update lifetime.
> -	 *
> -	 * --yoshfuji
> -	 */
> -	if ((ifp->flags & IFA_F_PERMANENT) && onlink < 1) {
> -		struct rt6_info *rt;
> -
> -		rt = addrconf_get_prefix_route(&ifp->addr,
> -					       ifp->prefix_len,
> -					       ifp->idev->dev,
> -					       0, RTF_GATEWAY | RTF_DEFAULT);
> -
> -		if (rt) {
> -			if (onlink == 0) {
> -				ip6_del_rt(rt);
> -				rt = NULL;
> -			} else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
> -				rt6_set_expires(rt, expires);
> -			}
> -		}
> -		ip6_rt_put(rt);
> -	}
> +	cleanup_prefix_route(ifp, expires, onlink);
>  
>  	/* clean up prefsrc entries */
>  	rt6_remove_prefsrc(ifp);
> @@ -3632,6 +3655,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
>  	clock_t expires;
>  	unsigned long timeout;
>  	bool was_managetempaddr;
> +	bool was_noprefixroute;
>  
>  	if (!valid_lft || (prefered_lft > valid_lft))
>  		return -EINVAL;
> @@ -3660,6 +3684,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
>  
>  	spin_lock_bh(&ifp->lock);
>  	was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
> +	was_noprefixroute = ifp->flags & IFA_F_NOPREFIXROUTE;
>  	ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
>  			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE);
>  	ifp->flags |= ifa_flags;
> @@ -3674,6 +3699,15 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
>  	if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
>  		addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
>  				      expires, flags);
> +	} else if (was_noprefixroute) {
> +		int onlink;
> +		unsigned long rt_expires;
> +
> +		write_lock_bh(&ifp->idev->lock);
> +		onlink = check_cleanup_prefix_routes(ifp, ifp->flags & ~IFA_F_NOPREFIXROUTE, &rt_expires);
> +		write_unlock_bh(&ifp->idev->lock);
> +
> +		cleanup_prefix_route(ifp, rt_expires, onlink);
>  	}
>  
>  	if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {

I somehow find this patch a bit hard to follow. Having the tests more
closely at the actions would make it much easier, IMHO.

Haven't seen any logic problems though.

Thanks,

  Hannes

[PATCH v4 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Thomas Haller]

v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
v2 -> v3: reword commit messages, code comments and some refactoring.
v3 -> v4: refactor, rename variables, add enum

Thomas Haller (2):
  ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
    IP6 routes
  ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE

 include/uapi/linux/if_addr.h |   1 +
 net/ipv6/addrconf.c          | 203 ++++++++++++++++++++++++++-----------------
 2 files changed, 123 insertions(+), 81 deletions(-)

-- 
1.8.4.2

[PATCH v4 1/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Thomas Haller]

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 for the
prefix should be added.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 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 31f75ea..7f0f011 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2433,8 +2433,11 @@ static int inet6_addr_add(struct net *net, int ifindex,
 			    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)) {
+			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
@@ -3658,7 +3661,8 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	spin_lock_bh(&ifp->lock);
 	was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
 	ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
-			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR);
+			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
+			IFA_F_NOPREFIXROUTE);
 	ifp->flags |= ifa_flags;
 	ifp->tstamp = jiffies;
 	ifp->valid_lft = valid_lft;
@@ -3668,8 +3672,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)) {
+		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))
@@ -3723,7 +3729,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

Re: [PATCH v4 1/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Hannes Frederic Sowa]

On Thu, Jan 09, 2014 at 01:30:03AM +0100, Thomas Haller 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 for the
> prefix should be added.
> 
> Signed-off-by: Thomas Haller <thaller@redhat.com>

Looks good!

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

[PATCH v4 2/2] ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE

[Thomas Haller]

Refactor the deletion/update of prefix routes when removing an
address. Now also consider IFA_F_NOPREFIXROUTE and if there is an address
present with this flag, to not cleanup the route. Instead, assume
that userspace is taking care of this route.

Also perform the same cleanup, when userspace changes an existing address
to add NOPREFIXROUTE (to an address that didn't have this flag). This is
done because when the address was added, a prefix route was created for it.
Since the user now wants to handle this route by himself, we cleanup this
route.

This cleanup of the route is not totally robust. There is no guarantee,
that the route we are about to delete was really the one added by the
kernel. This behavior does not change by the patch, and in practice it
should work just fine.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 net/ipv6/addrconf.c | 184 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 109 insertions(+), 75 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 7f0f011..f4e2b7a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -900,15 +900,95 @@ out:
 	goto out2;
 }
 
+enum cleanup_prefix_rt_t {
+	CLEANUP_PREFIX_RT_NOP,    /* no cleanup action for prefix route */
+	CLEANUP_PREFIX_RT_DEL,    /* delete the prefix route */
+	CLEANUP_PREFIX_RT_EXPIRE, /* update the lifetime of the prefix route */
+};
+
+/*
+ * Check, whether the prefix for ifp would still need a prefix route
+ * after deleting ifp. The function returns one of the CLEANUP_PREFIX_RT_*
+ * constants.
+ *
+ * 1) we don't purge prefix if address was not permanent.
+ *    prefix is managed by its own lifetime.
+ * 2) we also don't purge, if the address was IFA_F_NOPREFIXROUTE.
+ * 3) if there're no addresses, delete prefix.
+ * 4) if there're still other permanent address(es),
+ *    corresponding prefix is still permanent.
+ * 5) if there are still other addresses with IFA_F_NOPREFIXROUTE,
+ *    don't purge the prefix, assume user space is managing it.
+ * 6) otherwise, update prefix lifetime to the
+ *    longest valid lifetime among the corresponding
+ *    addresses on the device.
+ *    Note: subsequent RA will update lifetime.
+ **/
+static enum cleanup_prefix_rt_t
+check_cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long *expires)
+{
+	struct inet6_ifaddr *ifa;
+	struct inet6_dev *idev = ifp->idev;
+	unsigned long lifetime;
+	enum cleanup_prefix_rt_t action = CLEANUP_PREFIX_RT_DEL;
+
+	*expires = jiffies;
+
+	list_for_each_entry(ifa, &idev->addr_list, if_list) {
+		if (ifa == ifp)
+			continue;
+		if (!ipv6_prefix_equal(&ifa->addr, &ifp->addr,
+				       ifp->prefix_len))
+			continue;
+		if (ifa->flags & (IFA_F_PERMANENT | IFA_F_NOPREFIXROUTE))
+			return CLEANUP_PREFIX_RT_NOP;
+
+		action = CLEANUP_PREFIX_RT_EXPIRE;
+
+		spin_lock(&ifa->lock);
+
+		lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
+		/*
+		 * Note: Because this address is
+		 * not permanent, lifetime <
+		 * LONG_MAX / HZ here.
+		 */
+		if (time_before(*expires, ifa->tstamp + lifetime * HZ))
+			*expires = ifa->tstamp + lifetime * HZ;
+		spin_unlock(&ifa->lock);
+	}
+
+	return action;
+}
+
+static void
+cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long expires, bool del_rt)
+{
+	struct rt6_info *rt;
+
+	rt = addrconf_get_prefix_route(&ifp->addr,
+				       ifp->prefix_len,
+				       ifp->idev->dev,
+				       0, RTF_GATEWAY | RTF_DEFAULT);
+	if (rt) {
+		if (del_rt)
+			ip6_del_rt(rt);
+		else {
+			if (!(rt->rt6i_flags & RTF_EXPIRES))
+				rt6_set_expires(rt, expires);
+			ip6_rt_put(rt);
+		}
+	}
+}
+
+
 /* This function wants to get referenced ifp and releases it before return */
 
 static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 {
-	struct inet6_ifaddr *ifa, *ifn;
-	struct inet6_dev *idev = ifp->idev;
 	int state;
-	int deleted = 0, onlink = 0;
-	unsigned long expires = jiffies;
+	enum cleanup_prefix_rt_t action = CLEANUP_PREFIX_RT_NOP;
+	unsigned long expires;
 
 	spin_lock_bh(&ifp->state_lock);
 	state = ifp->state;
@@ -922,7 +1002,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 	hlist_del_init_rcu(&ifp->addr_lst);
 	spin_unlock_bh(&addrconf_hash_lock);
 
-	write_lock_bh(&idev->lock);
+	write_lock_bh(&ifp->idev->lock);
 
 	if (ifp->flags&IFA_F_TEMPORARY) {
 		list_del(&ifp->tmp_list);
@@ -933,45 +1013,13 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 		__in6_ifa_put(ifp);
 	}
 
-	list_for_each_entry_safe(ifa, ifn, &idev->addr_list, if_list) {
-		if (ifa == ifp) {
-			list_del_init(&ifp->if_list);
-			__in6_ifa_put(ifp);
+	if (ifp->flags & IFA_F_PERMANENT && !(ifp->flags & IFA_F_NOPREFIXROUTE))
+		action = check_cleanup_prefix_route(ifp, &expires);
 
-			if (!(ifp->flags & IFA_F_PERMANENT) || onlink > 0)
-				break;
-			deleted = 1;
-			continue;
-		} else if (ifp->flags & IFA_F_PERMANENT) {
-			if (ipv6_prefix_equal(&ifa->addr, &ifp->addr,
-					      ifp->prefix_len)) {
-				if (ifa->flags & IFA_F_PERMANENT) {
-					onlink = 1;
-					if (deleted)
-						break;
-				} else {
-					unsigned long lifetime;
-
-					if (!onlink)
-						onlink = -1;
-
-					spin_lock(&ifa->lock);
-
-					lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
-					/*
-					 * Note: Because this address is
-					 * not permanent, lifetime <
-					 * LONG_MAX / HZ here.
-					 */
-					if (time_before(expires,
-							ifa->tstamp + lifetime * HZ))
-						expires = ifa->tstamp + lifetime * HZ;
-					spin_unlock(&ifa->lock);
-				}
-			}
-		}
-	}
-	write_unlock_bh(&idev->lock);
+	list_del_init(&ifp->if_list);
+	__in6_ifa_put(ifp);
+
+	write_unlock_bh(&ifp->idev->lock);
 
 	addrconf_del_dad_timer(ifp);
 
@@ -979,38 +1027,9 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 
 	inet6addr_notifier_call_chain(NETDEV_DOWN, ifp);
 
-	/*
-	 * Purge or update corresponding prefix
-	 *
-	 * 1) we don't purge prefix here if address was not permanent.
-	 *    prefix is managed by its own lifetime.
-	 * 2) if there're no addresses, delete prefix.
-	 * 3) if there're still other permanent address(es),
-	 *    corresponding prefix is still permanent.
-	 * 4) otherwise, update prefix lifetime to the
-	 *    longest valid lifetime among the corresponding
-	 *    addresses on the device.
-	 *    Note: subsequent RA will update lifetime.
-	 *
-	 * --yoshfuji
-	 */
-	if ((ifp->flags & IFA_F_PERMANENT) && onlink < 1) {
-		struct rt6_info *rt;
-
-		rt = addrconf_get_prefix_route(&ifp->addr,
-					       ifp->prefix_len,
-					       ifp->idev->dev,
-					       0, RTF_GATEWAY | RTF_DEFAULT);
-
-		if (rt) {
-			if (onlink == 0) {
-				ip6_del_rt(rt);
-				rt = NULL;
-			} else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-				rt6_set_expires(rt, expires);
-			}
-		}
-		ip6_rt_put(rt);
+	if (action != CLEANUP_PREFIX_RT_NOP) {
+		cleanup_prefix_route(ifp, expires,
+			action == CLEANUP_PREFIX_RT_DEL);
 	}
 
 	/* clean up prefsrc entries */
@@ -3632,6 +3651,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	clock_t expires;
 	unsigned long timeout;
 	bool was_managetempaddr;
+	bool had_prefixroute;
 
 	if (!valid_lft || (prefered_lft > valid_lft))
 		return -EINVAL;
@@ -3660,6 +3680,8 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 
 	spin_lock_bh(&ifp->lock);
 	was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
+	had_prefixroute = ifp->flags & IFA_F_PERMANENT &&
+			  !(ifp->flags & IFA_F_NOPREFIXROUTE);
 	ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
 			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
 			IFA_F_NOPREFIXROUTE);
@@ -3675,6 +3697,18 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
 		addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
 				      expires, flags);
+	} else if (had_prefixroute) {
+		enum cleanup_prefix_rt_t action;
+		unsigned long rt_expires;
+
+		write_lock_bh(&ifp->idev->lock);
+		action = check_cleanup_prefix_route(ifp, &rt_expires);
+		write_unlock_bh(&ifp->idev->lock);
+
+		if (action != CLEANUP_PREFIX_RT_NOP) {
+			cleanup_prefix_route(ifp, rt_expires,
+				action == CLEANUP_PREFIX_RT_DEL);
+		}
 	}
 
 	if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
-- 
1.8.4.2

Re: [PATCH v4 2/2] ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE

[Hannes Frederic Sowa]

On Thu, Jan 09, 2014 at 01:30:04AM +0100, Thomas Haller wrote:
> Refactor the deletion/update of prefix routes when removing an
> address. Now also consider IFA_F_NOPREFIXROUTE and if there is an address
> present with this flag, to not cleanup the route. Instead, assume
> that userspace is taking care of this route.
> 
> Also perform the same cleanup, when userspace changes an existing address
> to add NOPREFIXROUTE (to an address that didn't have this flag). This is
> done because when the address was added, a prefix route was created for it.
> Since the user now wants to handle this route by himself, we cleanup this
> route.
> 
> This cleanup of the route is not totally robust. There is no guarantee,
> that the route we are about to delete was really the one added by the
> kernel. This behavior does not change by the patch, and in practice it
> should work just fine.
> 
> Signed-off-by: Thomas Haller <thaller@redhat.com>

I think, this is now simpler than the original version.

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Thanks!

Re: [PATCH v4 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[David Miller]

From: Thomas Haller <thaller@redhat.com>
Date: Thu,  9 Jan 2014 01:30:02 +0100

> v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
> v2 -> v3: reword commit messages, code comments and some refactoring.
> v3 -> v4: refactor, rename variables, add enum
> 
> Thomas Haller (2):
>   ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
>     IP6 routes
>   ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE

Series applied, thanks Thomas.

Re: [PATCH v4 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Jiri Pirko]

Sat, Jan 11, 2014 at 12:10:30AM CET, davem@davemloft.net wrote:
>From: Thomas Haller <thaller@redhat.com>
>Date: Thu,  9 Jan 2014 01:30:02 +0100
>
>> v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
>> v2 -> v3: reword commit messages, code comments and some refactoring.
>> v3 -> v4: refactor, rename variables, add enum
>> 
>> Thomas Haller (2):
>>   ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
>>     IP6 routes
>>   ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE
>
>Series applied, thanks Thomas.

Hi Dave. Have you pushed this already? I can't see these patches in
net-next.

Thanks

Re: [PATCH v4 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 13 Jan 2014 16:31:10 +0100

> Sat, Jan 11, 2014 at 12:10:30AM CET, davem@davemloft.net wrote:
>>From: Thomas Haller <thaller@redhat.com>
>>Date: Thu,  9 Jan 2014 01:30:02 +0100
>>
>>> v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
>>> v2 -> v3: reword commit messages, code comments and some refactoring.
>>> v3 -> v4: refactor, rename variables, add enum
>>> 
>>> Thomas Haller (2):
>>>   ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
>>>     IP6 routes
>>>   ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE
>>
>>Series applied, thanks Thomas.
> 
> Hi Dave. Have you pushed this already? I can't see these patches in
> net-next.

Sorry, I must have forgotten to push these changes out before I travelled
on Saturday.

Please respin and resubmit and I'll make sure they get integrated properly.

Thanks.

Re: [PATCH v4 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Hannes Frederic Sowa]

On Wed, Jan 15, 2014 at 01:53:28PM -0800, David Miller wrote:
> From: Jiri Pirko <jiri@resnulli.us>
> Date: Mon, 13 Jan 2014 16:31:10 +0100
> 
> > Sat, Jan 11, 2014 at 12:10:30AM CET, davem@davemloft.net wrote:
> >>From: Thomas Haller <thaller@redhat.com>
> >>Date: Thu,  9 Jan 2014 01:30:02 +0100
> >>
> >>> v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
> >>> v2 -> v3: reword commit messages, code comments and some refactoring.
> >>> v3 -> v4: refactor, rename variables, add enum
> >>> 
> >>> Thomas Haller (2):
> >>>   ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
> >>>     IP6 routes
> >>>   ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE
> >>
> >>Series applied, thanks Thomas.
> > 
> > Hi Dave. Have you pushed this already? I can't see these patches in
> > net-next.
> 
> Sorry, I must have forgotten to push these changes out before I travelled
> on Saturday.
> 
> Please respin and resubmit and I'll make sure they get integrated properly.

Just fyi, I noticed at least one other patch missing from the repos:
http://patchwork.ozlabs.org/patch/308452/

Greetings,

  Hannes

Re: [PATCH v4 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 16 Jan 2014 14:38:18 +0100

> Just fyi, I noticed at least one other patch missing from the repos:
> http://patchwork.ozlabs.org/patch/308452/

Thanks for catching that, taking care of it now.

[PATCH v5 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Thomas Haller]

v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
v2 -> v3: reword commit messages, code comments and some refactoring.
v3 -> v4: refactor, rename variables, add enum
v4 -> v5: rebase, so that patch applies cleanly to current net-next/master

Thomas Haller (2):
  ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
    IP6 routes
  ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE

 include/uapi/linux/if_addr.h |   1 +
 net/ipv6/addrconf.c          | 203 ++++++++++++++++++++++++++-----------------
 2 files changed, 123 insertions(+), 81 deletions(-)

-- 
1.8.4.2

[PATCH v5 1/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Thomas Haller]

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 for the
prefix should be added.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 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 1b2e4ee..85100c1 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2434,8 +2434,11 @@ static int inet6_addr_add(struct net *net, int ifindex,
 			    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)) {
+			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,7 +3665,8 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	spin_lock_bh(&ifp->lock);
 	was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
 	ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
-			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR);
+			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
+			IFA_F_NOPREFIXROUTE);
 	ifp->flags |= ifa_flags;
 	ifp->tstamp = jiffies;
 	ifp->valid_lft = valid_lft;
@@ -3672,8 +3676,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)) {
+		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))
@@ -3727,7 +3733,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

[PATCH v5 2/2] ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE

[Thomas Haller]

Refactor the deletion/update of prefix routes when removing an
address. Now also consider IFA_F_NOPREFIXROUTE and if there is an address
present with this flag, to not cleanup the route. Instead, assume
that userspace is taking care of this route.

Also perform the same cleanup, when userspace changes an existing address
to add NOPREFIXROUTE (to an address that didn't have this flag). This is
done because when the address was added, a prefix route was created for it.
Since the user now wants to handle this route by himself, we cleanup this
route.

This cleanup of the route is not totally robust. There is no guarantee,
that the route we are about to delete was really the one added by the
kernel. This behavior does not change by the patch, and in practice it
should work just fine.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 net/ipv6/addrconf.c | 184 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 109 insertions(+), 75 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 85100c1..6913a82 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -900,15 +900,95 @@ out:
 	goto out2;
 }
 
+enum cleanup_prefix_rt_t {
+	CLEANUP_PREFIX_RT_NOP,    /* no cleanup action for prefix route */
+	CLEANUP_PREFIX_RT_DEL,    /* delete the prefix route */
+	CLEANUP_PREFIX_RT_EXPIRE, /* update the lifetime of the prefix route */
+};
+
+/*
+ * Check, whether the prefix for ifp would still need a prefix route
+ * after deleting ifp. The function returns one of the CLEANUP_PREFIX_RT_*
+ * constants.
+ *
+ * 1) we don't purge prefix if address was not permanent.
+ *    prefix is managed by its own lifetime.
+ * 2) we also don't purge, if the address was IFA_F_NOPREFIXROUTE.
+ * 3) if there are no addresses, delete prefix.
+ * 4) if there are still other permanent address(es),
+ *    corresponding prefix is still permanent.
+ * 5) if there are still other addresses with IFA_F_NOPREFIXROUTE,
+ *    don't purge the prefix, assume user space is managing it.
+ * 6) otherwise, update prefix lifetime to the
+ *    longest valid lifetime among the corresponding
+ *    addresses on the device.
+ *    Note: subsequent RA will update lifetime.
+ **/
+static enum cleanup_prefix_rt_t
+check_cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long *expires)
+{
+	struct inet6_ifaddr *ifa;
+	struct inet6_dev *idev = ifp->idev;
+	unsigned long lifetime;
+	enum cleanup_prefix_rt_t action = CLEANUP_PREFIX_RT_DEL;
+
+	*expires = jiffies;
+
+	list_for_each_entry(ifa, &idev->addr_list, if_list) {
+		if (ifa == ifp)
+			continue;
+		if (!ipv6_prefix_equal(&ifa->addr, &ifp->addr,
+				       ifp->prefix_len))
+			continue;
+		if (ifa->flags & (IFA_F_PERMANENT | IFA_F_NOPREFIXROUTE))
+			return CLEANUP_PREFIX_RT_NOP;
+
+		action = CLEANUP_PREFIX_RT_EXPIRE;
+
+		spin_lock(&ifa->lock);
+
+		lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
+		/*
+		 * Note: Because this address is
+		 * not permanent, lifetime <
+		 * LONG_MAX / HZ here.
+		 */
+		if (time_before(*expires, ifa->tstamp + lifetime * HZ))
+			*expires = ifa->tstamp + lifetime * HZ;
+		spin_unlock(&ifa->lock);
+	}
+
+	return action;
+}
+
+static void
+cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long expires, bool del_rt)
+{
+	struct rt6_info *rt;
+
+	rt = addrconf_get_prefix_route(&ifp->addr,
+				       ifp->prefix_len,
+				       ifp->idev->dev,
+				       0, RTF_GATEWAY | RTF_DEFAULT);
+	if (rt) {
+		if (del_rt)
+			ip6_del_rt(rt);
+		else {
+			if (!(rt->rt6i_flags & RTF_EXPIRES))
+				rt6_set_expires(rt, expires);
+			ip6_rt_put(rt);
+		}
+	}
+}
+
+
 /* This function wants to get referenced ifp and releases it before return */
 
 static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 {
-	struct inet6_ifaddr *ifa, *ifn;
-	struct inet6_dev *idev = ifp->idev;
 	int state;
-	int deleted = 0, onlink = 0;
-	unsigned long expires = jiffies;
+	enum cleanup_prefix_rt_t action = CLEANUP_PREFIX_RT_NOP;
+	unsigned long expires;
 
 	spin_lock_bh(&ifp->state_lock);
 	state = ifp->state;
@@ -922,7 +1002,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 	hlist_del_init_rcu(&ifp->addr_lst);
 	spin_unlock_bh(&addrconf_hash_lock);
 
-	write_lock_bh(&idev->lock);
+	write_lock_bh(&ifp->idev->lock);
 
 	if (ifp->flags&IFA_F_TEMPORARY) {
 		list_del(&ifp->tmp_list);
@@ -933,45 +1013,13 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 		__in6_ifa_put(ifp);
 	}
 
-	list_for_each_entry_safe(ifa, ifn, &idev->addr_list, if_list) {
-		if (ifa == ifp) {
-			list_del_init(&ifp->if_list);
-			__in6_ifa_put(ifp);
+	if (ifp->flags & IFA_F_PERMANENT && !(ifp->flags & IFA_F_NOPREFIXROUTE))
+		action = check_cleanup_prefix_route(ifp, &expires);
 
-			if (!(ifp->flags & IFA_F_PERMANENT) || onlink > 0)
-				break;
-			deleted = 1;
-			continue;
-		} else if (ifp->flags & IFA_F_PERMANENT) {
-			if (ipv6_prefix_equal(&ifa->addr, &ifp->addr,
-					      ifp->prefix_len)) {
-				if (ifa->flags & IFA_F_PERMANENT) {
-					onlink = 1;
-					if (deleted)
-						break;
-				} else {
-					unsigned long lifetime;
-
-					if (!onlink)
-						onlink = -1;
-
-					spin_lock(&ifa->lock);
-
-					lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
-					/*
-					 * Note: Because this address is
-					 * not permanent, lifetime <
-					 * LONG_MAX / HZ here.
-					 */
-					if (time_before(expires,
-							ifa->tstamp + lifetime * HZ))
-						expires = ifa->tstamp + lifetime * HZ;
-					spin_unlock(&ifa->lock);
-				}
-			}
-		}
-	}
-	write_unlock_bh(&idev->lock);
+	list_del_init(&ifp->if_list);
+	__in6_ifa_put(ifp);
+
+	write_unlock_bh(&ifp->idev->lock);
 
 	addrconf_del_dad_timer(ifp);
 
@@ -979,38 +1027,9 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 
 	inet6addr_notifier_call_chain(NETDEV_DOWN, ifp);
 
-	/*
-	 * Purge or update corresponding prefix
-	 *
-	 * 1) we don't purge prefix here if address was not permanent.
-	 *    prefix is managed by its own lifetime.
-	 * 2) if there are no addresses, delete prefix.
-	 * 3) if there are still other permanent address(es),
-	 *    corresponding prefix is still permanent.
-	 * 4) otherwise, update prefix lifetime to the
-	 *    longest valid lifetime among the corresponding
-	 *    addresses on the device.
-	 *    Note: subsequent RA will update lifetime.
-	 *
-	 * --yoshfuji
-	 */
-	if ((ifp->flags & IFA_F_PERMANENT) && onlink < 1) {
-		struct rt6_info *rt;
-
-		rt = addrconf_get_prefix_route(&ifp->addr,
-					       ifp->prefix_len,
-					       ifp->idev->dev,
-					       0, RTF_GATEWAY | RTF_DEFAULT);
-
-		if (rt) {
-			if (onlink == 0) {
-				ip6_del_rt(rt);
-				rt = NULL;
-			} else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-				rt6_set_expires(rt, expires);
-			}
-		}
-		ip6_rt_put(rt);
+	if (action != CLEANUP_PREFIX_RT_NOP) {
+		cleanup_prefix_route(ifp, expires,
+			action == CLEANUP_PREFIX_RT_DEL);
 	}
 
 	/* clean up prefsrc entries */
@@ -3636,6 +3655,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	clock_t expires;
 	unsigned long timeout;
 	bool was_managetempaddr;
+	bool had_prefixroute;
 
 	if (!valid_lft || (prefered_lft > valid_lft))
 		return -EINVAL;
@@ -3664,6 +3684,8 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 
 	spin_lock_bh(&ifp->lock);
 	was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
+	had_prefixroute = ifp->flags & IFA_F_PERMANENT &&
+			  !(ifp->flags & IFA_F_NOPREFIXROUTE);
 	ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
 			IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
 			IFA_F_NOPREFIXROUTE);
@@ -3679,6 +3701,18 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
 	if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
 		addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
 				      expires, flags);
+	} else if (had_prefixroute) {
+		enum cleanup_prefix_rt_t action;
+		unsigned long rt_expires;
+
+		write_lock_bh(&ifp->idev->lock);
+		action = check_cleanup_prefix_route(ifp, &rt_expires);
+		write_unlock_bh(&ifp->idev->lock);
+
+		if (action != CLEANUP_PREFIX_RT_NOP) {
+			cleanup_prefix_route(ifp, rt_expires,
+				action == CLEANUP_PREFIX_RT_DEL);
+		}
 	}
 
 	if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
-- 
1.8.4.2

Re: [PATCH v5 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Hannes Frederic Sowa]

On Wed, Jan 15, 2014 at 03:36:57PM +0100, Thomas Haller wrote:
> v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
> v2 -> v3: reword commit messages, code comments and some refactoring.
> v3 -> v4: refactor, rename variables, add enum
> v4 -> v5: rebase, so that patch applies cleanly to current net-next/master

Huh? I thought those were already applied?

Hmm, they seem to be in no repository? I hope that no other patches are
affected, too.

Greetings,

  Hannes

Re: [PATCH v5 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[Thomas Haller]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

On Wed, 2014-01-15 at 15:43 +0100, Hannes Frederic Sowa wrote:
> On Wed, Jan 15, 2014 at 03:36:57PM +0100, Thomas Haller wrote:
> > v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
> > v2 -> v3: reword commit messages, code comments and some refactoring.
> > v3 -> v4: refactor, rename variables, add enum
> > v4 -> v5: rebase, so that patch applies cleanly to current net-next/master
> 
> Huh? I thought those were already applied?
> 
> Hmm, they seem to be in no repository? I hope that no other patches are
> affected, too.
> 
> Greetings,
> 
>   Hannes
> 

I thought so too, but I cannot find it, so I re-posted v5.

Thomas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v5 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes

[David Miller]

From: Thomas Haller <thaller@redhat.com>
Date: Wed, 15 Jan 2014 15:36:57 +0100

> v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
> v2 -> v3: reword commit messages, code comments and some refactoring.
> v3 -> v4: refactor, rename variables, add enum
> v4 -> v5: rebase, so that patch applies cleanly to current net-next/master

Applied, sorry for not pushing these out properly last time.