Merge commit 'origin/citrix'
authorJustin Pettit <jpettit@nicira.com>
Wed, 1 Jul 2009 18:31:37 +0000 (11:31 -0700)
committerJustin Pettit <jpettit@nicira.com>
Wed, 1 Jul 2009 18:31:37 +0000 (11:31 -0700)
14 files changed:
lib/cfg.c
lib/rconn.c
lib/rconn.h
secchan/status.c
vswitchd/bridge.c
vswitchd/mgmt.c
vswitchd/mgmt.h
vswitchd/ovs-brcompatd.c
vswitchd/ovs-vswitchd.c
xenserver/etc_init.d_vswitch
xenserver/etc_xapi.d_plugins_vswitch-cfg-update
xenserver/etc_xensource_scripts_vif
xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py
xenserver/vswitch-xen.spec

index 833e6e5..901315e 100644 (file)
--- a/lib/cfg.c
+++ b/lib/cfg.c
@@ -332,13 +332,16 @@ try_lock(int fd, bool block)
 int
 cfg_lock(uint8_t *cookie, int timeout)
 {
-    long long int start = time_msec();
+    long long int start;
     long long int elapsed = 0;
     int fd;
     uint8_t curr_cookie[CFG_COOKIE_LEN];
 
     assert(lock_fd < 0);
     COVERAGE_INC(cfg_lock);
+
+    time_refresh();
+    start = time_msec();
     for (;;) {
         int error;
 
index deb9519..b18a2e5 100644 (file)
@@ -698,6 +698,15 @@ rconn_get_last_connection(const struct rconn *rc)
     return rc->last_connected;
 }
 
+/* Returns the time at which the last OpenFlow message was received by 'rc'.
+ * If no packets have been received on 'rc', returns the time at which 'rc'
+ * was created. */
+time_t
+rconn_get_last_received(const struct rconn *rc)
+{
+    return rc->last_received;
+}
+
 /* Returns the time at which 'rc' was created. */
 time_t
 rconn_get_creation_time(const struct rconn *rc)
index f9cbe70..1249b84 100644 (file)
@@ -78,6 +78,7 @@ const char *rconn_get_state(const struct rconn *);
 unsigned int rconn_get_attempted_connections(const struct rconn *);
 unsigned int rconn_get_successful_connections(const struct rconn *);
 time_t rconn_get_last_connection(const struct rconn *);
+time_t rconn_get_last_received(const struct rconn *);
 time_t rconn_get_creation_time(const struct rconn *);
 unsigned long int rconn_get_total_time_connected(const struct rconn *);
 int rconn_get_backoff(const struct rconn *);
index 4d3f411..c7598f3 100644 (file)
@@ -96,6 +96,7 @@ rconn_status_cb(struct status_reply *sr, void *rconn_)
     status_reply_put(sr, "name=%s", rconn_get_name(rconn));
     status_reply_put(sr, "state=%s", rconn_get_state(rconn));
     status_reply_put(sr, "backoff=%d", rconn_get_backoff(rconn));
+    status_reply_put(sr, "probe-interval=%d", rconn_get_probe_interval(rconn));
     status_reply_put(sr, "is-connected=%s",
                      rconn_is_connected(rconn) ? "true" : "false");
     status_reply_put(sr, "sent-msgs=%u", rconn_packets_sent(rconn));
@@ -106,6 +107,8 @@ rconn_status_cb(struct status_reply *sr, void *rconn_)
                      rconn_get_successful_connections(rconn));
     status_reply_put(sr, "last-connection=%ld",
                      (long int) (now - rconn_get_last_connection(rconn)));
+    status_reply_put(sr, "last-received=%ld",
+                     (long int) (now - rconn_get_last_received(rconn)));
     status_reply_put(sr, "time-connected=%lu",
                      rconn_get_total_time_connected(rconn));
     status_reply_put(sr, "state-elapsed=%u", rconn_get_state_elapsed(rconn));
index 0236f14..ba90528 100644 (file)
@@ -27,6 +27,7 @@
 #include <strings.h>
 #include <sys/stat.h>
 #include <sys/socket.h>
