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@redhat.com>
---
 src/util/virnetdevip.c | 108 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 106 insertions(+), 2 deletions(-)

diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index 7bd5a75f85..93f47f22d9 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -43,8 +43,11 @@
 #ifdef __linux__
 # include <linux/sockios.h>
 # include <linux/if_vlan.h>
+# include <linux/ipv6_route.h>
 #endif
 
+#define PROC_NET_IPV6_ROUTE "/proc/net/ipv6_route"
+
 #define VIR_FROM_THIS VIR_FROM_NONE
 
 VIR_LOG_INIT("util.netdevip");
@@ -688,15 +691,116 @@ virNetDevIPRouteAdd(const char *ifname,
     return 0;
 }
 
+#endif /* defined(__linux__) && defined(HAVE_LIBNL) */
+
+
+#if defined(__linux__)
+
+/**
+ * virNetDevIPCheckIPv6Forwarding
+ *
+ * This function checks if IPv6 routes have the RTF_ADDRCONF flag set,
+ * indicating they have been created by the kernel's RA configuration
+ * handling.  These routes are subject to being flushed when ipv6
+ * forwarding is enabled unless accept_ra is explicitly set to "2".
+ * This will most likely result in ipv6 networking being broken.
+ *
+ * Returns: true if it is safe to enable forwarding, or false if
+ * breakable routes are found.
+ *
+ **/
+bool
+virNetDevIPCheckIPv6Forwarding(void)
+{
+    int len;
+    char *cur;
+    g_autofree char *buf = NULL;
+    /* lines are 150 chars */
+    enum {MAX_ROUTE_SIZE = 150*100000};
+
+    /* This is /proc/sys/net/ipv6/conf/all/accept_ra */
+    int all_accept_ra = virNetDevIPGetAcceptRA(NULL);
+
+    /* Read ipv6 routes */
+    if ((len = virFileReadAll(PROC_NET_IPV6_ROUTE,
+                              MAX_ROUTE_SIZE, &buf)) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unable to read %s "
+                         "for ipv6 forwarding checks"), PROC_NET_IPV6_ROUTE);
+        return false;
+    }
+
+    /* VIR_DEBUG("%s output:\n%s", PROC_NET_IPV6_ROUTE, buf); */
+
+    /* Dropping the last character to stop the loop */
+    if (len > 0)
+        buf[len-1] = '\0';
+
+    cur = buf;
+    while (cur) {
+        char route[33], flags[9], iface[9];
+        unsigned int flags_val;
+        char *iface_val;
+        int num;
+        char *nl = strchr(cur, '\n');
+
+        if (nl)
+            *nl++ = '\0';
+
+        num = sscanf(cur, "%32s %*s %*s %*s %*s %*s %*s %*s %8s %8s",
+                     route, flags, iface);
+
+        cur = nl;
+        if (num != 3) {
+            VIR_DEBUG("Failed to parse route line: %s", cur);
+            continue;
+        }
+
+        if (virStrToLong_ui(flags, NULL, 16, &flags_val)) {
+            VIR_DEBUG("Failed to parse flags: %s", flags);
+            continue;
+        }
+
+        /* This is right justified, strip leading spaces */
+        iface_val = &iface[0];
+        while (*iface_val && g_ascii_isspace(*iface_val))
+            iface_val++;
+
+        VIR_DEBUG("%s iface %s flags %s : RTF_ADDRCONF %sset",
+                  route, iface_val, flags,
+                  (flags_val & RTF_ADDRCONF ? "" : "not "));
+
+        if (flags_val & RTF_ADDRCONF) {
+            int ret = virNetDevIPGetAcceptRA(iface_val);
+            VIR_DEBUG("%s reports accept_ra of %d",
+                      iface_val, ret);
+            /* If the interface for this autoconfigured route
+             * has accept_ra == 1, or it is default and the "all"
+             * value of accept_ra == 1, it will be subject to
+             * flushing if forwarding is enabled.
+             */
+            if (ret == 1 || (ret == 0 && all_accept_ra == 1)) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Check the host setup: interface %s has kernel "
+                                 "autoconfigured IPv6 routes and enabling forwarding "
+                                 " without accept_ra set to 2 will cause the kernel "
+                                 "to flush them, breaking networking."), iface_val);
+                return false;
+            }
+        }
+    }
+    return true;
+}
+#else
 
 bool
 virNetDevIPCheckIPv6Forwarding(void)
 {
-    VIR_WARN("built without libnl: unable to check if IPv6 forwarding can be safely enabled");
+    VIR_DEBUG("No checks for IPv6 forwarding issues on non-Linux systems", flags);
     return true;
 }
 
