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

[libvirt] [PATCH] Fix dnsmasq/radvd on bridges not being able to be bound to IPv6 address on "recent" kernels



From: Benjamin Cama <benjamin cama telecom-bretagne eu>

Le mercredi 20 juin 2012 à 11:05 +0100, Daniel P. Berrange a écrit :
> I think you also need to have a VIR_FOFRCE_CLOSE(tapfd) in the
> very last of the cleanup jump labels. Since between the time
> we open the tapfd & close it, we might have done a 'goto err1'
> or similar.

Yes, I forgot that. New patch following.

> Looks reasonable apart from the cleanup bug.

Thanks. BTW, if this patch is commited, I'm already in AUTHORS under
another email address, which is OK.
---

This is my attempt at resolving the merge conflict, but I'm not
confident enough with the original patch, nor with my test
environment, to see if I actually solved anything (not to mention
the commit message used by 'git am' is horrible).  Is it worth
using this version?

 src/network/bridge_driver.c | 22 ++++++++++++++++++++--
 src/uml/uml_conf.c          |  3 ++-
 src/util/virnetdevtap.c     | 24 ++++++++++++++----------
 src/util/virnetdevtap.h     |  2 ++
 4 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index fce1739..33aba74 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -65,6 +65,7 @@
 #include "virnetdevtap.h"
 #include "virnetdevvportprofile.h"
 #include "virdbus.h"
+#include "virfile.h"

 #define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
 #define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
@@ -993,6 +994,7 @@ networkRadvdConfContents(virNetworkObjPtr network, char **configstr)
                       "  AdvSendAdvert on;\n"
                       "  AdvManagedFlag off;\n"
                       "  AdvOtherConfigFlag off;\n"
+                      "  IgnoreIfMissing on;\n"
                       "\n",
                       network->def->bridge);

@@ -2061,6 +2063,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
     virErrorPtr save_err = NULL;
     virNetworkIpDefPtr ipdef;
     char *macTapIfName = NULL;
+    int tapfd = -1;

     /* Check to see if any network IP collides with an existing route */
     if (networkCheckRouteCollision(network) < 0)
@@ -2082,10 +2085,13 @@ networkStartNetworkVirtual(struct network_driver *driver,
             virReportOOMError();
             goto err0;
         }
+        /* Keep tun fd open and interface up to allow for IPv6 DAD to happen */
         if (virNetDevTapCreateInBridgePort(network->def->bridge,
                                            &macTapIfName, &network->def->mac,
-                                           NULL, NULL, NULL, NULL,
-                                           VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE) < 0) {
+                                           NULL, &tapfd, NULL, NULL,
+                                           VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE |
+                                           VIR_NETDEV_TAP_CREATE_IFUP |
+                                           VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
             VIR_FREE(macTapIfName);
             goto err0;
         }
@@ -2149,6 +2155,16 @@ networkStartNetworkVirtual(struct network_driver *driver,
     if (v6present && networkStartRadvd(network) < 0)
         goto err4;

+    /* DAD has happened (dnsmasq waits for it), dnsmasq is now bound to the
+     * bridge's IPv6 address, and radvd to the interface, so we can now set the
+     * dummy tun down.
+     */
+    if (tapfd >= 0) {
+        if (virNetDevSetOnline(macTapIfName, false) < 0)
+            goto err4;
+        VIR_FORCE_CLOSE(tapfd);
+    }
+
     if (virNetDevBandwidthSet(network->def->bridge, network->def->bandwidth) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("cannot set bandwidth limits on %s"),
@@ -2187,6 +2203,8 @@ networkStartNetworkVirtual(struct network_driver *driver,
         save_err = virSaveLastError();

     if (macTapIfName) {
+        if (tapfd >= 0)
+            VIR_FORCE_CLOSE(tapfd);
         ignore_value(virNetDevTapDelete(macTapIfName));
         VIR_FREE(macTapIfName);
     }
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index a317bcc..e8b8779 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -142,7 +142,8 @@ umlConnectTapDevice(virConnectPtr conn,
                                        vm->uuid, NULL,
                                        virDomainNetGetActualVirtPortProfile(net),
                                        virDomainNetGetActualVlan(net),
-                                       VIR_NETDEV_TAP_CREATE_IFUP) < 0) {
+                                       VIR_NETDEV_TAP_CREATE_IFUP |
+                                       VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
         if (template_ifname)
             VIR_FREE(net->ifname);
         goto error;
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 9f2dca1..0eadd6c 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -112,18 +112,20 @@ virNetDevProbeVnetHdr(int tapfd)
  *
  *   VIR_NETDEV_TAP_CREATE_VNET_HDR
  *     - Enable IFF_VNET_HDR on the tap device
+ *   VIR_NETDEV_TAP_CREATE_PERSIST
+ *     - The device will persist after the file descriptor is closed
  *
  * Creates a tap interface.
- * If the @tapfd parameter is supplied, the open tap device file
- * descriptor will be returned, otherwise the TAP device will be made
- * persistent and closed. The caller must use virNetDevTapDelete to
- * remove a persistent TAP devices when it is no longer needed.
+ * If the @tapfd parameter is supplied, the open tap device file descriptor
+ * will be returned, otherwise the TAP device will be closed. The caller must
+ * use virNetDevTapDelete to remove a persistent TAP device when it is no
+ * longer needed.
  *
  * Returns 0 in case of success or -1 on failure.
  */
 int virNetDevTapCreate(char **ifname,
                        int *tapfd,
-                       unsigned int flags ATTRIBUTE_UNUSED)
+                       unsigned int flags)
 {
     int fd;
     struct ifreq ifr;
@@ -160,7 +162,7 @@ int virNetDevTapCreate(char **ifname,
         goto cleanup;
     }

-    if (!tapfd &&
+    if ((flags & VIR_NETDEV_TAP_CREATE_PERSIST) &&
         (errno = ioctl(fd, TUNSETPERSIST, 1))) {
         virReportSystemError(errno,
                              _("Unable to set tap device %s to persistent"),
@@ -261,14 +263,16 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)
  *     - Enable IFF_VNET_HDR on the tap device
  *   VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE
  *     - Set this interface's MAC as the bridge's MAC address
+ *   VIR_NETDEV_TAP_CREATE_PERSIST
+ *     - The device will persist after the file descriptor is closed
  *
  * This function creates a new tap device on a bridge. @ifname can be either
  * a fixed name or a name template with '%d' for dynamic name allocation.
  * in either case the final name for the bridge will be stored in @ifname.
- * If the @tapfd parameter is supplied, the open tap device file
- * descriptor will be returned, otherwise the TAP device will be made
- * persistent and closed. The caller must use virNetDevTapDelete to remove
- * a persistent TAP devices when it is no longer needed.
+ * If the @tapfd parameter is supplied, the open tap device file descriptor
+ * will be returned, otherwise the TAP device will be closed. The caller must
+ * use virNetDevTapDelete to remove a persistent TAP device when it is no
+ * longer needed.
  *
  * Returns 0 in case of success or -1 on failure
  */
diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
index fa6a647..980db61 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -43,6 +43,8 @@ typedef enum {
    VIR_NETDEV_TAP_CREATE_VNET_HDR           = 1 << 1,
    /* Set this interface's MAC as the bridge's MAC address */
    VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE = 1 << 2,
+   /* The device will persist after the file descriptor is closed */
+   VIR_NETDEV_TAP_CREATE_PERSIST            = 1 << 3,
 } virNetDevTapCreateFlags;

 int virNetDevTapCreateInBridgePort(const char *brname,
-- 
1.7.11.4


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