+#include <sys/types.h>
 #include <unistd.h>
 #include "bitmap.h"
 #include "cfg.h"
@@ -323,6 +324,7 @@ bridge_configure_ssl(void)
     static char *private_key_file;
     static char *certificate_file;
     static char *cacert_file;
+    struct stat s;
 
     if (config_string_change("ssl.private-key", &private_key_file)) {
         vconn_ssl_set_private_key_file(private_key_file);
@@ -332,7 +334,13 @@ bridge_configure_ssl(void)
         vconn_ssl_set_certificate_file(certificate_file);
     }
 
-    if (config_string_change("ssl.ca-cert", &cacert_file)) {
+    /* We assume that even if the filename hasn't changed, if the CA cert 
+     * file has been removed, that we want to move back into
+     * boot-strapping mode.  This opens a small security hole, because
+     * the old certificate will still be trusted until vSwitch is
+     * restarted.  We may want to address this in vconn's SSL library. */
+    if (config_string_change("ssl.ca-cert", &cacert_file)
+            || (stat(cacert_file, &s) && errno == ENOENT)) {
         vconn_ssl_set_ca_cert_file(cacert_file,
                                    cfg_get_bool(0, "ssl.bootstrap-ca-cert"));
     }
@@ -1123,8 +1131,13 @@ bridge_reconfigure_controller(struct bridge *br)
                              || !strcmp(fail_mode, "open")));
 
         probe = cfg_get_int(0, "%s.inactivity-probe", pfx);
-        ofproto_set_probe_interval(br->ofproto,
-                                   probe ? probe : cfg_get_int(0, "mgmt.inactivity-probe"));
+        if (probe < 5) {
+            probe = cfg_get_int(0, "mgmt.inactivity-probe");
+            if (probe < 5) {
+                probe = 15;
+            }
+        }
+        ofproto_set_probe_interval(br->ofproto, probe);
 
         max_backoff = cfg_get_int(0, "%s.max-backoff", pfx);
         if (!max_backoff) {
index e43c6e8..45c3580 100644 (file)
@@ -19,6 +19,9 @@
 #include <assert.h>
 #include <errno.h>
 #include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/socket.h>
+#include <sys/types.h>
 
 #include "bridge.h"
 #include "cfg.h"
@@ -46,6 +49,7 @@
 
 static struct svec mgmt_cfg;
 static uint8_t cfg_cookie[CFG_COOKIE_LEN];
+static bool need_reconfigure = false;
 static struct rconn *mgmt_rconn;
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(60, 60);
 static struct svec capabilities;
@@ -100,6 +104,7 @@ mgmt_configure_ssl(void)
     static char *private_key_file;
     static char *certificate_file;
     static char *cacert_file;
+    struct stat s;
 
     /* XXX SSL should be configurable separate from the bridges.
      * XXX should be possible to de-configure SSL. */
@@ -111,7 +116,13 @@ mgmt_configure_ssl(void)
         vconn_ssl_set_certificate_file(certificate_file);
     }
 
-    if (config_string_change("ssl.ca-cert", &cacert_file)) {
+    /* We assume that even if the filename hasn't changed, if the CA cert 
+     * file has been removed, that we want to move back into
+     * boot-strapping mode.  This opens a small security hole, because
+     * the old certificate will still be trusted until vSwitch is
+     * restarted.  We may want to address this in vconn's SSL library. */
+    if (config_string_change("ssl.ca-cert", &cacert_file) 
+            || (stat(cacert_file, &s) && errno == ENOENT)) {
         vconn_ssl_set_ca_cert_file(cacert_file,
                 cfg_get_bool(0, "ssl.bootstrap-ca-cert"));
     }
@@ -130,6 +141,7 @@ mgmt_reconfigure(void)
     int retval;
 
     if (!cfg_has_section("mgmt")) {
+        svec_clear(&mgmt_cfg);
         if (mgmt_rconn) {
             rconn_destroy(mgmt_rconn);
             mgmt_rconn = NULL;
@@ -655,8 +667,7 @@ recv_ofmp_config_update(uint32_t xid, const struct ofmp_header *ofmph,
      * connection settings may have changed. */
     send_config_update_ack(xid, true);
 
-    reconfigure();
-
+    need_reconfigure = true;
 
     return 0;
 }
@@ -807,15 +818,16 @@ handle_msg(uint32_t xid, const void *msg, size_t length)
     return handler(xid, msg);
 }
 
-void 
+bool 
 mgmt_run(void)
 {
     int i;
 
     if (!mgmt_rconn) {
-        return;
+        return false;
     }
 
+    need_reconfigure = false;
     rconn_run(mgmt_rconn);
 
     /* Do some processing, but cap it at a reasonable amount so that
@@ -837,6 +849,8 @@ mgmt_run(void)
             VLOG_WARN_RL(&rl, "received too-short OpenFlow message");
         }
     }
+
+    return need_reconfigure;
 }
 
 void
index 83950cd..f05c916 100644 (file)
@@ -18,7 +18,7 @@
 
 void mgmt_init(void);
 void mgmt_reconfigure(void);
-void mgmt_run(void);
+bool mgmt_run(void);
 void mgmt_wait(void);
 uint64_t mgmt_get_mgmt_id(void);
 
index 530ea99..5bb60cb 100644 (file)
@@ -257,12 +257,12 @@ prune_ports(void)
 
             error = netdev_nodev_get_flags(iface_name, &flags);
             if (error == ENODEV) {
-                VLOG_DBG_RL(&rl, "removing dead interface %s from %s", 
-                        iface_name, br_name);
+                VLOG_INFO_RL(&rl, "removing dead interface %s from %s",
+                             iface_name, br_name);
                 svec_add(&delete, iface_name);
             } else if (error) {
-                VLOG_DBG_RL(&rl, "unknown error %d on interface %s from %s", 
-                        error, iface_name, br_name);
+                VLOG_INFO_RL(&rl, "unknown error %d on interface %s from %s",
+                             error, iface_name, br_name);
             }
         }
         svec_destroy(&ifaces);
@@ -292,7 +292,9 @@ prune_ports(void)
  * context, since ovs-vswitchd.conf may cause vswitchd to create or destroy
  * network devices based on iface.*.internal settings.
  *
- * XXX may want to move this to lib/netdev. */
+ * XXX may want to move this to lib/netdev.
+ *
+ * XXX why not just use netdev_nodev_get_flags() or similar function? */
 static bool
 netdev_exists(const char *name)
 {
@@ -563,6 +565,7 @@ rtnl_recv_update(void)
             char br_name[IFNAMSIZ];
             uint32_t br_idx = nl_attr_get_u32(attrs[IFLA_MASTER]);
             struct svec ports;
+            enum netdev_flags flags;
 
             if (!if_indextoname(br_idx, br_name)) {
                 ofpbuf_delete(buf);
@@ -576,12 +579,62 @@ rtnl_recv_update(void)
                 return;
             }
 
-            svec_init(&ports);
-            cfg_get_all_keys(&ports, "bridge.%s.port", br_name);
-            svec_sort(&ports);
-            if (svec_contains(&ports, port_name)) {
-                del_port(br_name, port_name);
-                rewrite_and_reload_config();
+            if (netdev_nodev_get_flags(port_name, &flags) == ENODEV) {
+                /* Network device is really gone. */
+                VLOG_INFO("network device %s destroyed, "
+                          "removing from bridge %s", port_name, br_name);
+                svec_init(&ports);
+                cfg_get_all_keys(&ports, "bridge.%s.port", br_name);
+                svec_sort(&ports);
+                if (svec_contains(&ports, port_name)) {
+                    del_port(br_name, port_name);
+                    rewrite_and_reload_config();
+                }
+            } else {
+                /* A network device by that name exists even though the kernel
+                 * told us it had disappeared.  Probably, what happened was
+                 * this:
+                 *
+                 *      1. Device destroyed.
+                 *      2. Notification sent to us.
+                 *      3. New device created with same name as old one.
+                 *      4. ovs-brcompatd notified, removes device from bridge.
+                 *
+                 * There's no a priori reason that in this situation that the
+                 * new device with the same name should remain in the bridge;
+                 * on the contrary, that would be unexpected.  *But* there is
+                 * one important situation where, if we do this, bad things
+                 * happen.  This is the case of XenServer Tools version 5.0.0,
+                 * which on boot of a Windows VM cause something like this to
+                 * happen on the Xen host:
+                 *
+                 *      i. Create tap1.0 and vif1.0.
+                 *      ii. Delete tap1.0.
+                 *      iii. Delete vif1.0.
+                 *      iv. Re-create vif1.0.
+                 *
+                 * (XenServer Tools 5.5.0 does not exhibit this behavior, and
+                 * neither does a VM without Tools installed at all.@.)
+                 *
+                 * Steps iii and iv happen within a few seconds of each other.
+                 * Step iv causes /etc/xensource/scripts/vif to run, which in
+                 * turn calls ovs-cfg-mod to add the new device to the bridge.
+                 * If step iv happens after step 4 (in our first list of
+                 * steps), then all is well, but if it happens between 3 and 4
+                 * (which can easily happen if ovs-brcompatd has to wait to
+                 * lock the configuration file), then we will remove the new
+                 * incarnation from the bridge instead of the old one!
+                 *
+                 * So, to avoid this problem, we do nothing here.  This is
+                 * strictly incorrect except for this one particular case, and
+                 * perhaps that will bite us someday.  If that happens, then we
+                 * will have to somehow track network devices by ifindex, since
+                 * a new device will have a new ifindex even if it has the same
+                 * name as an old device.
+                 */
+                VLOG_INFO("kernel reported network device %s removed but "
+                          "a device by that name exists (XS Tools 5.0.0?)",
+                          port_name);
             }
             cfg_unlock();
         }
index 84c373f..e95ee0a 100644 (file)
@@ -93,7 +93,9 @@ main(int argc, char *argv[])
             vlog_reopen_log_file();
             reconfigure();
         }
-        mgmt_run();
+        if (mgmt_run()) {
+            need_reconfigure = true;
+        }
         if (bridge_run()) {
             need_reconfigure = true;
         }
index d1fc75b..37269d6 100755 (executable)
@@ -48,7 +48,7 @@ BRCOMPATD_PIDFILE="${BRCOMPATD_PIDFILE:-/var/run/ovs-brcompatd.pid}"
 BRCOMPATD_PRIORITY="${BRCOMPATD_PRIORITY:--5}"
 BRCOMPATD_LOGFILE="${BRCOMPATD_LOGFILE:-/var/log/ovs-brcompatd.log}"
 BRCOMPATD_FILE_LOGLEVEL="${BRCOMPATD_FILE_LOGLEVEL:-}"
-BRCOMPATD_SYSLOG_LOGLEVEL="${BRCOMPATD_SYSLOG_LOGLEVEL:-WARN}"
+BRCOMPATD_SYSLOG_LOGLEVEL="${BRCOMPATD_SYSLOG_LOGLEVEL:-INFO}"
 BRCOMPATD_MEMLEAK_LOGFILE="${BRCOMPATD_MEMLEAK_LOGFILE:-}"
 BRCOMPATD_STRACE_LOG="${BRCOMPATD_STRACE_LOG:-}"
 BRCOMPATD_STRACE_OPT="${BRCOMPATD_STRACE_OPT:-}"
index ef4ce78..ce407ad 100755 (executable)
@@ -27,10 +27,20 @@ logging.basicConfig(filename="/var/log/vswitch-cfg-update.log", level=logging.DE
 
 import XenAPIPlugin
 import XenAPI
+import os
 import subprocess
 
 cfg_mod="/root/vswitch/bin/ovs-cfg-mod"
 vswitchd_cfg_filename="/etc/ovs-vswitchd.conf"
+cacert_filename="/etc/ovs-vswitchd.cacert"
+
+# Delete the CA certificate, so that we go back to boot-strapping mode
+def delete_cacert():
+    try:
+        os.remove(cacert_filename)
+    except OSError:
+        # Ignore error if file doesn't exist
+        pass
 
 def update(session, args):
     pools = session.xenapi.pool.get_all()
@@ -49,6 +59,7 @@ def update(session, args):
     currentController = vswitchCurrentController()
     if controller == "" and currentController != "":
         log.debug("Removing controller configuration.")
+        delete_cacert()
         removeControllerCfg()
         return "Successfully removed controller config"
     elif controller != currentController:
@@ -56,6 +67,7 @@ def update(session, args):
             log.debug("Setting controller to: %s" % (controller))
         else:
             log.debug("Changing controller from %s to %s" % (currentController, controller))
+        delete_cacert()
         setControllerCfg(controller)
         return "Successfully set controller to " + controller
     else:
index 35ada1f..aebb4cc 100755 (executable)
@@ -128,9 +128,10 @@ remove)
        xenstore-rm "${HOTPLUG}/hotplug"
        vif=vif${DOMID}.${DEVID}
        logger -t scripts-vif "${vif} has been removed"
-        $cfg_mod -vANY:console:emer -F /etc/ovs-vswitchd.conf \
-            --del-match="bridge.*.port=${vif}" \
+       $cfg_mod -vANY:console:emer -F /etc/ovs-vswitchd.conf \
+           --del-match="bridge.*.port=${vif}" \
            --del-match="vlan.${vif}.[!0-9]*" \
            --del-match="port.${vif}.[!0-9]*" -c
+       $service vswitch reload
        ;;
 esac
index 8f4be31..4523139 100644 (file)
@@ -12,6 +12,7 @@ log = logging.getLogger("vswitch-cfg-update")
 logging.basicConfig(filename="/var/log/vswitch-xsplugin.log", level=logging.DEBUG)
 
 import os
+import socket
 import subprocess
 
 cfg_mod="/root/vswitch/bin/ovs-cfg-mod"
@@ -90,7 +91,11 @@ class VSwitchControllerDialogue(Dialogue):
 
         self.hostsInPool = 0
         self.hostsUpdated = 0
-        self.controller = data.GetPoolForThisHost().get("other_config", {}).get("vSwitchController", "")
+        pool = data.GetPoolForThisHost()
+        if pool is not None:
+            self.controller = pool.get("other_config", {}).get("vSwitchController", "")
+        else:
+            self.controller = ""
 
         choiceDefs = [
             ChoiceDef(Lang("Set pool-wide controller"),
@@ -158,6 +163,14 @@ class VSwitchControllerDialogue(Dialogue):
             inputValues = pane.GetFieldValues()
             self.controller = inputValues['address']
             Layout.Inst().PopDialogue()
+
+            # Make sure the controller is specified as a valid dotted quad
+            try:
+                socket.inet_aton(self.controller)
+            except socket.error:
+                Layout.Inst().PushDialogue(InfoDialogue(Lang("Please enter in dotted quad format")))
+                return True
+
             Layout.Inst().TransientBanner(Lang("Setting controller..."))
             try:
                 self.SetController(self.controller)
@@ -253,7 +266,13 @@ class XSFeatureVSwitch:
         inPane.AddStatusField(Lang("Version", 20), versionStr)
 
         inPane.NewLine()
-        dbController = data.GetPoolForThisHost().get("other_config", {}).get("vSwitchController", "")
+
+        pool = data.GetPoolForThisHost()
+        if pool is not None:
+            dbController = pool.get("other_config", {}).get("vSwitchController", "")
+        else:
+            dbController = ""
+
         if dbController == "":
             dbController = Lang("<None>")
         inPane.AddStatusField(Lang("Controller (config)", 20), dbController)
index 05f9be9..e660027 100644 (file)
@@ -67,7 +67,7 @@ install -m 755 xenserver/etc_xensource_scripts_vif \
              $RPM_BUILD_ROOT%{_prefix}/scripts/vif
 install -m 755 xenserver/root_vswitch_scripts_dump-vif-details \
                $RPM_BUILD_ROOT%{_prefix}/scripts/dump-vif-details
-install -m 755 \
+install -m 644 \
         xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py \
                $RPM_BUILD_ROOT%{_prefix}/scripts/XSFeatureVSwitch.py