Re: [PATCH v2 2/2] ipv6: Allow accepting RA from local IP addresses.

["YOSHIFUJI Hideaki/吉藤英明" <hideaki.yoshifuji@miraclelinux.com>]

Hi,

2014/06/25 12:19, Ben Greear wrote:>
>
> On 06/24/2014 03:22 PM, YOSHIFUJI Hideaki wrote:
>> Hello.
>>
>> (2014/06/25 6:14), greearb@candelatech.com wrote:
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> This can be used in virtual networking applications, and
>>> may have other uses as well.  The option is disabled by
>>> default, so no change to current operating behaviour
>>
>>                                     standard compliant behavior?
>
> I've no idea.  Can you point me to the proper standard (and
> pertinent section)?

I was wrong.

I found this code was added by commit 9f56220 ("ipv6: Do not
use routes from locally generated RAs") to fix behavior when
accept_ra == 2.

But I do not understand why it is not enough to restrict local
address on receiving interface.

Andi, would you explain?

>
>>> without the user explicitly changing the behaviour.
>>>
>>
>> Would you include your specific example?
>
> I gave one in a response to comments on v1 of this patch.
>
> Basically, I make a single OS instance look like a bunch of
> routers, bridges, and hosts.  Without use of network namespaces,
> virtual machines, or other such virtualization.  Just clever use
> of ip rules and routes.  So, I need interfaces to be able to accept
> RA from other interfaces on the same system.
>
> http://www.spinics.net/lists/netdev/msg286764.html
>
>

>>> +static bool ipv6_accept_ra_local(struct inet6_dev *in6_dev, struct
sk_buf *skb)
>>> +{
>>> +	/* Do not accept RA with source-addr found on local machine unless
>>> +	 * accept_ra_from_local is set to true.
>>> +	 */
>>> +	if (!in6_dev->cnf.accept_ra_from_local &&
>>> +	    ipv6_chk_addr(dev_net(in6_dev->dev), &ipv6_hdr(skb)->saddr,
>>> +			  NULL, 0))
>>> +		return false;
>>> +	return true;
>>> +}
>>> +
>>> +
>>>     static void ndisc_router_discovery(struct sk_buff *skb)
>>>     {
>>>     	struct ra_msg *ra_msg = (struct ra_msg *)skb_transport_header(skb);
>>> @@ -1151,10 +1164,9 @@ static void ndisc_router_discovery(struct
sk_buff *skb)
>>>     		goto skip_defrtr;
>>>     	}
>>>
>>> -	if (ipv6_chk_addr(dev_net(in6_dev->dev), &ipv6_hdr(skb)->saddr,
>>> -			  NULL, 0)) {
>>> +	if (!ipv6_accept_ra_local(in6_dev, skb)) {
>>>     		ND_PRINTK(2, info,
>>> -			  "RA: %s, chk_addr failed for dev: %s\n",
>>> +			  "RA: %s, accept_ra_local failed for dev: %s\n",
>>>     			  __func__, skb->dev->name);
>>>     		goto skip_defrtr;
>>>     	}
>>
>> Hmm, without global knob, I see little benefit by
>> having new helper.
>
> A previous reviewer requested it.  I don't care either
> way, seems fine to open-code it to me.
>
>> At least, it should be called ipv6_chk_addr_ra(),
>> ipv6_check_ra_saddr(), ipv6_is_nonlocal_ra() or
>> something else.
>>
>> I think we do not need to change debugging output,
>> or we could say "RA from local address detected;
>> default router ignored." or something like.
>
> That does seem like a more useful error message.
>
> Thanks,
> Ben
>

--yoshfuji