Re: [PATCH] ipv6: Reparent temporary address(es) if global address was deleted from userspace

[Daniel Borkmann <dborkman@redhat.com>]

On 04/06/2014 09:04 PM, Heiner Kallweit wrote:
> If the kernel takes care of global and temporary addresses it can happen that the global
> address is deleted from userspace, e.g. by network managers in case the link goes (temporarily) down.
> In addition to the then orphaned temporary address(es) the next RA will create a new temporary address.
> Therefore we might end up with more than one non-deprecated temporary address for the same prefix
> for a longer period of time (>regen_advance). According to RfC 4941 sect. 3.4 this should not happen.

Nit: your commit description should only be ~75 chars wide max

> Fix this by reparenting orphaned temporary addresses.
>
> Signed-off-by: Heiner Kallweit <heiner.kallweit@web.de>
> ---
>   net/ipv6/addrconf.c | 34 +++++++++++++++++++++++++++++++---
>   1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 6c7fa08..4d5595b 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2077,14 +2077,34 @@ static void manage_tempaddrs(struct inet6_dev *idev,
>   {
>   	u32 flags;
>   	struct inet6_ifaddr *ift;
> +	unsigned long regen_advance;
>
>   	read_lock_bh(&idev->lock);
> +	regen_advance = idev->cnf.regen_max_retry *
> +			idev->cnf.dad_transmits *
> +			NEIGH_VAR(idev->nd_parms, RETRANS_TIME) / HZ;
>   	/* update all temporary addresses in the list */
>   	list_for_each_entry(ift, &idev->tempaddr_list, tmp_list) {
>   		int age, max_valid, max_prefered;
> +		bool reparent;
>
> -		if (ifp != ift->ifpub)
> -			continue;
> +		reparent = false;

You can collapse initialization where you declare the variable.

> +		spin_lock(&ift->lock);
> +		if (ifp != ift->ifpub) {
> +			/* reparent if orphaned temporary address with same
> +			 * prefix is found
> +			 */
> +			if (ipv6_prefix_equal(&ift->addr, &ifp->addr,
> +					      ift->prefix_len)) {
> +				in6_ifa_hold(ifp);
> +				in6_ifa_put(ift->ifpub);
> +				ift->ifpub = ifp;
> +				reparent = true;
> +			} else {
> +				spin_unlock(&ift->lock);
> +				continue;
> +			}
> +		}
>
>   		/* RFC 4941 section 3.3:
>   		 * If a received option will extend the lifetime of a public
> @@ -2110,7 +2130,6 @@ static void manage_tempaddrs(struct inet6_dev *idev,
>   		if (prefered_lft > max_prefered)
>   			prefered_lft = max_prefered;
>
> -		spin_lock(&ift->lock);
>   		flags = ift->flags;
>   		ift->valid_lft = valid_lft;
>   		ift->prefered_lft = prefered_lft;
> @@ -2119,6 +2138,15 @@ static void manage_tempaddrs(struct inet6_dev *idev,
>   			ift->flags &= ~IFA_F_DEPRECATED;
>
>   		spin_unlock(&ift->lock);
> +		/* Don't create a temporary address if we reparented one which
> +		 * is prefered more than regen_advance

Nit: s/prefered/preferred/

> +		 */
> +		if (reparent && prefered_lft > regen_advance) {
> +			create = false;
> +			pr_info("%s: reparent orphaned temporary address\n",
> +				__func__);

The pr_info() should ideally be removed (or at least made pr_debug()).

> +		}
> +
>   		if (!(flags&IFA_F_TENTATIVE))
>   			ipv6_ifa_notify(0, ift);
>   	}
>