[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 13/13] Run radvd for virtual networks with IPv6 addresses



Running an instance of the router advertisement daemon (radvd) allows
guests using the virtual network to automatically acquire and IPv6
address and default route. Note that acquiring an address only works
for networks with a prefix length of exactly 64 - radvd is still run
in other circumstances, and still advertises routes, but autoconf will
not work because it requires exactly 64 bits of address info from the
network prefix.

This patch should not be committed as-is, because there is a race
condition with the pidfile that I still need to remedy. Unlike
dnsmasq, radvd does not guarantee that its pidfile has been written
before its original process exits; instead it uses daemon(3) to fork
and exit the parent process in a single step, then writes the pidfile
later. The result is that libvirtd tries to read the pidfile before
it's been created, and ends up not having the proper pid to use when
it later tries to kill radvd.

There are two possible solutions for this:

1) Don't attempt to immediately read the pidfile and store the pid in
   memory. Instead, just read the pidfile later when we want to kill
   radvd. (This could still lead to a race if networkStart and
   networkDestroy were called in tight sequence by some application)

2) Don't allow radvd to daemonize itself. Instead, run it with "-d 1"
   (setting a debuglevel > 0 implies that it should not daemonize),
   and use virCommand's own pidfile/daemonize functions. (The problem
   here is that there's no way to tell radvd to not write a pidfile
   itself, so we would end up with 2 pidfiles for each instance. This
   isn't a functional problem, just cosmetic).

Another unresolved(?) problem is that the location of the radvd binary
is found by autoconf during the build, so if it's not present on the
build machine. Should this be handled by specfiles on specific distros
adding a BuildRequires for the radvd package, or can/should more be
done in the libvirt tree? (Current behavior is identical to dnsmasq,
so I guess it's acceptable, as long as all the downstream builders are
made aware of the need for radvd for proper IPv6 support).
---
 configure.ac                |    4 +
 src/conf/network_conf.h     |    1 +
 src/network/bridge_driver.c |  232 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 216 insertions(+), 21 deletions(-)

