diff mbox

[RFC,07/12] addrconf: put prefix address add in an own function

Message ID 1464031328-17524-8-git-send-email-aar@pengutronix.de
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Alexander Aring May 23, 2016, 7:22 p.m. UTC
This patch moves the functionality to add a RA PIO prefix generated
address in an own function. This move prepares to add a hook for
adding a second address for a second link-layer address. E.g. short
address for 802.15.4 6LoWPAN.

Cc: David S. Miller <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Alexander Aring <aar@pengutronix.de>
---
 net/ipv6/addrconf.c | 191 ++++++++++++++++++++++++++++------------------------
 1 file changed, 102 insertions(+), 89 deletions(-)

Comments

Stefan Schmidt May 27, 2016, 9:45 a.m. UTC | #1
Hello.

On 23/05/16 21:22, Alexander Aring wrote:
> This patch moves the functionality to add a RA PIO prefix generated
> address in an own function. This move prepares to add a hook for
> adding a second address for a second link-layer address. E.g. short
> address for 802.15.4 6LoWPAN.
>
> Cc: David S. Miller<davem@davemloft.net>
> Cc: Alexey Kuznetsov<kuznet@ms2.inr.ac.ru>
> Cc: James Morris<jmorris@namei.org>
> Cc: Hideaki YOSHIFUJI<yoshfuji@linux-ipv6.org>
> Cc: Patrick McHardy<kaber@trash.net>
> Signed-off-by: Alexander Aring<aar@pengutronix.de>
> ---
>   net/ipv6/addrconf.c | 191 ++++++++++++++++++++++++++++------------------------
>   1 file changed, 102 insertions(+), 89 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index beaad49..393cdbf 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2333,6 +2333,104 @@ static bool is_addr_mode_generate_stable(struct inet6_dev *idev)
>   	       idev->addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
>   }
>   
> +static void addrconf_prefix_rcv_add_addr(struct net *net,
> +					 struct net_device *dev,
> +					 const struct prefix_info *pinfo,
> +					 struct inet6_dev *in6_dev,
> +					 const struct in6_addr *addr,
> +					 int addr_type, u32 addr_flags,
> +					 bool sllao, bool tokenized,
> +					 __u32 valid_lft, u32 prefered_lft)
> +{
> +	struct inet6_ifaddr *ifp = ipv6_get_ifaddr(net, addr, dev, 1);
> +	int create = 0, update_lft = 0;
> +
> +	if (!ifp && valid_lft) {
> +		int max_addresses = in6_dev->cnf.max_addresses;
> +
> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
> +		if (in6_dev->cnf.optimistic_dad &&
> +		    !net->ipv6.devconf_all->forwarding && sllao)
> +			addr_flags |= IFA_F_OPTIMISTIC;
> +#endif
> +
> +		/* Do not allow to create too much of autoconfigured
> +		 * addresses; this would be too easy way to crash kernel.
> +		 */
> +		if (!max_addresses ||
> +		    ipv6_count_addresses(in6_dev) < max_addresses)
> +			ifp = ipv6_add_addr(in6_dev, addr, NULL,
> +					    pinfo->prefix_len,
> +					    addr_type&IPV6_ADDR_SCOPE_MASK,
> +					    addr_flags, valid_lft,
> +					    prefered_lft);
> +
> +		if (IS_ERR_OR_NULL(ifp)) {
> +			in6_dev_put(in6_dev);
> +			return;
> +		}
> +
> +		update_lft = 0;
> +		create = 1;
> +		spin_lock_bh(&ifp->lock);
> +		ifp->flags |= IFA_F_MANAGETEMPADDR;
> +		ifp->cstamp = jiffies;
> +		ifp->tokenized = tokenized;
> +		spin_unlock_bh(&ifp->lock);
> +		addrconf_dad_start(ifp);
> +	}
> +
> +	if (ifp) {
> +		u32 flags;
> +		unsigned long now;
> +		u32 stored_lft;
> +
> +		/* update lifetime (RFC2462 5.5.3 e) */
> +		spin_lock_bh(&ifp->lock);
> +		now = jiffies;
> +		if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
> +			stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
> +		else
> +			stored_lft = 0;
> +		if (!update_lft && !create && stored_lft) {
> +			const u32 minimum_lft = min_t(u32,
> +				stored_lft, MIN_VALID_LIFETIME);
> +			valid_lft = max(valid_lft, minimum_lft);
> +
> +			/* RFC4862 Section 5.5.3e:
> +			 * "Note that the preferred lifetime of the
> +			 *  corresponding address is always reset to
> +			 *  the Preferred Lifetime in the received
> +			 *  Prefix Information option, regardless of
> +			 *  whether the valid lifetime is also reset or
> +			 *  ignored."
> +			 *
> +			 * So we should always update prefered_lft here.
> +			 */
> +			update_lft = 1;
> +		}
> +
> +		if (update_lft) {
> +			ifp->valid_lft = valid_lft;
> +			ifp->prefered_lft = prefered_lft;
> +			ifp->tstamp = now;
> +			flags = ifp->flags;
> +			ifp->flags &= ~IFA_F_DEPRECATED;
> +			spin_unlock_bh(&ifp->lock);
> +
> +			if (!(flags&IFA_F_TENTATIVE))
> +				ipv6_ifa_notify(0, ifp);
> +		} else
> +			spin_unlock_bh(&ifp->lock);
> +
> +		manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
> +				 create, now);
> +
> +		in6_ifa_put(ifp);
> +		addrconf_verify();
> +	}
> +}
> +
>   void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>   {
>   	struct prefix_info *pinfo;
> @@ -2432,9 +2530,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>   	/* Try to figure out our local address for this prefix */
>   
>   	if (pinfo->autoconf && in6_dev->cnf.autoconf) {
> -		struct inet6_ifaddr *ifp;
>   		struct in6_addr addr;
> -		int create = 0, update_lft = 0;
>   		bool tokenized = false;
>   
>   		if (pinfo->prefix_len == 64) {
> @@ -2464,93 +2560,10 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>   		return;
>   
>   ok:
> -
> -		ifp = ipv6_get_ifaddr(net, &addr, dev, 1);
> -
> -		if (!ifp && valid_lft) {
> -			int max_addresses = in6_dev->cnf.max_addresses;
> -
> -#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
> -			if (in6_dev->cnf.optimistic_dad &&
> -			    !net->ipv6.devconf_all->forwarding && sllao)
> -				addr_flags |= IFA_F_OPTIMISTIC;
> -#endif
> -
> -			/* Do not allow to create too much of autoconfigured
> -			 * addresses; this would be too easy way to crash kernel.
> -			 */
> -			if (!max_addresses ||
> -			    ipv6_count_addresses(in6_dev) < max_addresses)
> -				ifp = ipv6_add_addr(in6_dev, &addr, NULL,
> -						    pinfo->prefix_len,
> -						    addr_type&IPV6_ADDR_SCOPE_MASK,
> -						    addr_flags, valid_lft,
> -						    prefered_lft);
> -
> -			if (IS_ERR_OR_NULL(ifp)) {
> -				in6_dev_put(in6_dev);
> -				return;
> -			}
> -
> -			update_lft = 0;
> -			create = 1;
> -			spin_lock_bh(&ifp->lock);
> -			ifp->flags |= IFA_F_MANAGETEMPADDR;
> -			ifp->cstamp = jiffies;
> -			ifp->tokenized = tokenized;
> -			spin_unlock_bh(&ifp->lock);
> -			addrconf_dad_start(ifp);
> -		}
> -
> -		if (ifp) {
> -			u32 flags;
> -			unsigned long now;
> -			u32 stored_lft;
> -
> -			/* update lifetime (RFC2462 5.5.3 e) */
> -			spin_lock_bh(&ifp->lock);
> -			now = jiffies;
> -			if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
> -				stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
> -			else
> -				stored_lft = 0;
> -			if (!update_lft && !create && stored_lft) {
> -				const u32 minimum_lft = min_t(u32,
> -					stored_lft, MIN_VALID_LIFETIME);
> -				valid_lft = max(valid_lft, minimum_lft);
> -
> -				/* RFC4862 Section 5.5.3e:
> -				 * "Note that the preferred lifetime of the
> -				 *  corresponding address is always reset to
> -				 *  the Preferred Lifetime in the received
> -				 *  Prefix Information option, regardless of
> -				 *  whether the valid lifetime is also reset or
> -				 *  ignored."
> -				 *
> -				 * So we should always update prefered_lft here.
> -				 */
> -				update_lft = 1;
> -			}
> -
> -			if (update_lft) {
> -				ifp->valid_lft = valid_lft;
> -				ifp->prefered_lft = prefered_lft;
> -				ifp->tstamp = now;
> -				flags = ifp->flags;
> -				ifp->flags &= ~IFA_F_DEPRECATED;
> -				spin_unlock_bh(&ifp->lock);
> -
> -				if (!(flags&IFA_F_TENTATIVE))
> -					ipv6_ifa_notify(0, ifp);
> -			} else
> -				spin_unlock_bh(&ifp->lock);
> -
> -			manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
> -					 create, now);
> -
> -			in6_ifa_put(ifp);
> -			addrconf_verify();
> -		}
> +		addrconf_prefix_rcv_add_addr(net, dev, pinfo, in6_dev, &addr,
> +					     addr_type, addr_flags, sllao,
> +					     tokenized, valid_lft,
> +					     prefered_lft);
>   	}
>   	inet6_prefix_notify(RTM_NEWPREFIX, in6_dev, pinfo);
>   	in6_dev_put(in6_dev);

Reviewed-by: Stefan Schmidt<stefan@osg.samsung.com>

regards
Stefan Schmidt
Alexander Aring May 27, 2016, 11:41 a.m. UTC | #2
Hi,

On 05/27/2016 11:45 AM, Stefan Schmidt wrote:
> Hello.
> 

sorry, again I will change something on this patch which is buggy.

> On 23/05/16 21:22, Alexander Aring wrote:
>> This patch moves the functionality to add a RA PIO prefix generated
>> address in an own function. This move prepares to add a hook for
>> adding a second address for a second link-layer address. E.g. short
>> address for 802.15.4 6LoWPAN.
>>
>> Cc: David S. Miller<davem@davemloft.net>
>> Cc: Alexey Kuznetsov<kuznet@ms2.inr.ac.ru>
>> Cc: James Morris<jmorris@namei.org>
>> Cc: Hideaki YOSHIFUJI<yoshfuji@linux-ipv6.org>
>> Cc: Patrick McHardy<kaber@trash.net>
>> Signed-off-by: Alexander Aring<aar@pengutronix.de>
>> ---
>>   net/ipv6/addrconf.c | 191 ++++++++++++++++++++++++++++------------------------
>>   1 file changed, 102 insertions(+), 89 deletions(-)
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index beaad49..393cdbf 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -2333,6 +2333,104 @@ static bool is_addr_mode_generate_stable(struct inet6_dev *idev)
>>              idev->addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
>>   }
>>   +static void addrconf_prefix_rcv_add_addr(struct net *net,

will change to return int here to indicates errors.

>> +                     struct net_device *dev,
>> +                     const struct prefix_info *pinfo,
>> +                     struct inet6_dev *in6_dev,
>> +                     const struct in6_addr *addr,
>> +                     int addr_type, u32 addr_flags,
>> +                     bool sllao, bool tokenized,
>> +                     __u32 valid_lft, u32 prefered_lft)
>> +{
>> +    struct inet6_ifaddr *ifp = ipv6_get_ifaddr(net, addr, dev, 1);
>> +    int create = 0, update_lft = 0;
>> +
>> +    if (!ifp && valid_lft) {
>> +        int max_addresses = in6_dev->cnf.max_addresses;
>> +
>> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>> +        if (in6_dev->cnf.optimistic_dad &&
>> +            !net->ipv6.devconf_all->forwarding && sllao)
>> +            addr_flags |= IFA_F_OPTIMISTIC;
>> +#endif
>> +
>> +        /* Do not allow to create too much of autoconfigured
>> +         * addresses; this would be too easy way to crash kernel.
>> +         */
>> +        if (!max_addresses ||
>> +            ipv6_count_addresses(in6_dev) < max_addresses)
>> +            ifp = ipv6_add_addr(in6_dev, addr, NULL,
>> +                        pinfo->prefix_len,
>> +                        addr_type&IPV6_ADDR_SCOPE_MASK,
>> +                        addr_flags, valid_lft,
>> +                        prefered_lft);
>> +
>> +        if (IS_ERR_OR_NULL(ifp)) {
>> +            in6_dev_put(in6_dev);
>> +            return;

return -1; and remove in6_dev_put stuff.

>> +        }
>> +
>> +        update_lft = 0;
>> +        create = 1;
>> +        spin_lock_bh(&ifp->lock);
>> +        ifp->flags |= IFA_F_MANAGETEMPADDR;
>> +        ifp->cstamp = jiffies;
>> +        ifp->tokenized = tokenized;
>> +        spin_unlock_bh(&ifp->lock);
>> +        addrconf_dad_start(ifp);
>> +    }
>> +
>> +    if (ifp) {
>> +        u32 flags;
>> +        unsigned long now;
>> +        u32 stored_lft;
>> +
>> +        /* update lifetime (RFC2462 5.5.3 e) */
>> +        spin_lock_bh(&ifp->lock);
>> +        now = jiffies;
>> +        if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
>> +            stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
>> +        else
>> +            stored_lft = 0;
>> +        if (!update_lft && !create && stored_lft) {
>> +            const u32 minimum_lft = min_t(u32,
>> +                stored_lft, MIN_VALID_LIFETIME);
>> +            valid_lft = max(valid_lft, minimum_lft);
>> +
>> +            /* RFC4862 Section 5.5.3e:
>> +             * "Note that the preferred lifetime of the
>> +             *  corresponding address is always reset to
>> +             *  the Preferred Lifetime in the received
>> +             *  Prefix Information option, regardless of
>> +             *  whether the valid lifetime is also reset or
>> +             *  ignored."
>> +             *
>> +             * So we should always update prefered_lft here.
>> +             */
>> +            update_lft = 1;
>> +        }
>> +
>> +        if (update_lft) {
>> +            ifp->valid_lft = valid_lft;
>> +            ifp->prefered_lft = prefered_lft;
>> +            ifp->tstamp = now;
>> +            flags = ifp->flags;
>> +            ifp->flags &= ~IFA_F_DEPRECATED;
>> +            spin_unlock_bh(&ifp->lock);
>> +
>> +            if (!(flags&IFA_F_TENTATIVE))
>> +                ipv6_ifa_notify(0, ifp);
>> +        } else
>> +            spin_unlock_bh(&ifp->lock);
>> +
>> +        manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
>> +                 create, now);
>> +
>> +        in6_ifa_put(ifp);
>> +        addrconf_verify();
>> +    }
>> +}
>> +
>>   void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>>   {
>>       struct prefix_info *pinfo;
>> @@ -2432,9 +2530,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>>       /* Try to figure out our local address for this prefix */
>>         if (pinfo->autoconf && in6_dev->cnf.autoconf) {
>> -        struct inet6_ifaddr *ifp;
>>           struct in6_addr addr;
>> -        int create = 0, update_lft = 0;
>>           bool tokenized = false;
>>             if (pinfo->prefix_len == 64) {
>> @@ -2464,93 +2560,10 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>>           return;
>>     ok:
>> -
>> -        ifp = ipv6_get_ifaddr(net, &addr, dev, 1);
>> -
>> -        if (!ifp && valid_lft) {
>> -            int max_addresses = in6_dev->cnf.max_addresses;
>> -
>> -#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>> -            if (in6_dev->cnf.optimistic_dad &&
>> -                !net->ipv6.devconf_all->forwarding && sllao)
>> -                addr_flags |= IFA_F_OPTIMISTIC;
>> -#endif
>> -
>> -            /* Do not allow to create too much of autoconfigured
>> -             * addresses; this would be too easy way to crash kernel.
>> -             */
>> -            if (!max_addresses ||
>> -                ipv6_count_addresses(in6_dev) < max_addresses)
>> -                ifp = ipv6_add_addr(in6_dev, &addr, NULL,
>> -                            pinfo->prefix_len,
>> -                            addr_type&IPV6_ADDR_SCOPE_MASK,
>> -                            addr_flags, valid_lft,
>> -                            prefered_lft);
>> -
>> -            if (IS_ERR_OR_NULL(ifp)) {
>> -                in6_dev_put(in6_dev);
>> -                return;
>> -            }
>> -
>> -            update_lft = 0;
>> -            create = 1;
>> -            spin_lock_bh(&ifp->lock);
>> -            ifp->flags |= IFA_F_MANAGETEMPADDR;
>> -            ifp->cstamp = jiffies;
>> -            ifp->tokenized = tokenized;
>> -            spin_unlock_bh(&ifp->lock);
>> -            addrconf_dad_start(ifp);
>> -        }
>> -
>> -        if (ifp) {
>> -            u32 flags;
>> -            unsigned long now;
>> -            u32 stored_lft;
>> -
>> -            /* update lifetime (RFC2462 5.5.3 e) */
>> -            spin_lock_bh(&ifp->lock);
>> -            now = jiffies;
>> -            if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
>> -                stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
>> -            else
>> -                stored_lft = 0;
>> -            if (!update_lft && !create && stored_lft) {
>> -                const u32 minimum_lft = min_t(u32,
>> -                    stored_lft, MIN_VALID_LIFETIME);
>> -                valid_lft = max(valid_lft, minimum_lft);
>> -
>> -                /* RFC4862 Section 5.5.3e:
>> -                 * "Note that the preferred lifetime of the
>> -                 *  corresponding address is always reset to
>> -                 *  the Preferred Lifetime in the received
>> -                 *  Prefix Information option, regardless of
>> -                 *  whether the valid lifetime is also reset or
>> -                 *  ignored."
>> -                 *
>> -                 * So we should always update prefered_lft here.
>> -                 */
>> -                update_lft = 1;
>> -            }
>> -
>> -            if (update_lft) {
>> -                ifp->valid_lft = valid_lft;
>> -                ifp->prefered_lft = prefered_lft;
>> -                ifp->tstamp = now;
>> -                flags = ifp->flags;
>> -                ifp->flags &= ~IFA_F_DEPRECATED;
>> -                spin_unlock_bh(&ifp->lock);
>> -
>> -                if (!(flags&IFA_F_TENTATIVE))
>> -                    ipv6_ifa_notify(0, ifp);
>> -            } else
>> -                spin_unlock_bh(&ifp->lock);
>> -
>> -            manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
>> -                     create, now);
>> -
>> -            in6_ifa_put(ifp);
>> -            addrconf_verify();
>> -        }
>> +        addrconf_prefix_rcv_add_addr(net, dev, pinfo, in6_dev, &addr,
>> +                         addr_type, addr_flags, sllao,
>> +                         tokenized, valid_lft,
>> +                         prefered_lft);

check error here and goto in6_dev_put stuff. Oderwise
inet6_prefix_notify will call on error.

>>       }
>>       inet6_prefix_notify(RTM_NEWPREFIX, in6_dev, pinfo);
>>       in6_dev_put(in6_dev);
> 
> Reviewed-by: Stefan Schmidt<stefan@osg.samsung.com>
> 

