[PATCH] network: allow accept_ra == 0 when enabling ipv6 forwarding

[Laine Stump]

On 9/9/20 2:38 AM, Cedric Bosdonnat wrote:
> On Wed, 2020-09-09 at 12:39 +1000, Ian Wienand wrote:
>> On Tue, Sep 01, 2020 at 08:27:47AM +0000, Cedric Bosdonnat wrote:
>>> So the hypervisor has at least one (Router Advertised) RA route.
>>> After defining a network like the following, the RA route is removed if
>>> accept_ra isn't set to 2.
>>>
>>> <network ipv6="yes">
>>>    <name>test5</name>
>>>    <forward mode="nat"/>
>>>    <bridge name="708837c1d27-br0" stp="off"/>
>>>    <mac address="52:54:00:45:5f:27"/>
>>>    <ip
>>>       family="ipv6"
>>>       address="fc00:0000:0000:000f:0000:0000:0000:0001"
>>>       prefix="64"/>
>>> </network>
>>>
>>> The RA route was removed in networkEnableIPForwarding() when
>>> setting /proc/sys/net/ipv6/conf/all/forwarding to 1.
>>>
>>> Me not being a network expert (and even less on ipv6) doesn't help.
>>>
>>> I hope this explanation will help you better see the use case I had.
>> So it seems to be the intention of the kernel that when you enable
>> forwarding your routes are flushed; changing the sysctl gets into
>> addrconf_fixup_forwarding() [1] which then calls
>> rt6_purge_dflt_routers() when the forwarding status is changed.  That
>> then purges default routes, unless accept_ra == 2; that was introduced
>> with [3].
>>
>> I guess the idea is that a router should not accept
>> auto-configuration?
>>
>> HOWEVER ...
>>
>>                 if (rt->fib6_flags & (RTF_DEFAULT | RTF_ADDRCONF) &&
>>                      (!idev || idev->cnf.accept_ra != 2) &&
>>                      fib6_info_hold_safe(rt)) {
>>                          rcu_read_unlock();
>>                          ip6_del_rt(net, rt);
>>                          goto restart;
>>                  }
>>
>> I feel like this is checking the RTF_ADDRCONF flag before it flushes
>> any routes.  Checking that flag ...
>>
>>   #define RTF_ADDRCONF    0x00040000
>>
>> which I do not have set at all, from :
>>
>>   $ cat /proc/net/ipv6_route | awk  '{print $1 " " and(strtonum("0x"$9),strtonum("0x40000"))}'
>>
>> Based on this, I'm concluding that the userspace tools do not set this
>> flag on their routes, and so they are never flushed.  Empirically,
>> fiddling forwarding on and off I don't see any routes flushed.
>>
>> So, I do not think that enabling forwarding will remove routes on the
>> most common "sitting in-front of the computer" cases where you're
>> using NetworkManager/systemd userspace magic.
>>
>> Given this, I'd propose we revert the check?
> The check didn't involve any NetworkManager at all,


Right. I think we've established that any system using systemd-networkd 
or NetworkManager doesn't care what is the setting of accept_ra in the 
kernel. But we can't just leave users of more traditional network 
management systems (which leave RA handling to the kernel) out in the 
cold to fend for themselves. I mean, the RA code is there in the kernel 
for a reason; if it's really not necessary or used any more, then remove 
it (yes, that is a *joke* - I know you can't remove an API from the 
kernel). As long as it's there, we need to assume that some people may 
use it, and act accordingly.


Does your detailed spelunking of the kernel (nicely detailed above) 
maybe lead to some more reliable method of recognizing that we don't 
need the check (it kind of *sounds* like it does, but I'm unable to 
concentrate about it long enough to come up with a guaranteed answer)



>   but a network with
> RA route for the default route. Completely removing the check is rather
> likely to introduce a regression on that side.