From 8521345b51ef456819412b175538555eb9f3c152 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 19 Aug 2009 14:14:40 -0700 Subject: [PATCH] xenserver: Fix "brctl show" compatibility by introducing "brctl" wrapper. 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. Thanks to Justin and Reid for much feedback. Bug NIC-19. --- xenserver/README | 5 + xenserver/automake.mk | 1 + xenserver/usr_sbin_brctl | 181 +++++++++++++++++++++++++++++++++++++ xenserver/vswitch-xen.spec | 30 +++++- 4 files changed, 213 insertions(+), 4 deletions(-) create mode 100755 xenserver/usr_sbin_brctl diff --git a/xenserver/README b/xenserver/README index 6c91e941..b940e3f7 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 2fe1289c..ceebb9d9 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 00000000..b97b0786 --- /dev/null +++ b/xenserver/usr_sbin_brctl @@ -0,0 +1,181 @@ +#! /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 subprocess +import sys + +argv0 = 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: + sys.stderr.write("%s: could not open %s (%s)\n" + % (argv0, 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].append(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 in cfg: + 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 0. +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.append(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): + ifaces.extend(cfg.get("bonding.%s.slave" % port, [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): + try: + hwaddr = read_first_line_of_file("/sys/class/net/%s/address" % netdev) + return "8000.%s" % (hwaddr.replace(":", "")) + except: + return "8000.002320ffffff" + +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.append((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) + if linux_bridge in bridge_ports: + bridge_ports.remove(linux_bridge) + bridge_ports.sort() + bridge_id = get_bridge_id(linux_bridge) + first_port = "" + if 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: + sys.stderr.write("%s: %s (use --help for help)\n" % (argv0, 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": + subprocess.call([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 args 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 c7984ac0..fda211af 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 <