[ovs-discuss] [nic19-2 5/5] xenserver: Fix "brctl show" compatibility by introducing "brctl" wrapper.

Ben Pfaff blp at nicira.com
Tue Aug 18 23:08:31 UTC 2009


Bug NIC-19, which reported that "brctl show" did not format its output in
the way expected by Citrix QA scripts, was believed fixed by commit
35c979bff4 "vswitchd: Support creating fake bond device interfaces."
Unfortunately, this commit was not tested on a XenServer before it was
committed.  Due to differences in the actual test environment and the
XenServer environment, which have different versions of the bridge-utils
package that contains brctl, that commit did not fix the problem observed
by Citrix QA.  In particular, the XenServer brctl uses sysfs to obtain
the information displayed by "brctl show", but the previous commit only
fixed up the information output by the bridge ioctls.

The natural way to fix this problem would be to fix up the sysfs support
as well.  I started out along that path, but became bogged down in all
the details of the kernel sysfs.

This commit takes an alternate approach, by introducing a wrapper around
the system brctl binary that implements "brctl show" itself and delegates
all other functionality to the original binary (in a different location).
This will not fix tools that do not call into brctl, but to the best of
my knowledge there are no such tools used in the Citrix QA process.

Bug NIC-19.
---
 xenserver/README           |    5 +
 xenserver/automake.mk      |    1 +
 xenserver/usr_sbin_brctl   |  179 ++++++++++++++++++++++++++++++++++++++++++++
 xenserver/vswitch-xen.spec |   30 +++++++-
 4 files changed, 211 insertions(+), 4 deletions(-)
 create mode 100755 xenserver/usr_sbin_brctl

diff --git a/xenserver/README b/xenserver/README
index 6c91e94..b940e3f 100644
--- a/xenserver/README
+++ b/xenserver/README
@@ -60,6 +60,11 @@ files are:
         used to control vswitch when integrated with Citrix management
         tools.
 
+    usr_sbin_brctl
+
+        wrapper for /usr/sbin/brctl that provides some additional
+        bridge compatibility
+
     usr_sbin_xen-bugtool
 
         vswitch-aware replacement for Citrix script of the same name.
diff --git a/xenserver/automake.mk b/xenserver/automake.mk
index 2fe1289..ceebb9d 100644
--- a/xenserver/automake.mk
+++ b/xenserver/automake.mk
@@ -17,5 +17,6 @@ EXTRA_DIST += \
 	xenserver/opt_xensource_libexec_interface-reconfigure \
 	xenserver/root_vswitch_scripts_dump-vif-details \
 	xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py \
+	xenserver/usr_sbin_brctl \
 	xenserver/usr_sbin_xen-bugtool \
 	xenserver/vswitch-xen.spec