I hope this fits to your review by tag, I simple add it then in RFCv2.

- Alex
Stefan Schmidt May 27, 2016, 1:17 p.m. UTC | #3
Hello.

On 27/05/16 13:41, Alexander Aring wrote:
> Hi,
>
> On 05/27/2016 11:45 AM, Stefan Schmidt wrote:
>> Hello.
>>
> sorry, again I will change something on this patch which is buggy.
>
>> On 23/05/16 21:22, Alexander Aring wrote:
>>> This patch moves the functionality to add a RA PIO prefix generated
>>> address in an own function. This move prepares to add a hook for
>>> adding a second address for a second link-layer address. E.g. short
>>> address for 802.15.4 6LoWPAN.
>>>
>>> Cc: David S. Miller<davem@davemloft.net>
>>> Cc: Alexey Kuznetsov<kuznet@ms2.inr.ac.ru>
>>> Cc: James Morris<jmorris@namei.org>
>>> Cc: Hideaki YOSHIFUJI<yoshfuji@linux-ipv6.org>
>>> Cc: Patrick McHardy<kaber@trash.net>
>>> Signed-off-by: Alexander Aring<aar@pengutronix.de>
>>> ---
>>>    net/ipv6/addrconf.c | 191 ++++++++++++++++++++++++++++------------------------
>>>    1 file changed, 102 insertions(+), 89 deletions(-)
>>>
>>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>>> index beaad49..393cdbf 100644
>>> --- a/net/ipv6/addrconf.c
>>> +++ b/net/ipv6/addrconf.c
>>> @@ -2333,6 +2333,104 @@ static bool is_addr_mode_generate_stable(struct inet6_dev *idev)
>>>               idev->addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
>>>    }
>>>    +static void addrconf_prefix_rcv_add_addr(struct net *net,
> will change to return int here to indicates errors.
>
>>> +                     struct net_device *dev,
>>> +                     const struct prefix_info *pinfo,
>>> +                     struct inet6_dev *in6_dev,
>>> +                     const struct in6_addr *addr,
>>> +                     int addr_type, u32 addr_flags,
>>> +                     bool sllao, bool tokenized,
>>> +                     __u32 valid_lft, u32 prefered_lft)
>>> +{
>>> +    struct inet6_ifaddr *ifp = ipv6_get_ifaddr(net, addr, dev, 1);
>>> +    int create = 0, update_lft = 0;
>>> +
>>> +    if (!ifp && valid_lft) {
>>> +        int max_addresses = in6_dev->cnf.max_addresses;
>>> +
>>> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>>> +        if (in6_dev->cnf.optimistic_dad &&
>>> +            !net->ipv6.devconf_all->forwarding && sllao)
>>> +            addr_flags |= IFA_F_OPTIMISTIC;
>>> +#endif
>>> +
>>> +        /* Do not allow to create too much of autoconfigured
>>> +         * addresses; this would be too easy way to crash kernel.
>>> +         */
>>> +        if (!max_addresses ||
>>> +            ipv6_count_addresses(in6_dev) < max_addresses)
>>> +            ifp = ipv6_add_addr(in6_dev, addr, NULL,
>>> +                        pinfo->prefix_len,
>>> +                        addr_type&IPV6_ADDR_SCOPE_MASK,
>>> +                        addr_flags, valid_lft,
>>> +                        prefered_lft);
>>> +
>>> +        if (IS_ERR_OR_NULL(ifp)) {
>>> +            in6_dev_put(in6_dev);
>>> +            return;
> return -1; and remove in6_dev_put stuff.
>
>>> +        }
>>> +
>>> +        update_lft = 0;
>>> +        create = 1;
>>> +        spin_lock_bh(&ifp->lock);
>>> +        ifp->flags |= IFA_F_MANAGETEMPADDR;
>>> +        ifp->cstamp = jiffies;
>>> +        ifp->tokenized = tokenized;
>>> +        spin_unlock_bh(&ifp->lock);
>>> +        addrconf_dad_start(ifp);
>>> +    }
>>> +
>>> +    if (ifp) {
>>> +        u32 flags;
>>> +        unsigned long now;
>>> +        u32 stored_lft;
>>> +
>>> +        /* update lifetime (RFC2462 5.5.3 e) */
>>> +        spin_lock_bh(&ifp->lock);
>>> +        now = jiffies;
>>> +        if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
>>> +            stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
>>> +        else
>>> +            stored_lft = 0;
>>> +        if (!update_lft && !create && stored_lft) {
>>> +            const u32 minimum_lft = min_t(u32,
>>> +                stored_lft, MIN_VALID_LIFETIME);
>>> +            valid_lft = max(valid_lft, minimum_lft);
>>> +
>>> +            /* RFC4862 Section 5.5.3e:
>>> +             * "Note that the preferred lifetime of the
>>> +             *  corresponding address is always reset to
>>> +             *  the Preferred Lifetime in the received
>>> +             *  Prefix Information option, regardless of
>>> +             *  whether the valid lifetime is also reset or
>>> +             *  ignored."
>>> +             *
>>> +             * So we should always update prefered_lft here.
>>> +             */
>>> +            update_lft = 1;
>>> +        }
>>> +
>>> +        if (update_lft) {
>>> +            ifp->valid_lft = valid_lft;
>>> +            ifp->prefered_lft = prefered_lft;
>>> +            ifp->tstamp = now;
>>> +            flags = ifp->flags;
>>> +            ifp->flags &= ~IFA_F_DEPRECATED;
>>> +            spin_unlock_bh(&ifp->lock);
>>> +
>>> +            if (!(flags&IFA_F_TENTATIVE))
>>> +                ipv6_ifa_notify(0, ifp);
>>> +        } else
>>> +            spin_unlock_bh(&ifp->lock);
>>> +
>>> +        manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
>>> +                 create, now);
>>> +
>>> +        in6_ifa_put(ifp);
>>> +        addrconf_verify();
>>> +    }
>>> +}
>>> +
>>>    void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>>>    {
>>>        struct prefix_info *pinfo;
>>> @@ -2432,9 +2530,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>>>        /* Try to figure out our local address for this prefix */
>>>          if (pinfo->autoconf && in6_dev->cnf.autoconf) {
>>> -        struct inet6_ifaddr *ifp;
>>>            struct in6_addr addr;
>>> -        int create = 0, update_lft = 0;
>>>            bool tokenized = false;
>>>              if (pinfo->prefix_len == 64) {
>>> @@ -2464,93 +2560,10 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>>>            return;
>>>      ok:
>>> -
>>> -        ifp = ipv6_get_ifaddr(net, &addr, dev, 1);
>>> -
>>> -        if (!ifp && valid_lft) {
>>> -            int max_addresses = in6_dev->cnf.max_addresses;
>>> -
>>> -#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>>> -            if (in6_dev->cnf.optimistic_dad &&
>>> -                !net->ipv6.devconf_all->forwarding && sllao)
>>> -                addr_flags |= IFA_F_OPTIMISTIC;
>>> -#endif
>>> -
>>> -            /* Do not allow to create too much of autoconfigured
>>> -             * addresses; this would be too easy way to crash kernel.
>>> -             */
>>> -            if (!max_addresses ||
>>> -                ipv6_count_addresses(in6_dev) < max_addresses)
>>> -                ifp = ipv6_add_addr(in6_dev, &addr, NULL,
>>> -                            pinfo->prefix_len,
>>> -                            addr_type&IPV6_ADDR_SCOPE_MASK,
>>> -                            addr_flags, valid_lft,
>>> -                            prefered_lft);
>>> -
>>> -            if (IS_ERR_OR_NULL(ifp)) {
>>> -                in6_dev_put(in6_dev);
>>> -                return;
>>> -            }
>>> -
>>> -            update_lft = 0;
>>> -            create = 1;
>>> -            spin_lock_bh(&ifp->lock);
>>> -            ifp->flags |= IFA_F_MANAGETEMPADDR;
>>> -            ifp->cstamp = jiffies;
>>> -            ifp->tokenized = tokenized;
>>> -            spin_unlock_bh(&ifp->lock);
>>> -            addrconf_dad_start(ifp);
>>> -        }
>>> -
>>> -        if (ifp) {
>>> -            u32 flags;
>>> -            unsigned long now;
>>> -            u32 stored_lft;
>>> -
>>> -            /* update lifetime (RFC2462 5.5.3 e) */
>>> -            spin_lock_bh(&ifp->lock);
>>> -            now = jiffies;
>>> -            if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
>>> -                stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
>>> -            else
>>> -                stored_lft = 0;
>>> -            if (!update_lft && !create && stored_lft) {
>>> -                const u32 minimum_lft = min_t(u32,
>>> -                    stored_lft, MIN_VALID_LIFETIME);
>>> -                valid_lft = max(valid_lft, minimum_lft);
>>> -
>>> -                /* RFC4862 Section 5.5.3e:
>>> -                 * "Note that the preferred lifetime of the
>>> -                 *  corresponding address is always reset to
>>> -                 *  the Preferred Lifetime in the received
>>> -                 *  Prefix Information option, regardless of
>>> -                 *  whether the valid lifetime is also reset or
>>> -                 *  ignored."
>>> -                 *
>>> -                 * So we should always update prefered_lft here.
>>> -                 */
>>> -                update_lft = 1;
>>> -            }
>>> -
>>> -            if (update_lft) {
>>> -                ifp->valid_lft = valid_lft;
>>> -                ifp->prefered_lft = prefered_lft;
>>> -                ifp->tstamp = now;
>>> -                flags = ifp->flags;
>>> -                ifp->flags &= ~IFA_F_DEPRECATED;
>>> -                spin_unlock_bh(&ifp->lock);
>>> -
>>> -                if (!(flags&IFA_F_TENTATIVE))
>>> -                    ipv6_ifa_notify(0, ifp);
>>> -            } else
>>> -                spin_unlock_bh(&ifp->lock);
>>> -
>>> -            manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
>>> -                     create, now);
>>> -
>>> -            in6_ifa_put(ifp);
>>> -            addrconf_verify();
>>> -        }
>>> +        addrconf_prefix_rcv_add_addr(net, dev, pinfo, in6_dev, &addr,
>>> +                         addr_type, addr_flags, sllao,
>>> +                         tokenized, valid_lft,
>>> +                         prefered_lft);
> check error here and goto in6_dev_put stuff. Oderwise
> inet6_prefix_notify will call on error.
>
>>>        }
>>>        inet6_prefix_notify(RTM_NEWPREFIX, in6_dev, pinfo);
>>>        in6_dev_put(in6_dev);
>> Reviewed-by: Stefan Schmidt<stefan@osg.samsung.com>
>>
> I hope this fits to your review by tag, I simple add it then in RFCv2.

