Re: [PATCH net-next v2] ipv6: Do not iterate over all interfaces when finding source address on specific interface.

[YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>]

Hi,

Hajime Tazaki wrote:
> 
> Yoshifuji-san,
> 
> At Mon, 13 Jul 2015 17:38:48 +0900,
> Erik Kline wrote:
>>
>> On 13 July 2015 at 15:32, YOSHIFUJI Hideaki
>> <hideaki.yoshifuji@miraclelinux.com> wrote:
>>> Hi,
>>>
>>> Erik Kline wrote:
>>>> Hmm, when I run a UML linux with this patch (which, I'm ashamed to
>>>> say, I failed to do before) I get these kinds of errors:
>>>>
>>>>     unregister_netdevice: waiting for <TAPdevice> to become free.
>>>> Usage count = 1
>>>>     unregister_netdevice: waiting for <TAPdevice> to become free.
>>>> Usage count = 1
>>>>
>>>> Perhaps they're unrelated... I'm still investigating.
>>>
>>> Would you test attached patch please?
>>
>> That does look logically correct, so +1 to it regardless, but it does
>> not seem to have fixed the issue I'm seeing.
>>
>> I still haven't produced the smallest possible demo test program.
> 
> sorry to jump-in, but there is a side-effect with this
> patch, which my tcp and dccp tests (ipv6) are failed.
> 
> because newly added function (__ipv6_dev_get_saddr) won't
> update a variable 'hiscore' (it swaps with 'score' in some
> case), the caller (ipv6_dev_get_saddr) can't fill an
> appropriate saddr in the end.
> 
> I don't know if this is a good patch but the following diff
> makes my test happy.

We should update score as well...

> 
> -- Hajime
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 4ab74d5..c4e9416 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -1363,7 +1363,8 @@ static void __ipv6_dev_get_saddr(struct net *net,
>  				 unsigned int prefs,
>  				 const struct in6_addr *saddr,
>  				 struct inet6_dev *idev,
> -				 struct ipv6_saddr_score *scores)
> +				 struct ipv6_saddr_score *scores,
> +				 struct ipv6_saddr_score **in_hiscore)
>  {
>  	struct ipv6_saddr_score *score = &scores[0], *hiscore = &scores[1];
>  
> @@ -1424,6 +1425,7 @@ static void __ipv6_dev_get_saddr(struct net *net,
>  				in6_ifa_hold(score->ifa);
>  
>  				swap(hiscore, score);
> +				*in_hiscore = hiscore;
>  
>  				/* restore our iterator */
>  				score->ifa = hiscore->ifa;
> @@ -1480,13 +1482,15 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
>  	}
>  
>  	if (use_oif_addr) {
> -		__ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
> +		__ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev,
> +				     scores, &hiscore);
>  	} else {
>  		for_each_netdev_rcu(net, dev) {
>  			idev = __in6_dev_get(dev);
>  			if (!idev)
>  				continue;
> -			__ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
> +			__ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev,
> +					     scores, &hiscore);
>  		}
>  	}
>  	rcu_read_unlock();
> 

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION