[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <53423512.2030706@web.de>
Date: Mon, 07 Apr 2014 07:18:10 +0200
From: Heiner Kallweit <heiner.kallweit@....de>
To: Daniel Borkmann <dborkman@...hat.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Hannes Frederic Sowa <hannes@...essinduktion.org>
Subject: Re: [PATCH] ipv6: Reparent temporary address(es) if global address
was deleted from userspace
Am 06.04.2014 22:27, schrieb Daniel Borkmann:
> 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@....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);
>> }
>>
>
Thanks for the hints, will send a revised patch.
Rgds, Heiner
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists