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

The checks modified here were added with 00d28a78b5d1f6eaf79f06ac59e31c568af9da37 to avoid losing routes on hosts.

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.

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(a)redhat.com&gt; --- 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)); }