diff --git a/xenserver/usr_sbin_brctl b/xenserver/usr_sbin_brctl
new file mode 100755
index 0000000..41633cd
--- /dev/null
+++ b/xenserver/usr_sbin_brctl
@@ -0,0 +1,179 @@
+#! /usr/bin/python
+#
+# Copyright (c) 2009 Nicira Networks.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import getopt
+import os
+import re
+import sys
+
+argv = sys.argv[0]
+
+BRCTL = "/root/vswitch/xs-original/brctl"
+VSWITCHD_CONF = "/etc/ovs-vswitchd.conf"
+
+# Execute the real brctl program, passing the same arguments that were passed
+# to us.
+def delegate():
+    os.execl(BRCTL, BRCTL, *sys.argv[1:])
+    # execl should never return.  We only arrive here if brctl failed to exec.
+    sys.exit(1)
+
+# Read the ovs-vswitchd.conf file named 'filename' and return its contents as a
+# dictionary that maps from string keys to lists of string values.  (Even
+# singleton values are represented as lists.)
+def cfg_read(filename):
+    try:
+        f = open(filename)
+    except IOError, e:
+        print >>sys.stderr, "%s: open failed (%s)" % (filename, e.strerror)
+        sys.exit(1)
+
+    cfg = {}
+    rx = re.compile('([-._@$:+a-zA-Z0-9]+)(?:[ \t\r\n\v]*)=(?:[ \t\r\n\v]*)(.*)$')
+    for line in f:
+        line = line.strip()
+        if len(line) == 0 or line[0] == '#':
+            continue
+
+        match = rx.match(line)
+        if match == None:
+            continue
+
+        key, value = match.groups()
+        if key not in cfg:
+            cfg[key] = []
+        cfg[key] += [value]
+    return cfg
+
+# Returns a set of the immediate subsections of 'section' within 'cfg'.  For
+# example, if 'section' is "bridge" and keys bridge.a, bridge.b, bridge.b.c,
+# and bridge.c.x.y.z exist, returns set(['a', 'b', 'c']).
+def cfg_get_subsections(cfg, section):
+    subsections = set()
+    for key, value in cfg.iteritems():
+        if key.startswith(section + "."):
+            dot = key.find(".", len(section) + 1)
+            if dot == -1:
+                dot = len(key)
+            subsections.add(key[len(section) + 1:dot])
+    return subsections
+
+# Returns True if 'cfg' contains a key whose single value is 'true'.  Otherwise
+# returns False.
+def cfg_get_bool(cfg, name):
+    return name in cfg and cfg[name] == ['true']
+
+# If 'cfg' has a port named 'port' configured with an implicit VLAN, returns
+# that VLAN number.  Otherwise, returns -1.
+def get_port_vlan(cfg, port):
+    try:
+        return int(cfg["vlan.%s.tag" % port][0])
+    except (ValueError, KeyError):
+        return 0
+
+# Returns all the ports within 'bridge' in 'cfg'.  If 'vlan' is nonnegative,
+# the ports returned are only those configured with implicit VLAN 'vlan'.
+def get_bridge_ports(cfg, bridge, vlan):
+    ports = []
+    for port in cfg["bridge.%s.port" % bridge]:
+        if vlan < 0 or get_port_vlan(cfg, port) == vlan:
+            ports += [port]
+    return ports
+
+# Returns all the interfaces within 'bridge' in 'cfg'.  If 'vlan' is
+# nonnegative, the interfaces returned are only those whose ports are
+# configured with implicit VLAN 'vlan'.
+def get_bridge_ifaces(cfg, bridge, vlan):
+    ifaces = []
+    for port in get_bridge_ports(cfg, bridge, vlan):
+        slave_key = "bonding.%s.slave" % port
+        if slave_key in cfg:
+            ifaces += cfg[slave_key]
+        else:
+            ifaces += [port]
+    return ifaces
+
+# Returns the first line of the file named 'name', with the trailing new-line
+# (if any) stripped off.
+def read_first_line_of_file(name):
+    file = None
+    try:
+        file = open(name, 'r')
+        return file.readline().rstrip('\n')
+    finally:
+        if file != None:
+            file.close()
+
+# Returns a bridge ID constructed from the MAC address of network device
+# 'netdev', in the format "8000.000102030405".
+def get_bridge_id(netdev):
+    hwaddr = read_first_line_of_file("/sys/class/net/%s/address" % netdev)
+    return "8000.%s" % (hwaddr.replace(":", ""))
+
+def cmd_show():
+    print "bridge name\tbridge id\t\tSTP enabled\tinterfaces"
+    cfg = cfg_read(VSWITCHD_CONF)
+
+    # Find all the bridges.
+    real_bridges = [(br, br, 0) for br in cfg_get_subsections(cfg, "bridge")]
+    fake_bridges = []
+    for linux_bridge, ovs_bridge, vlan in real_bridges:
+        for iface in get_bridge_ifaces(cfg, ovs_bridge, -1):
+            if cfg_get_bool(cfg, "iface.%s.fake-bridge" % iface):
+                fake_bridges += [(iface, ovs_bridge,
+                                  get_port_vlan(cfg, iface))]
+    bridges = real_bridges + fake_bridges
+
+    # Find all the interfaces on each bridge.
+    for linux_bridge, ovs_bridge, vlan in bridges:
+        bridge_ports = get_bridge_ports(cfg, ovs_bridge, vlan)
+        bridge_ports = [port for port in bridge_ports if port != linux_bridge]
+        bridge_ports.sort()
+        bridge_id = get_bridge_id(linux_bridge)
+        first_port = ""
+        if len(bridge_ports):
+            first_port = bridge_ports[0]
+        print "%s\t\t%s\t%s\t\t%s" % (linux_bridge, bridge_id, "no", first_port)
+        for port in bridge_ports[1:]:
+            print "\t\t\t\t\t\t\t%s" % port
+
+def main():
+    # Parse the command line.
+    try:
+        options, args = getopt.gnu_getopt(sys.argv[1:],
+                                          "hV", ["help", "version"])
+    except getopt.GetoptError, msg:
+        print >>sys.stderr, "%s (use --help for help)" % msg
+        sys.exit(1)
+
+    # Handle command-line options.
+    for opt, optarg in options:
+        if opt == "-h" or opt == "--help":
+            delegate()
+        elif opt == "-V" or opt == "--version":
+            os.spawnl(os.P_WAIT, BRCTL, BRCTL, "--version")
+            print "Open vSwitch brctl wrapper"
+            sys.exit(0)
+
+    # Execute commands.  Most commands are delegated to the brctl binary that
+    # we are wrapping, but we implement the "show" command ourselves.
+    if len(args) > 0 and args[0] == "show":
+        cmd_show()
+    else:
+        delegate()
+
+if __name__ == "__main__":
+    main()
diff --git a/xenserver/vswitch-xen.spec b/xenserver/vswitch-xen.spec
index c7984ac..fda211a 100644
--- a/xenserver/vswitch-xen.spec
+++ b/xenserver/vswitch-xen.spec
@@ -71,6 +71,8 @@ install -m 755 xenserver/root_vswitch_scripts_dump-vif-details \
                $RPM_BUILD_ROOT%{_prefix}/scripts/dump-vif-details
 install -m 755 xenserver/usr_sbin_xen-bugtool \
              $RPM_BUILD_ROOT%{_prefix}/scripts/xen-bugtool