diff --git a/configure.ac b/configure.ac
index 279ccd2..231d0c7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -135,6 +135,8 @@ dnl detect them, in which case we'll search for the program
 dnl along the $PATH at runtime and fail if it's not there.
 AC_PATH_PROG([DNSMASQ], [dnsmasq], [dnsmasq],
 	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
+AC_PATH_PROG([RADVD], [radvd], [radvd],
+	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
 AC_PATH_PROG([BRCTL], [brctl], [brctl],
 	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
 AC_PATH_PROG([UDEVADM], [udevadm], [],
@@ -146,6 +148,8 @@ AC_PATH_PROG([MODPROBE], [modprobe], [],
 
 AC_DEFINE_UNQUOTED([DNSMASQ],["$DNSMASQ"],
         [Location or name of the dnsmasq program])
+AC_DEFINE_UNQUOTED([RADVD],["$RADVD"],
+        [Location or name of the radvd program])
 AC_DEFINE_UNQUOTED([BRCTL],["$BRCTL"],
         [Location or name of the brctl program (see bridge-utils)])
 if test -n "$UDEVADM"; then
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 47f0408..ca5784e 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -107,6 +107,7 @@ struct _virNetworkObj {
     virMutex lock;
 
     pid_t dnsmasqPid;
+    pid_t radvdPid;
     unsigned int active : 1;
     unsigned int autostart : 1;
     unsigned int persistent : 1;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 665a13f..37f0eed 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -65,6 +65,7 @@
 #define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
 
 #define DNSMASQ_STATE_DIR LOCALSTATEDIR "/lib/libvirt/dnsmasq"
+#define RADVD_STATE_DIR LOCALSTATEDIR "/lib/libvirt/radvd"
 
 #define VIR_FROM_THIS VIR_FROM_NETWORK
 
@@ -107,6 +108,25 @@ static void networkReloadIptablesRules(struct network_driver *driver);
 
 static struct network_driver *driverState = NULL;
 
+static char *
+networkRadvdPidfileBasename(const char *netname)
+{
+    /* this is simple but we want to be sure it's consistently done */
+    char *pidfilebase;
+
+    virAsprintf(&pidfilebase, "%s-radvd", netname);
+    return pidfilebase;
+}
+
+static char *
+networkRadvdConfigFileName(const char *netname)
+{
+    char *configfile;
+
+    virAsprintf(&configfile, "%s/%s-radvd.conf",
+                RADVD_STATE_DIR, netname);
+    return configfile;
+}
 
 static void
 networkFindActiveConfigs(struct network_driver *driver) {
@@ -144,26 +164,43 @@ networkFindActiveConfigs(struct network_driver *driver) {
             brHasBridge(driver->brctl, obj->def->bridge) == 0) {
             obj->active = 1;
 
-            /* Finally try and read dnsmasq pid if any */
-            if (obj->def->ips && (obj->def->nips > 0) &&
-                virFileReadPid(NETWORK_PID_DIR, obj->def->name,
-                               &obj->dnsmasqPid) == 0) {
-
-                /* Check its still alive */
-                if (kill(obj->dnsmasqPid, 0) != 0)
-                    obj->dnsmasqPid = -1;
-
-#ifdef __linux__
-                char *pidpath;
+            /* Try and read dnsmasq/radvd pids if any */
+            if (obj->def->ips && (obj->def->nips > 0)) {
+                char *pidpath, *radvdpidbase;
+
+                if (virFileReadPid(NETWORK_PID_DIR, obj->def->name,
+                                   &obj->dnsmasqPid) == 0) {
+                    /* Check that it's still alive */
+                    if (kill(obj->dnsmasqPid, 0) != 0)
+                        obj->dnsmasqPid = -1;
+                    if (virAsprintf(&pidpath, "/proc/%d/exe", obj->dnsmasqPid) < 0) {
+                        virReportOOMError();
+                        goto cleanup;
+                    }
+                    if (virFileLinkPointsTo(pidpath, DNSMASQ) == 0)
+                        obj->dnsmasqPid = -1;
+                    VIR_FREE(pidpath);
+                }
 
-                if (virAsprintf(&pidpath, "/proc/%d/exe", obj->dnsmasqPid) < 0) {
+                if (!(radvdpidbase = networkRadvdPidfileBasename(obj->def->name))) {
                     virReportOOMError();
                     goto cleanup;
                 }
-                if (virFileLinkPointsTo(pidpath, DNSMASQ) == 0)
-                    obj->dnsmasqPid = -1;
-                VIR_FREE(pidpath);
-#endif
+                if (virFileReadPid(NETWORK_PID_DIR, radvdpidbase,
+                                   &obj->radvdPid) == 0) {
+                    /* Check that it's still alive */
+                    if (kill(obj->radvdPid, 0) != 0)
+                        obj->radvdPid = -1;
+                    if (virAsprintf(&pidpath, "/proc/%d/exe", obj->radvdPid) < 0) {
+                        virReportOOMError();
+                        VIR_FREE(radvdpidbase);
+                        goto cleanup;
+                    }
+                    if (virFileLinkPointsTo(pidpath, RADVD) == 0)
+                        obj->radvdPid = -1;
+                    VIR_FREE(pidpath);
+                }
+                VIR_FREE(radvdpidbase);
             }
         }
 
@@ -588,6 +625,123 @@ cleanup:
 }
 
 static int
+networkStartRadvd(virNetworkObjPtr network)
+{
+    char *pidfile = NULL;
+    char *radvdpidbase = NULL;
+    virBuffer configbuf = VIR_BUFFER_INITIALIZER;;
+    char *configstr = NULL;
+    char *configfile = NULL;
+    virCommandPtr cmd = NULL;
+    int ret = -1, err, ii;
+    virNetworkIpDefPtr ipdef;
+
+    network->radvdPid = -1;
+
+    if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) {
+        virReportSystemError(err,
+                             _("cannot create directory %s"),
+                             NETWORK_PID_DIR);
+        goto cleanup;
+    }
+    if ((err = virFileMakePath(RADVD_STATE_DIR)) != 0) {
+        virReportSystemError(err,
+                             _("cannot create directory %s"),
+                             RADVD_STATE_DIR);
+        goto cleanup;
+    }
+
+    /* construct pidfile name */
+    if (!(radvdpidbase = networkRadvdPidfileBasename(network->def->name))) {
+        virReportOOMError();
+        goto cleanup;
+    }
+    if (!(pidfile = virFilePid(NETWORK_PID_DIR, radvdpidbase))) {
+        virReportOOMError();
+        goto cleanup;
+    }
+
+    /* create radvd config file appropriate for this network */
+    virBufferVSprintf(&configbuf, "interface %s\n"
+                      "{\n"
+                      "  AdvSendAdvert on;\n"
+                      "  AdvManagedFlag off;\n"
+                      "  AdvOtherConfigFlag off;\n"
+                      "\n",
+                      network->def->bridge);
+    for (ii = 0;
+         (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET6, ii));
+         ii++) {
+        int prefix;
+        char *netaddr;
+
+        prefix = virNetworkIpDefPrefix(ipdef);
+        if (prefix < 0) {
+            networkReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("bridge  '%s' has an invalid prefix"),
+                               network->def->bridge);
+            goto cleanup;
+        }
+        if (!(netaddr = virSocketFormatAddr(&ipdef->address)))
+            goto cleanup;
+        virBufferVSprintf(&configbuf,
+                          "  prefix %s/%d\n"
+                          "  {\n"
+                          "    AdvOnLink on;\n"
+                          "    AdvAutonomous on;\n"
+                          "    AdvRouterAddr off;\n"
+                          "  };\n",
+                          netaddr, prefix);
+        VIR_FREE(netaddr);
+    }
+
+    virBufferAddLit(&configbuf, "};\n");
+
+    if (virBufferError(&configbuf)) {
+        virReportOOMError();
+        goto cleanup;
+    }
+    if (!(configstr = virBufferContentAndReset(&configbuf))) {
+        virReportOOMError();
+        goto cleanup;
+    }
+
+    /* construct the filename */
+    if (!(configfile = networkRadvdConfigFileName(network->def->name))) {
+        virReportOOMError();
+        goto cleanup;
+    }
+    /* write the file */
+    if (virFileWriteStr(configfile, configstr, 0600) < 0) {
+        virReportSystemError(errno,
+                             _("couldn't write radvd config file '%s'"),
+                             configfile);
+        goto cleanup;
+    }
+
+    cmd = virCommandNew(RADVD);
+    virCommandAddArgList(cmd, "--config", configfile,
+                         "--pidfile", pidfile,
+                         NULL);
+    if (virCommandRun(cmd, NULL) < 0)
+        goto cleanup;
+
+    if (virFileReadPid(NETWORK_PID_DIR, radvdpidbase,
+                       &network->radvdPid) < 0)
+        goto cleanup;
+
+    ret = 0;
+cleanup:
+    virCommandFree(cmd);
+    VIR_FREE(configfile);
+    VIR_FREE(configstr);
+    virBufferFreeAndReset(&configbuf);
+    VIR_FREE(radvdpidbase);
+    VIR_FREE(pidfile);
+    return ret;
+}
+
+static int
 networkAddMasqueradingIptablesRules(struct network_driver *driver,
                                     virNetworkObjPtr network,
                                     virNetworkIpDefPtr ipdef)
@@ -1154,9 +1308,14 @@ networkReloadIptablesRules(struct network_driver *driver)
 
 /* Enable IP Forwarding. Return 0 for success, -1 for failure. */
 static int
-networkEnableIpForwarding(void)
+networkEnableIpForwarding(int enableIPv4, int enableIPv6)
 {
-    return virFileWriteStr("/proc/sys/net/ipv4/ip_forward", "1\n", 0);
+    int ret = 0;
+    if (enableIPv4)
+        ret = virFileWriteStr("/proc/sys/net/ipv4/ip_forward", "1\n", 0);
+    if (enableIPv6 && ret == 0)
+        ret = virFileWriteStr("/proc/sys/net/ipv6/conf/all/forwarding", "1\n", 0);
+    return ret;
 }
 
 #define SYSCTL_PATH "/proc/sys"
@@ -1431,7 +1590,7 @@ networkStartNetworkDaemon(struct network_driver *driver,
 
     /* If forwardType != NONE, turn on global IP forwarding */
     if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE &&
-        networkEnableIpForwarding() < 0) {
+        networkEnableIpForwarding(v4present, v6present) < 0) {
         virReportSystemError(errno, "%s",
                              _("failed to enable IP forwarding"));
         goto err3;
@@ -1442,15 +1601,28 @@ networkStartNetworkDaemon(struct network_driver *driver,
     if (v4present && networkStartDhcpDaemon(network) < 0)
         goto err3;
 
+    /* start radvd if there are any ipv6 addresses */
+    if (v6present && networkStartRadvd(network) < 0)
+        goto err4;
+
     /* Persist the live configuration now we have bridge info  */
     if (virNetworkSaveConfig(NETWORK_STATE_DIR, network->def) < 0) {
-        goto err4;
+        goto err5;
     }
 
     network->active = 1;
 
     return 0;
 
+ err5:
+    if (!save_err)
+        save_err = virSaveLastError();
+
+    if (network->radvdPid > 0) {
+        kill(network->radvdPid, SIGTERM);
+        network->radvdPid = -1;
+    }
+
  err4:
     if (!save_err)
         save_err = virSaveLastError();
@@ -1509,6 +1681,9 @@ static int networkShutdownNetworkDaemon(struct network_driver *driver,
     unlink(stateFile);
     VIR_FREE(stateFile);
 
+    if (network->radvdPid > 0)
+        kill(network->radvdPid, SIGTERM);
+
     if (network->dnsmasqPid > 0)
         kill(network->dnsmasqPid, SIGTERM);
 
@@ -1529,8 +1704,13 @@ static int networkShutdownNetworkDaemon(struct network_driver *driver,
     if (network->dnsmasqPid > 0 &&
         (kill(network->dnsmasqPid, 0) == 0))
         kill(network->dnsmasqPid, SIGKILL);
-
     network->dnsmasqPid = -1;
+
+    if (network->radvdPid > 0 &&
+        (kill(network->radvdPid, 0) == 0))
+        kill(network->radvdPid, SIGKILL);
+    network->radvdPid = -1;
+
     network->active = 0;
 
     if (network->newDef) {
@@ -1891,6 +2071,16 @@ static int networkUndefine(virNetworkPtr net) {
         dnsmasqContextFree(dctx);
     }
 
+    if (v6present) {
+        char *configfile = networkRadvdConfigFileName(network->def->name);
+
+        if (!configfile) {
+            virReportOOMError();
+            goto cleanup;
+        }
+        unlink(configfile);
+    }
+
     virNetworkRemoveInactive(&driver->networks,
                              network);
     network = NULL;
-- 
1.7.3.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]