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

[Ian Wienand]

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 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));
     }
 
-- 
2.26.2