You can do the change you described here and still keep my reviewed by.

regards
Stefan Schmidt
diff mbox

Patch

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index beaad49..393cdbf 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2333,6 +2333,104 @@  static bool is_addr_mode_generate_stable(struct inet6_dev *idev)
 	       idev->addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
 }
 
+static void addrconf_prefix_rcv_add_addr(struct net *net,
+					 struct net_device *dev,
+					 const struct prefix_info *pinfo,
+					 struct inet6_dev *in6_dev,
+					 const struct in6_addr *addr,
+					 int addr_type, u32 addr_flags,
+					 bool sllao, bool tokenized,
+					 __u32 valid_lft, u32 prefered_lft)
+{
+	struct inet6_ifaddr *ifp = ipv6_get_ifaddr(net, addr, dev, 1);
+	int create = 0, update_lft = 0;
+
+	if (!ifp && valid_lft) {
+		int max_addresses = in6_dev->cnf.max_addresses;
+
+#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
+		if (in6_dev->cnf.optimistic_dad &&
+		    !net->ipv6.devconf_all->forwarding && sllao)
+			addr_flags |= IFA_F_OPTIMISTIC;
+#endif
+
+		/* Do not allow to create too much of autoconfigured
+		 * addresses; this would be too easy way to crash kernel.
+		 */
+		if (!max_addresses ||
+		    ipv6_count_addresses(in6_dev) < max_addresses)
+			ifp = ipv6_add_addr(in6_dev, addr, NULL,
+					    pinfo->prefix_len,
+					    addr_type&IPV6_ADDR_SCOPE_MASK,
+					    addr_flags, valid_lft,
+					    prefered_lft);
+
+		if (IS_ERR_OR_NULL(ifp)) {
+			in6_dev_put(in6_dev);
+			return;
+		}
+
+		update_lft = 0;
+		create = 1;
+		spin_lock_bh(&ifp->lock);
+		ifp->flags |= IFA_F_MANAGETEMPADDR;
+		ifp->cstamp = jiffies;
+		ifp->tokenized = tokenized;
+		spin_unlock_bh(&ifp->lock);
+		addrconf_dad_start(ifp);
+	}
+
+	if (ifp) {
+		u32 flags;
+		unsigned long now;
+		u32 stored_lft;
+
+		/* update lifetime (RFC2462 5.5.3 e) */
+		spin_lock_bh(&ifp->lock);
+		now = jiffies;
+		if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
+			stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
+		else
+			stored_lft = 0;
+		if (!update_lft && !create && stored_lft) {
+			const u32 minimum_lft = min_t(u32,
+				stored_lft, MIN_VALID_LIFETIME);
+			valid_lft = max(valid_lft, minimum_lft);
+
+			/* RFC4862 Section 5.5.3e:
+			 * "Note that the preferred lifetime of the
+			 *  corresponding address is always reset to
+			 *  the Preferred Lifetime in the received
+			 *  Prefix Information option, regardless of
+			 *  whether the valid lifetime is also reset or
+			 *  ignored."
+			 *
+			 * So we should always update prefered_lft here.
+			 */
+			update_lft = 1;
+		}
+
+		if (update_lft) {
+			ifp->valid_lft = valid_lft;
+			ifp->prefered_lft = prefered_lft;
+			ifp->tstamp = now;
+			flags = ifp->flags;
+			ifp->flags &= ~IFA_F_DEPRECATED;
+			spin_unlock_bh(&ifp->lock);
+
+			if (!(flags&IFA_F_TENTATIVE))
+				ipv6_ifa_notify(0, ifp);
+		} else
+			spin_unlock_bh(&ifp->lock);
+
+		manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
+				 create, now);
+
+		in6_ifa_put(ifp);
+		addrconf_verify();
+	}
+}
+
 void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 {
 	struct prefix_info *pinfo;
@@ -2432,9 +2530,7 @@  void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 	/* Try to figure out our local address for this prefix */
 
 	if (pinfo->autoconf && in6_dev->cnf.autoconf) {
-		struct inet6_ifaddr *ifp;
 		struct in6_addr addr;
-		int create = 0, update_lft = 0;
 		bool tokenized = false;
 
 		if (pinfo->prefix_len == 64) {
@@ -2464,93 +2560,10 @@  void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 		return;
 
 ok:
-
-		ifp = ipv6_get_ifaddr(net, &addr, dev, 1);
-
-		if (!ifp && valid_lft) {
-			int max_addresses = in6_dev->cnf.max_addresses;
-
-#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
-			if (in6_dev->cnf.optimistic_dad &&
-			    !net->ipv6.devconf_all->forwarding && sllao)
-				addr_flags |= IFA_F_OPTIMISTIC;
-#endif
-
-			/* Do not allow to create too much of autoconfigured
-			 * addresses; this would be too easy way to crash kernel.
-			 */
-			if (!max_addresses ||
-			    ipv6_count_addresses(in6_dev) < max_addresses)
-				ifp = ipv6_add_addr(in6_dev, &addr, NULL,
-						    pinfo->prefix_len,
-						    addr_type&IPV6_ADDR_SCOPE_MASK,
-						    addr_flags, valid_lft,
-						    prefered_lft);
-
-			if (IS_ERR_OR_NULL(ifp)) {
-				in6_dev_put(in6_dev);
-				return;
-			}
-
-			update_lft = 0;
-			create = 1;
-			spin_lock_bh(&ifp->lock);
-			ifp->flags |= IFA_F_MANAGETEMPADDR;
-			ifp->cstamp = jiffies;
-			ifp->tokenized = tokenized;
-			spin_unlock_bh(&ifp->lock);
-			addrconf_dad_start(ifp);
-		}
-
-		if (ifp) {
-			u32 flags;
-			unsigned long now;
-			u32 stored_lft;
-
-			/* update lifetime (RFC2462 5.5.3 e) */
-			spin_lock_bh(&ifp->lock);
-			now = jiffies;
-			if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
-				stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
-			else
-				stored_lft = 0;
-			if (!update_lft && !create && stored_lft) {
-				const u32 minimum_lft = min_t(u32,
-					stored_lft, MIN_VALID_LIFETIME);
-				valid_lft = max(valid_lft, minimum_lft);
-
-				/* RFC4862 Section 5.5.3e:
-				 * "Note that the preferred lifetime of the
-				 *  corresponding address is always reset to
-				 *  the Preferred Lifetime in the received
-				 *  Prefix Information option, regardless of
-				 *  whether the valid lifetime is also reset or
-				 *  ignored."
-				 *
-				 * So we should always update prefered_lft here.
-				 */
-				update_lft = 1;
-			}
-
-			if (update_lft) {
-				ifp->valid_lft = valid_lft;
-				ifp->prefered_lft = prefered_lft;
-				ifp->tstamp = now;
-				flags = ifp->flags;
-				ifp->flags &= ~IFA_F_DEPRECATED;
-				spin_unlock_bh(&ifp->lock);
-
-				if (!(flags&IFA_F_TENTATIVE))
-					ipv6_ifa_notify(0, ifp);
-			} else
-				spin_unlock_bh(&ifp->lock);
-
-			manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
-					 create, now);
-
-			in6_ifa_put(ifp);
-			addrconf_verify();
-		}
+		addrconf_prefix_rcv_add_addr(net, dev, pinfo, in6_dev, &addr,
+					     addr_type, addr_flags, sllao,
+					     tokenized, valid_lft,
+					     prefered_lft);
 	}
 	inet6_prefix_notify(RTM_NEWPREFIX, in6_dev, pinfo);
 	in6_dev_put(in6_dev);