+install -m 755 xenserver/usr_sbin_brctl \
+             $RPM_BUILD_ROOT%{_prefix}/scripts/brctl
 install -m 644 \
         xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py \
                $RPM_BUILD_ROOT%{_prefix}/scripts/XSFeatureVSwitch.py
@@ -115,11 +117,28 @@ EOF
         printf "\nThe original XenServer scripts replaced by this package\n"
         printf "are different than expected.  This could lead to unexpected\n"
         printf "behavior of your server.  Unless you are sure you know what\n"
-        printf "you are doing, it is highly recomended that you remove this\n"
+        printf "you are doing, it is highly recommended that you remove this\n"
         printf "package immediately after the install completes, which\n"
         printf "will restore the XenServer scripts that you were previously\n"
         printf "using.\n\n"
     fi
+    if test "`/usr/sbin/brctl --version`" != "bridge-utils, 1.1"; then
+cat <<EOF
+
+/usr/sbin/brctl replaced by this package reports the following version:
+
+`/usr/sbin/brctl --version`
+
+The expected version was:
+
+bridge-utils, 1.1
+
+Unless you are sure you know what you are doing, it is highly recommended that
+you remove this package immediately after the install completes, which will
+restore the original /usr/sbin/brctl.
+
+EOF
+    fi
 fi
 
 if test ! -e /etc/ovs-vswitch.dbcache; then
@@ -188,13 +207,14 @@ fi
 # Ensure ovs-vswitchd.conf exists
 touch /etc/ovs-vswitchd.conf
 
-# Replace original XenServer files
+# Replace XenServer files by our versions.
 mkdir -p %{_prefix}/xs-original \
     || printf "Could not create script backup directory.\n"
 for f in \
     /opt/xensource/libexec/interface-reconfigure \
     /etc/xensource/scripts/vif \
-    /usr/sbin/xen-bugtool
+    /usr/sbin/xen-bugtool \
+    /usr/sbin/brctl
 do
     s=$(basename "$f")
     t=$(readlink "$f")
@@ -252,7 +272,8 @@ if [ "$1" = "0" ]; then     # $1 = 1 for upgrade
     for f in \
         /opt/xensource/libexec/interface-reconfigure \
         /etc/xensource/scripts/vif \
-        /usr/sbin/xen-bugtool
+        /usr/sbin/xen-bugtool \
+        /usr/sbin/brctl
     do
         s=$(basename "$f")
         if [ ! -f "%{_prefix}/xs-original/$s" ]; then
@@ -297,6 +318,7 @@ fi
 /root/vswitch/scripts/vif
 /root/vswitch/scripts/xen-bugtool
 /root/vswitch/scripts/XSFeatureVSwitch.py
+/root/vswitch/scripts/brctl
 # Following two files are generated automatically by rpm.  We don't
 # really need them and they won't be used on the XenServer, but there
 # isn't an obvious place to get rid of them since they are generated
-- 
1.6.3.3





More information about the discuss mailing list