[PATCH] network: allow accept_ra == 0 when enabling ipv6 forwarding
Laine Stump
laine at redhat.com
Wed Aug 12 23:21:14 UTC 2020
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.
>
> 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 at redhat.com>
> ---
> 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));
> }
>
More information about the libvir-list
mailing list