-#endif /* defined(__linux__) && defined(WITH_LIBNL) */
+#endif /* defined(__linux__) */
 
 
 /**
-- 
2.26.2

Please ignore this version, sorry, as I forgot to commit
the removal of the old functions in the patch.

On Fri, Sep 11, 2020 at 09:30:30PM +1000, 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@redhat.com>
> ---
>  src/util/virnetdevip.c | 108 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 106 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
> index 7bd5a75f85..93f47f22d9 100644
> --- a/src/util/virnetdevip.c
> +++ b/src/util/virnetdevip.c
> @@ -43,8 +43,11 @@
>  #ifdef __linux__
>  # include <linux/sockios.h>
>  # include <linux/if_vlan.h>
> +# include <linux/ipv6_route.h>
>  #endif
>  
> +#define PROC_NET_IPV6_ROUTE "/proc/net/ipv6_route"
> +
>  #define VIR_FROM_THIS VIR_FROM_NONE
>  
>  VIR_LOG_INIT("util.netdevip");
> @@ -688,15 +691,116 @@ virNetDevIPRouteAdd(const char *ifname,
>      return 0;
>  }
>  
> +#endif /* defined(__linux__) && defined(HAVE_LIBNL) */
> +
> +
> +#if defined(__linux__)
> +
> +/**
> + * virNetDevIPCheckIPv6Forwarding
> + *
> + * This function checks if IPv6 routes have the RTF_ADDRCONF flag set,
> + * indicating they have been created by the kernel's RA configuration
> + * handling.  These routes are subject to being flushed when ipv6
> + * forwarding is enabled unless accept_ra is explicitly set to "2".
> + * This will most likely result in ipv6 networking being broken.
> + *
> + * Returns: true if it is safe to enable forwarding, or false if
> + * breakable routes are found.
> + *
> + **/
> +bool
> +virNetDevIPCheckIPv6Forwarding(void)
> +{
> +    int len;
> +    char *cur;
> +    g_autofree char *buf = NULL;
> +    /* lines are 150 chars */
> +    enum {MAX_ROUTE_SIZE = 150*100000};
> +
> +    /* This is /proc/sys/net/ipv6/conf/all/accept_ra */
> +    int all_accept_ra = virNetDevIPGetAcceptRA(NULL);
> +
> +    /* Read ipv6 routes */
> +    if ((len = virFileReadAll(PROC_NET_IPV6_ROUTE,
> +                              MAX_ROUTE_SIZE, &buf)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Unable to read %s "
> +                         "for ipv6 forwarding checks"), PROC_NET_IPV6_ROUTE);
> +        return false;
> +    }
> +
> +    /* VIR_DEBUG("%s output:\n%s", PROC_NET_IPV6_ROUTE, buf); */
> +
> +    /* Dropping the last character to stop the loop */
> +    if (len > 0)
> +        buf[len-1] = '\0';
> +
> +    cur = buf;
> +    while (cur) {
> +        char route[33], flags[9], iface[9];
> +        unsigned int flags_val;
> +        char *iface_val;
> +        int num;
> +        char *nl = strchr(cur, '\n');
> +
> +        if (nl)
> +            *nl++ = '\0';
> +
> +        num = sscanf(cur, "%32s %*s %*s %*s %*s %*s %*s %*s %8s %8s",
> +                     route, flags, iface);
> +
> +        cur = nl;
> +        if (num != 3) {
> +            VIR_DEBUG("Failed to parse route line: %s", cur);
> +            continue;
> +        }
> +
> +        if (virStrToLong_ui(flags, NULL, 16, &flags_val)) {
> +            VIR_DEBUG("Failed to parse flags: %s", flags);
> +            continue;
> +        }
> +
> +        /* This is right justified, strip leading spaces */
> +        iface_val = &iface[0];
> +        while (*iface_val && g_ascii_isspace(*iface_val))
> +            iface_val++;
> +
> +        VIR_DEBUG("%s iface %s flags %s : RTF_ADDRCONF %sset",
> +                  route, iface_val, flags,
> +                  (flags_val & RTF_ADDRCONF ? "" : "not "));
> +
> +        if (flags_val & RTF_ADDRCONF) {
> +            int ret = virNetDevIPGetAcceptRA(iface_val);
> +            VIR_DEBUG("%s reports accept_ra of %d",
> +                      iface_val, ret);
> +            /* If the interface for this autoconfigured route
> +             * has accept_ra == 1, or it is default and the "all"
> +             * value of accept_ra == 1, it will be subject to
> +             * flushing if forwarding is enabled.
> +             */
> +            if (ret == 1 || (ret == 0 && all_accept_ra == 1)) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Check the host setup: interface %s has kernel "
> +                                 "autoconfigured IPv6 routes and enabling forwarding "
> +                                 " without accept_ra set to 2 will cause the kernel "
> +                                 "to flush them, breaking networking."), iface_val);
> +                return false;
> +            }
> +        }
> +    }
> +    return true;
> +}
> +#else
>  
>  bool
>  virNetDevIPCheckIPv6Forwarding(void)
>  {
> -    VIR_WARN("built without libnl: unable to check if IPv6 forwarding can be safely enabled");
> +    VIR_DEBUG("No checks for IPv6 forwarding issues on non-Linux systems", flags);
>      return true;
>  }
>  
> -#endif /* defined(__linux__) && defined(WITH_LIBNL) */
> +#endif /* defined(__linux__) */
>  
>  
>  /**
> -- 
> 2.26.2
>