On Wed, 2020-08-12 at 19:21 -0400, Laine Stump wrote:
> Yay! A user who follows up their conversation on the libvirt-users list
> with a patch! :-)
>
> On 8/11/20 8:14 PM, Ian Wienand wrote:
> > The checks modified here were added with
> > 00d28a78b5d1f6eaf79f06ac59e31c568af9da37 to avoid losing routes on
> > hosts.
>
> I'm Cc'ing the author of that patch, Cédric Bosdonnat, to make sure that
> whatever we end up doing doesn't upset his use case.
Ouch! that is old... even reading my bugzilla log I have troubles
explaining that thing properly. I'll try it though.
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.
--
Cedric
> > However, tools such as systemd-networking and NetworkManager manage
> > RA's in userspace and thus IPv6 may be up and working on an interface
> > even with accept_ra == 0.
> >
> > This modifies the check to only error if an interface's accept_ra is
> > already set to "1"; as noted inline this seems to when it is likely
> > that enabling forwarding may change the RA acceptance behaviour of the
> > interface.
>
> Unfortunately, on my Fedora 32 machine that has NetworkManager enabled,
> not all the interfaces have accept_ra=0 by default. One of the bridge
> devices (created by NetworkManager in response to an ifcfg file in
> /etc/sysconfig/network-scripts) has accept_ra = 1. (there are several
> other interfaces that have accept_ra=1, but those interfaces are either
> not online, or they don't have an IPv6 address.
>
>
> So this doesn't work for all cases. I think we need to get more
> information on how to most easily, generically, and reliably determine
> if the kernel accept_ra setting can be ignored. Possibly the
> NetworkManager people can help us out here.
>
>
> (or alternately we could punt and just make a configuration setting,
> although that is taking the easy road, and not a good precedent to set)
>
>
> > I have noticed this because I am using the IPv6 NAT features enabled
> > with 927acaedec7effbe67a154d8bfa0e67f7d08e6c7. I am using this on my
> > laptop which switches between wired and wireless connections; both of
> > which are configured in an unremarkable way by Fedora's NetworkManager
> > and get configured for IPv6 via SLAAC and whatever NetworkManager
> > magic it does. With this I can define and start a libvirt network
> > with <nat ipv6="yes"> and <ip family='ipv6'
> > address='fc00:abcd:ef12:34::' prefix='64'> and it seems to "just work"
> > for guests.
> >
> > Signed-off-by: Ian Wienand <iwienand@xxxxxxxxxx>
> > ---
> > src/util/virnetdevip.c | 41 +++++++++++++++++++++++++++--------------
> > 1 file changed, 27 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
> > index 409f062c5c..de27cacfc9 100644
> > --- a/src/util/virnetdevip.c
> > +++ b/src/util/virnetdevip.c
> > @@ -496,7 +496,7 @@ virNetDevIPGetAcceptRA(const char *ifname)
> > }
> >
> > struct virNetDevIPCheckIPv6ForwardingData {
> > - bool hasRARoutes;
> > + bool hasKernelRARoutes;
> >
> > /* Devices with conflicting accept_ra */
> > char **devices;
> > @@ -552,15 +552,26 @@ virNetDevIPCheckIPv6ForwardingCallback(struct nlmsghdr *resp,
> > if (!ifname)
> > return -1;
> >
> > - accept_ra = virNetDevIPGetAcceptRA(ifname);
> > -
> > VIR_DEBUG("Checking route for device %s (%d), accept_ra: %d",
> > ifname, ifindex, accept_ra);
> >
> > - if (accept_ra != 2 && virNetDevIPCheckIPv6ForwardingAddIF(data, &ifname) < 0)
> > + accept_ra = virNetDevIPGetAcceptRA(ifname);
> > + /* 0 = do no accept RA
> > + * 1 = accept if forwarding disabled
> > + * 2 = ovveride and accept RA when forwarding enabled
> > + *
> > + * When RA is managed by userspace (systemd-networkd or
> > + * NetworkManager) accept_ra is unset and we don't need to
> > + * worry about it. If it is 1, enabling forwarding might
> > + * change the behaviour so the user needs to be warned.
> > + */
> > + if (accept_ra == 0)
> > + return 0;
> > +
> > + if (accept_ra == 1 && virNetDevIPCheckIPv6ForwardingAddIF(data, &ifname) < 0)
> > return -1;
> >
> > - data->hasRARoutes = true;
> > + data->hasKernelRARoutes = true;
> > return 0;
> > }
> >
> > @@ -590,11 +601,13 @@ virNetDevIPCheckIPv6ForwardingCallback(struct nlmsghdr *resp,
> > VIR_DEBUG("Checking multipath route nexthop device %s (%d), accept_ra: %d",
> > ifname, nh->rtnh_ifindex, accept_ra);
> >
> > - if (accept_ra != 2 && virNetDevIPCheckIPv6ForwardingAddIF(data, &ifname) < 0)
> > - return -1;
> > + if (accept_ra == 1) {
> > + if (virNetDevIPCheckIPv6ForwardingAddIF(data, &ifname) < 0)
> > + return -1;
> > + data->hasKernelRARoutes = true;
> > + }
> >
> > VIR_FREE(ifname);
> > - data->hasRARoutes = true;
> >
> > len -= NLMSG_ALIGN(nh->rtnh_len);
> > VIR_WARNINGS_NO_CAST_ALIGN
> > @@ -613,7 +626,7 @@ virNetDevIPCheckIPv6Forwarding(void)
> > struct rtgenmsg genmsg;
> > size_t i;
> > struct virNetDevIPCheckIPv6ForwardingData data = {
> > - .hasRARoutes = false,
> > + .hasKernelRARoutes = false,
> > .devices = NULL,
> > .ndevices = 0
> > };
> > @@ -644,11 +657,11 @@ virNetDevIPCheckIPv6Forwarding(void)
> > goto cleanup;
> > }
> >
> > - valid = !data.hasRARoutes || data.ndevices == 0;
> > + valid = !data.hasKernelRARoutes || data.ndevices == 0;
> >
> > /* Check the global accept_ra if at least one isn't set on a
> > per-device basis */
> > - if (!valid && data.hasRARoutes) {
> > + if (!valid && data.hasKernelRARoutes) {
> > int accept_ra = virNetDevIPGetAcceptRA(NULL);
> > valid = accept_ra == 2;
> > VIR_DEBUG("Checked global accept_ra: %d", accept_ra);
> > @@ -663,9 +676,9 @@ virNetDevIPCheckIPv6Forwarding(void)
> > }
> >
> > virReportError(VIR_ERR_INTERNAL_ERROR,
> > - _("Check the host setup: enabling IPv6 forwarding with "
> > - "RA routes without accept_ra set to 2 is likely to cause "
> > - "routes loss. Interfaces to look at: %s"),
> > + _("Check the host setup: interface has accept_ra set to 1 "
> > + "and enabling forwarding without accept_ra set to 2 is "
> > + "likely to cause routes loss. Interfaces to look at: %s"),
> > virBufferCurrentContent(&buf));
> > }
> >
>
>
