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

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



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.

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

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 79d3010..e9f3c90 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -62,6 +62,7 @@
 #include "virnetdev.h"
 #include "virnetdevbridge.h"
 #include "virnetdevtap.h"
+#include "virfile.h"
 
 #define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
 #define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
@@ -840,6 +841,7 @@ networkStartRadvd(virNetworkObjPtr network)
                       "  AdvSendAdvert on;\n"
                       "  AdvManagedFlag off;\n"
                       "  AdvOtherConfigFlag off;\n"
+                      "  IgnoreIfMissing on;\n"
                       "\n",
                       network->def->bridge);
     for (ii = 0;
@@ -1744,6 +1746,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)
@@ -1765,10 +1768,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,
-                                           VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE) < 0) {
+                                           NULL, &tapfd, 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;
         }
@@ -1828,6 +1834,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) {
         networkReportError(VIR_ERR_INTERNAL_ERROR,
                            _("cannot set bandwidth limits on %s"),
@@ -1866,6 +1882,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 79b249d..c9b5044 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -141,7 +141,7 @@ umlConnectTapDevice(virConnectPtr conn,
     if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac,
                                        vm->uuid, NULL,
                                        virDomainNetGetActualVirtPortProfile(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 5d21164..45ff20f 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -118,18 +118,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 an errno code in case of failure.
  */
 int virNetDevTapCreate(char **ifname,
                        int *tapfd,
-                       unsigned int flags ATTRIBUTE_UNUSED)
+                       unsigned int flags)
 {
     int fd;
     struct ifreq ifr;
@@ -166,7 +168,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"),
@@ -267,14 +269,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 d9a3593..f1355ba 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -42,6 +42,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,

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


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