[PATCH v2] network: Only check kernel added routes in virNetDevIPCheckIPv6Forwarding

[Cedric Bosdonnat]

On Sun, 2020-09-13 at 13:34 -0400, Laine Stump wrote:
> On 9/11/20 1:08 PM, Laine Stump wrote:
> > On 9/11/20 7:34 AM, Ian Wienand wrote:
> > > The original motivation for adding virNetDevIPCheckIPv6Forwarding
> > > (00d28a78b5d1f6eaf79f06ac59e31c568af9da37) was that networking routes
> > > would disappear when ipv6 forwarding was enabled for an interface.
> > > 
> > > This is a fairly undocumented side-effect of the "accept_ra" sysctl
> > > for an interface.  1 means the interface will accept_ra's if not
> > > forwarding, 2 means always accept_RAs; but it is not explained that
> > > enabling forwarding when accept_ra==1 will also clear any kernel RA
> > > assigned routes, very likely breaking your networking.
> > > 
> > > The check to warn about this currently uses netlink to go through all
> > > the routes and then look at the accept_ra status of the interfaces.
> > > 
> > > However, it has been noticed that this problem does not affect systems
> > > where IPv6 RA configuration is handled in userspace, e.g. via tools
> > > such as NetworkManager.  In this case, the error message from libvirt
> > > is spurious, and modifying the forwarding state will not affect the RA
> > > state or disable your networking.
> > > 
> > > If you refer to the function rt6_purge_dflt_routers() in the kernel,
> > > we can see that the routes being purged are only those with the
> > > kernel's RTF_ADDRCONF flag set; that is, routes added by the kernel's
> > > RA handling.  Why does it do this?  I think this is a Linux
> > > implementation decision; it has always been like that and there are
> > > some comments suggesting that it is because a router should be
> > > statically configured, rather than accepting external configurations.
> > > 
> > > The solution implemented here is to convert the existing check into a
> > > walk of /proc/net/ipv6_route and look for routes with this flag set.
> > > We then check the accept_ra status for the interface, and if enabling
> > > forwarding would break things raise an error.
> > > 
> > > This should hopefully avoid "interactive" users, who are likely to be
> > > using NetworkManager and the like, having false warnings when enabling
> > > IPv6, but retain the error check for users relying on kernel-based
> > > IPv6 interface auto-configuration.
> > > 
> > > Signed-off-by: Ian Wienand <iwienand at redhat.com>
> > > ---
> > 
> > [...]
> > 
> > 
> > Reviewed-by: Laine Stump <laine at redhat.com>
> > 
> > 
> > (pending successful completion of CI (see below) and verification that 
> > the error is triggered when appropriate. Once I've verified those two, 
> > I'll push it).
> 
> I've fixed the couple of VIR_DEBUG problems (the commented out line that 
> Cedric noticed, and the one that fails CI due to the extraneous arg). I 
> also checked on my private gitlab fork that it will pass CI when pushed.
> 
> 
> Additionally, I disabled NetworkManager on one of my systems and 
> re-enabled the old deprecation-bound network.service (which uses the 
> kernel for RA). When I tried to start an IPv6 network, I got this message:
> 
> error: Failed to start network ipv6
> error: internal error: Check the host setup: interface enp7s0 has kernel 
> autoconfigured IPv6 routes and enabling forwarding  without accept_ra 
> set to 2 will cause the kernel to flush them, breaking networking.
> 
> This is enough proof for me, so I'm pushing the patch!

Thanks Laine for your testing.

--
Cedric