[PATCH 0/5] Setup private bridge in KVM autotest, get rid of ifup scripts

[PATCH 0/5] Setup private bridge in KVM autotest, get rid of ifup scripts

[Lucas Meneghel Rodrigues]

This patch combines 2 awesome patchsets by Amos and Jason to
single handedly slay 2 long time annoyances for KVM autotest users:

* By default we were using qemu userspace networking, a mode unsupported
by KVM developers and not terribly useful, and many network tests are
not supposed to work properly with it
* Use of ifup scripts that tried to do a very naive form of bridge
autodetection. If we control our bridge creation, we don't need no
autodetection.

Carefully made, verified, but not tested in several setups.
This might need some more testing love before we can get it
upstream once for all.

Lucas Meneghel Rodrigues (5):
  KVM test: Adding framework code to control bridge creation
  KVM test: Add helpers to control the TAP/bridge
  KVM test: virt_env_process: Setup private bridge during
    postprocessing
  KVM test: setup tap fd and pass it to qemu-kvm
  KVM test: Changing KVM autotest default to private bridge

 client/tests/kvm/scripts/qemu-ifup     |   11 --
 client/tests/kvm/tests_base.cfg.sample |   12 +-
 client/virt/kvm_vm.py                  |   57 ++++++---
 client/virt/virt_env_process.py        |    9 ++
 client/virt/virt_test_setup.py         |  103 +++++++++++++++
 client/virt/virt_utils.py              |  218 ++++++++++++++++++++++++++++++++
 6 files changed, 374 insertions(+), 36 deletions(-)
 delete mode 100755 client/tests/kvm/scripts/qemu-ifup

-- 
1.7.5.1

[PATCH 1/5] KVM test: Adding framework code to control bridge creation

[Lucas Meneghel Rodrigues]

Provide in framework utility code to control the creation
of a bridge, in order to provide TAP functionality for
autotest users without relying on previous setup made by
the user.

This is a reimplementation of Amos's code, the differences
are:

 * Implemented as a setup class, taking advantage of object
internal state to use in different places of the code
 * Split up the operations to make it easier to understand
the steps and why we are doing them
 * Use of autotest API instead of commands

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
Signed-off-by: Amos Kong <akong@redhat.com>
---
 client/virt/virt_test_setup.py |  103 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 103 insertions(+), 0 deletions(-)

diff --git a/client/virt/virt_test_setup.py b/client/virt/virt_test_setup.py
index f915c1b..cdc0e23 100644
--- a/client/virt/virt_test_setup.py
+++ b/client/virt/virt_test_setup.py
@@ -105,3 +105,106 @@ class HugePageConfig(object):
             return
         utils.system("echo 0 > %s" % self.kernel_hp_file)
         logging.debug("Hugepage memory successfuly dealocated")
+
+
+class PrivateBridgeError(Exception):
+    def __init__(self, brname):
+        self.brname = brname
+
+    def __str__(self):
+        return "Bridge %s not available after setup" % self.brname
+
+
+class PrivateBridgeConfig(object):
+    __shared_state = {}
+    def __init__(self, params=None):
+        self.__dict__ = self.__shared_state
+        if params is not None:
+            self.brname = params.get("priv_brname", 'atbr0')
+            self.subnet = params.get("priv_subnet", '192.168.58')
+            self.ip_version = params.get("bridge_ip_version", "ipv4")
+            self.dhcp_server_pid = None
+
+
+    def _add_bridge(self):
+        utils.system("brctl addbr %s" % self.brname)
+        ip_fwd_path = "/proc/sys/net/%s/ip_forward" % self.ip_version
+        ip_fwd = open(ip_fwd_path, "w")
+        ip_fwd.write("1\n")
+        utils.system("brctl stp %s on" % self.brname)
+        utils.system("brctl setfd %s 0" % self.brname)
+
+
+    def _bring_bridge_up(self):
+        utils.system("ifconfig %s %s.1 up" % (self.brname, self.subnet))
+
+
+    def _enable_nat(self):
+        utils.system("iptables -t nat -A POSTROUTING -s %s.254/24 ! "
+                     "-d %s.254/24 -j MASQUERADE" % (self.subnet, self.subnet))
+
+
+    def _start_dhcp_server(self):
+        utils.system("service dnsmasq stop")
+        utils.system("dnsmasq --strict-order --bind-interfaces "
+                     "--listen-address %s.1 --dhcp-range %s.1,%s.254 "
+                     "--pid-file=/tmp/dnsmasq.pid "
+                     "--log-facility=/tmp/dnsmasq.log" %
+                     (self.subnet, self.subnet, self.subnet))
+        try:
+            self.dhcp_server_pid = int(open('/tmp/dnsmasq.pid', 'r').read())
+        except ValueError:
+            raise PrivateBridgeError(self.brname)
+        logging.debug("Started internal DHCP server with PID %s",
+                      self.dhcp_server_pid)
+
+
+    def _verify_bridge(self):
+        brctl_output = utils.system_output("brctl show")
+        if self.brname not in brctl_output:
+            raise PrivateBridgeError(self.brname)
+
+
+    def setup(self):
+        brctl_output = utils.system_output("brctl show")
+        if self.brname not in brctl_output:
+            logging.info("Configuring KVM test private bridge %s", self.brname)
+            self._add_bridge()
+            self._bring_bridge_up()
+            self._enable_nat()
+            self._start_dhcp_server()
+            self._verify_bridge()
+
+
+    def _stop_dhcp_server(self):
+        os.kill(self.dhcp_server_pid, 15)
+
+
+    def _bring_bridge_down(self):
+        utils.system("ifconfig %s down" % self.brname)
+
+
+    def _disable_nat(self):
+        utils.system("iptables -t nat -D POSTROUTING -s %s.254/24 !"
+                     " -d %s.254/24 -j MASQUERADE" % (self.subnet, self.subnet))
+
+
+    def _remove_bridge(self):
+        utils.system("brctl delbr %s" % self.brname)
+
+
+    def cleanup(self):
+        brctl_output = utils.system_output("brctl show")
+        cleanup = False
+        for line in brctl_output:
+            if line.startswith(self.brname):
+                # len == 4 means there is a TAP using the bridge
+                # so don't try to clean up
+                if len(line.split()) < 4:
+                    cleanup = True
+        if cleanup:
+            logging.debug("Cleaning up KVM test private bridge %s", self.brname)
+            self._stop_dhcp_server()
+            self._bring_bridge_down()
+            self._disable_nat()
+            self._remove_bridge()
-- 
1.7.5.1

[PATCH 2/5] KVM test: Add helpers to control the TAP/bridge

[Lucas Meneghel Rodrigues]

This patch adds some helpers to assist virt test to setup the bridge or
macvtap based guest networking.

Changes from v1:
 * Fixed undefined variable errors on the exception class definitions

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/virt/virt_utils.py |  218 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 218 insertions(+), 0 deletions(-)

diff --git a/client/virt/virt_utils.py b/client/virt/virt_utils.py
index 5510c89..96b9c84 100644
--- a/client/virt/virt_utils.py
+++ b/client/virt/virt_utils.py
@@ -6,6 +6,7 @@ KVM test utility functions.
 
 import time, string, random, socket, os, signal, re, logging, commands, cPickle
 import fcntl, shelve, ConfigParser, threading, sys, UserDict, inspect
+import struct
 from autotest_lib.client.bin import utils, os_dep
 from autotest_lib.client.common_lib import error, logging_config
 import rss_client, aexpect
@@ -15,6 +16,20 @@ try:
 except ImportError:
     KOJI_INSTALLED = False
 
+# From include/linux/sockios.h
+SIOCSIFHWADDR = 0x8924
+SIOCGIFHWADDR = 0x8927
+SIOCSIFFLAGS = 0x8914
+SIOCGIFINDEX = 0x8933
+SIOCBRADDIF = 0x89a2
+# From linux/include/linux/if_tun.h
+TUNSETIFF = 0x400454ca
+TUNGETIFF = 0x800454d2
+TUNGETFEATURES = 0x800454cf
+IFF_UP = 0x1
+IFF_TAP = 0x0002
+IFF_NO_PI = 0x1000
+IFF_VNET_HDR = 0x4000
 
 def _lock_file(filename):
     f = open(filename, "w")
@@ -36,6 +51,76 @@ def is_vm(obj):
     return obj.__class__.__name__ == "VM"
 
 
+class NetError(Exception):
+    pass
+
+
+class TAPModuleError(NetError):
+    def __init__(self, devname):
+        NetError.__init__(self, devname)
+        self.devname = devname
+
+    def __str__(self):
+        return "Can't open %s" % self.devname
+
+class TAPNotExistError(NetError):
+    def __init__(self, ifname):
+        NetError.__init__(self, ifname)
+        self.ifname = ifname
+
+    def __str__(self):
+        return "Interface %s does not exist" % self.ifname
+
+
+class TAPCreationError(NetError):
+    def __init__(self, ifname):
+        NetError.__init__(self, ifname)
+        self.ifname = ifname
+
+    def __str__(self):
+        return "Cannot create TAP device %s" % self.ifname
+
+
+class TAPBringUpError(NetError):
+    def __init__(self, ifname):
+        NetError.__init__(self, ifname)
+        self.ifname = ifname
+
+    def __str__(self):
+        return "Cannot bring up TAP %s" % self.ifname
+
+
+class BRAddIfError(NetError):
+    def __init__(self, ifname, brname, details):
+        NetError.__init__(self, ifname, brname, details)
+        self.ifname = ifname
+        self.brname = brname
+        self.details = details
+
+    def __str__(self):
+        return ("Can not add if %s to bridge %s: %s" %
+                (self.ifname, self.brname, self.details))
+
+
+class HwAddrSetError(NetError):
+    def __init__(self, ifname, mac):
+        NetError.__init__(self, ifname, mac)
+        self.ifname = ifname
+        self.mac = mac
+
+    def __str__(self):
+        return "Can not set mac %s to interface %s" % (self.mac, self.ifname)
+
+
+class HwAddrGetError(NetError):
+    def __init__(self, ifname):
+        NetError.__init__(self, ifname)
+        self.ifname = ifname
+
+    def __str__(self):
+        return "Can not get mac of interface %s" % self.ifname
+
+
 class Env(UserDict.IterableUserDict):
     """
     A dict-like object containing global objects used by tests.
@@ -2307,3 +2392,136 @@ def install_host_kernel(job, params):
     else:
         logging.info('Chose %s, using the current kernel for the host',
                      install_type)
+
+
+def bridge_auto_detect():
+    """
+    Automatically detect a bridge for tap through brctl.
+    """
+    try:
+        brctl_output = utils.system_output("ip route list",
+                                           retain_output=True)
+        brname = re.findall("default.*dev (.*) ", brctl_output)[0]
+    except:
+        raise BRAutoDetectError
+    return brname
+
+
+def if_nametoindex(ifname):
+    """
+    Map an interface name into its corresponding index.
+    Returns 0 on error, as 0 is not a valid index
+
+    @param ifname: interface name
+    """
+    index = 0
+    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
+    ifr = struct.pack("16si", ifname, 0)
+    r = fcntl.ioctl(ctrl_sock, SIOCGIFINDEX, ifr)
+    index = struct.unpack("16si", r)[1]
+    ctrl_sock.close()
+    return index
+
+
+def vnet_hdr_probe(tapfd):
+    """
+    Check if the IFF_VNET_HDR is support by tun.
+
+    @param tapfd: the file descriptor of /dev/net/tun
+    """
+    u = struct.pack("I", 0)
+    r = fcntl.ioctl(tapfd, TUNGETFEATURES, u)
+    flags = struct.unpack("I", r)[0]
+    if flags & IFF_VNET_HDR:
+        return True
+    else:
+        return False
+
+
+def open_tap(devname, ifname, vnet_hdr=True):
+    """
+    Open a tap device and returns its file descriptor which is used by
+    fd=<fd> parameter of qemu-kvm.
+
+    @param ifname: TAP interface name
+    @param vnet_hdr: Whether enable the vnet header
+    """
+    try:
+        tapfd = os.open(devname, os.O_RDWR)
+    except OSError:
+        raise TAPModuleError(devname)
+    flags = IFF_TAP | IFF_NO_PI
+    if vnet_hdr and vnet_hdr_probe(tapfd):
+        flags |= IFF_VNET_HDR
+
+    ifr = struct.pack("16sh", ifname, flags)
+    try:
+        r = fcntl.ioctl(tapfd, TUNSETIFF, ifr)
+    except IOError:
+        raise TAPCreationError(ifname)
+    ifname = struct.unpack("16sh", r)[0].strip("\x00")
+    return tapfd
+
+
+def add_to_bridge(ifname, brname):
+    """
+    Add a TAP device to bridge
+
+    @param ifname: Name of TAP device
+    @param brname: Name of the bridge
+    """
+    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
+    index = if_nametoindex(ifname)
+    if index == 0:
+        raise TAPNotExistError(ifname)
+    ifr = struct.pack("16si", brname, index)
+    try:
+        r = fcntl.ioctl(ctrl_sock, SIOCBRADDIF, ifr)
+    except IOError, details:
+        raise BRAddIfError(ifname, brname, details)
+    ctrl_sock.close()
+
+
+def bring_up_ifname(ifname):
+    """
+    Bring up an interface
+
+    @param ifname: Name of the interface
+    """
+    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
+    ifr = struct.pack("16si", ifname, IFF_UP)
+    try:
+        fcntl.ioctl(ctrl_sock, SIOCSIFFLAGS, ifr)
+    except IOError:
+        raise TAPBringUpError(ifname)
+    ctrl_sock.close()
+
+
+def if_set_macaddress(ifname, mac):
+    """
+    Set the mac address for an interface
+
+    @param ifname: Name of the interface
+    @mac: Mac address
+    """
+    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
+
+    ifr = struct.pack("256s", ifname)
+    try:
+        mac_dev = fcntl.ioctl(ctrl_sock, SIOCGIFHWADDR, ifr)[18:24]
+        mac_dev = ":".join(["%02x" % ord(m) for m in mac_dev])
+    except IOError, e:
+        raise HwAddrGetError(ifname)
+
+    if mac_dev.lower() == mac.lower():
+        return
+
+    ifr = struct.pack("16sH14s", ifname, 1,
+                      "".join([chr(int(m, 16)) for m in mac.split(":")]))
+    try:
+        fcntl.ioctl(ctrl_sock, SIOCSIFHWADDR, ifr)
+    except IOError, e:
+        logging.info(e)
+        raise HwAddrSetError(ifname, mac)
+    ctrl_sock.close()
+
-- 
1.7.5.1

Re: [PATCH 2/5] KVM test: Add helpers to control the TAP/bridge

[Amos Kong]

On Sat, May 21, 2011 at 01:23:27AM -0300, Lucas Meneghel Rodrigues wrote:
> This patch adds some helpers to assist virt test to setup the bridge or
> macvtap based guest networking.
> 
> Changes from v1:
>  * Fixed undefined variable errors on the exception class definitions
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
> ---
>  client/virt/virt_utils.py |  218 +++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 218 insertions(+), 0 deletions(-)
> 
> diff --git a/client/virt/virt_utils.py b/client/virt/virt_utils.py
> index 5510c89..96b9c84 100644
> --- a/client/virt/virt_utils.py
> +++ b/client/virt/virt_utils.py
> @@ -6,6 +6,7 @@ KVM test utility functions.
>  
>  import time, string, random, socket, os, signal, re, logging, commands, cPickle
>  import fcntl, shelve, ConfigParser, threading, sys, UserDict, inspect
> +import struct
>  from autotest_lib.client.bin import utils, os_dep
>  from autotest_lib.client.common_lib import error, logging_config
>  import rss_client, aexpect
> @@ -15,6 +16,20 @@ try:
>  except ImportError:
>      KOJI_INSTALLED = False
>  
> +# From include/linux/sockios.h
> +SIOCSIFHWADDR = 0x8924
> +SIOCGIFHWADDR = 0x8927
> +SIOCSIFFLAGS = 0x8914
> +SIOCGIFINDEX = 0x8933
> +SIOCBRADDIF = 0x89a2
> +# From linux/include/linux/if_tun.h
> +TUNSETIFF = 0x400454ca
> +TUNGETIFF = 0x800454d2
> +TUNGETFEATURES = 0x800454cf
> +IFF_UP = 0x1
> +IFF_TAP = 0x0002
> +IFF_NO_PI = 0x1000
> +IFF_VNET_HDR = 0x4000
>  
>  def _lock_file(filename):
>      f = open(filename, "w")
> @@ -36,6 +51,76 @@ def is_vm(obj):
>      return obj.__class__.__name__ == "VM"
>  
>  
> +class NetError(Exception):
> +    pass
> +
> +
> +class TAPModuleError(NetError):
> +    def __init__(self, devname):
> +        NetError.__init__(self, devname)
> +        self.devname = devname
> +
> +    def __str__(self):
> +        return "Can't open %s" % self.devname
> +
> +class TAPNotExistError(NetError):
> +    def __init__(self, ifname):
> +        NetError.__init__(self, ifname)
> +        self.ifname = ifname
> +
> +    def __str__(self):
> +        return "Interface %s does not exist" % self.ifname
> +
> +
> +class TAPCreationError(NetError):
> +    def __init__(self, ifname):
> +        NetError.__init__(self, ifname)
> +        self.ifname = ifname
> +
> +    def __str__(self):
> +        return "Cannot create TAP device %s" % self.ifname
> +
> +
> +class TAPBringUpError(NetError):
> +    def __init__(self, ifname):
> +        NetError.__init__(self, ifname)
> +        self.ifname = ifname
> +
> +    def __str__(self):
> +        return "Cannot bring up TAP %s" % self.ifname
> +
> +
> +class BRAddIfError(NetError):
> +    def __init__(self, ifname, brname, details):
> +        NetError.__init__(self, ifname, brname, details)
> +        self.ifname = ifname
> +        self.brname = brname
> +        self.details = details
> +
> +    def __str__(self):
> +        return ("Can not add if %s to bridge %s: %s" %
> +                (self.ifname, self.brname, self.details))
> +
> +
> +class HwAddrSetError(NetError):
> +    def __init__(self, ifname, mac):
> +        NetError.__init__(self, ifname, mac)
> +        self.ifname = ifname
> +        self.mac = mac
> +
> +    def __str__(self):
> +        return "Can not set mac %s to interface %s" % (self.mac, self.ifname)
> +
> +
> +class HwAddrGetError(NetError):
> +    def __init__(self, ifname):
> +        NetError.__init__(self, ifname)
> +        self.ifname = ifname
> +
> +    def __str__(self):
> +        return "Can not get mac of interface %s" % self.ifname
> +
> +
>  class Env(UserDict.IterableUserDict):
>      """
>      A dict-like object containing global objects used by tests.
> @@ -2307,3 +2392,136 @@ def install_host_kernel(job, params):
>      else:
>          logging.info('Chose %s, using the current kernel for the host',
>                       install_type)
> +
> +
> +def bridge_auto_detect():
> +    """
> +    Automatically detect a bridge for tap through brctl.
> +    """
> +    try:
> +        brctl_output = utils.system_output("ip route list",
> +                                           retain_output=True)
> +        brname = re.findall("default.*dev (.*) ", brctl_output)[0]
> +    except:
> +        raise BRAutoDetectError
> +    return brname
> +
> +
> +def if_nametoindex(ifname):
> +    """
> +    Map an interface name into its corresponding index.
> +    Returns 0 on error, as 0 is not a valid index
> +
> +    @param ifname: interface name
> +    """
> +    index = 0
> +    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
> +    ifr = struct.pack("16si", ifname, 0)
> +    r = fcntl.ioctl(ctrl_sock, SIOCGIFINDEX, ifr)
> +    index = struct.unpack("16si", r)[1]
> +    ctrl_sock.close()
> +    return index
> +
> +
> +def vnet_hdr_probe(tapfd):
> +    """
> +    Check if the IFF_VNET_HDR is support by tun.
> +
> +    @param tapfd: the file descriptor of /dev/net/tun
> +    """
> +    u = struct.pack("I", 0)
> +    r = fcntl.ioctl(tapfd, TUNGETFEATURES, u)
> +    flags = struct.unpack("I", r)[0]
> +    if flags & IFF_VNET_HDR:
> +        return True
> +    else:
> +        return False

# ../../bin/autotest control --verbose --args="kill_vm=yes only=boot iterations=2"

05/23 13:24:41 INFO |      test:0286| Test started. Number of iterations: 2
05/23 13:24:41 INFO |      test:0289| Executing iteration 1 of 2
_PASS_

05/23 13:25:36 INFO |      test:0289| Executing iteration 2 of 2
...
  File "/project/rh/autotest-upstream/client/virt/virt_env_process.py", line 185, in process
    vm_func(test, vm_params, env, vm_name)
  File "/project/rh/autotest-upstream/client/virt/virt_env_process.py", line 82, in preprocess_vm
    migration_mode=params.get("migration_mode"))
  File "/project/rh/autotest-upstream/client/common_lib/error.py", line 138, in new_fn
    return fn(*args, **kwargs)
  File "/project/rh/autotest-upstream/client/virt/kvm_vm.py", line 643, in create
    tapfd = virt_utils.open_tap("/dev/net/tun", ifname)
  File "/project/rh/autotest-upstream/client/virt/virt_utils.py", line 2461, in open_tap
    raise TAPCreationError(ifname)
TAPCreationError: Cannot create TAP device t0-081158-LeCM


Tap device is not closed after iteration.1, iteration.2 fails of creating an existed tap device.
So we need a 'close_tap()' function, and call it in vm.destroy()

> +def open_tap(devname, ifname, vnet_hdr=True):
> +    """
> +    Open a tap device and returns its file descriptor which is used by
> +    fd=<fd> parameter of qemu-kvm.
> +
> +    @param ifname: TAP interface name
> +    @param vnet_hdr: Whether enable the vnet header
> +    """
> +    try:
> +        tapfd = os.open(devname, os.O_RDWR)
> +    except OSError:
> +        raise TAPModuleError(devname)
> +    flags = IFF_TAP | IFF_NO_PI
> +    if vnet_hdr and vnet_hdr_probe(tapfd):
> +        flags |= IFF_VNET_HDR
> +
> +    ifr = struct.pack("16sh", ifname, flags)
> +    try:
> +        r = fcntl.ioctl(tapfd, TUNSETIFF, ifr)
> +    except IOError:
> +        raise TAPCreationError(ifname)
> +    ifname = struct.unpack("16sh", r)[0].strip("\x00")
> +    return tapfd
> +
> +
> +def add_to_bridge(ifname, brname):
> +    """
> +    Add a TAP device to bridge
> +
> +    @param ifname: Name of TAP device
> +    @param brname: Name of the bridge
> +    """
> +    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
> +    index = if_nametoindex(ifname)
> +    if index == 0:
> +        raise TAPNotExistError(ifname)
> +    ifr = struct.pack("16si", brname, index)
> +    try:
> +        r = fcntl.ioctl(ctrl_sock, SIOCBRADDIF, ifr)
> +    except IOError, details:
> +        raise BRAddIfError(ifname, brname, details)
> +    ctrl_sock.close()
> +
> +
> +def bring_up_ifname(ifname):
> +    """
> +    Bring up an interface
> +
> +    @param ifname: Name of the interface
> +    """
> +    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
> +    ifr = struct.pack("16si", ifname, IFF_UP)
> +    try:
> +        fcntl.ioctl(ctrl_sock, SIOCSIFFLAGS, ifr)
> +    except IOError:
> +        raise TAPBringUpError(ifname)
> +    ctrl_sock.close()
> +
> +
> +def if_set_macaddress(ifname, mac):
> +    """
> +    Set the mac address for an interface
> +
> +    @param ifname: Name of the interface
> +    @mac: Mac address
> +    """
> +    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
> +
> +    ifr = struct.pack("256s", ifname)
> +    try:
> +        mac_dev = fcntl.ioctl(ctrl_sock, SIOCGIFHWADDR, ifr)[18:24]
> +        mac_dev = ":".join(["%02x" % ord(m) for m in mac_dev])
> +    except IOError, e:
> +        raise HwAddrGetError(ifname)
> +
> +    if mac_dev.lower() == mac.lower():
> +        return
> +
> +    ifr = struct.pack("16sH14s", ifname, 1,
> +                      "".join([chr(int(m, 16)) for m in mac.split(":")]))
> +    try:
> +        fcntl.ioctl(ctrl_sock, SIOCSIFHWADDR, ifr)
> +    except IOError, e:
> +        logging.info(e)
> +        raise HwAddrSetError(ifname, mac)
> +    ctrl_sock.close()
> +
> -- 
> 1.7.5.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] KVM test: Add helpers to control the TAP/bridge

[Lucas Meneghel Rodrigues]

On Mon, 2011-05-23 at 14:16 +0800, Amos Kong wrote:
> On Sat, May 21, 2011 at 01:23:27AM -0300, Lucas Meneghel Rodrigues wrote:
> > This patch adds some helpers to assist virt test to setup the bridge or
> > macvtap based guest networking.
> > 
> > Changes from v1:
> >  * Fixed undefined variable errors on the exception class definitions
> > 
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
> > ---
> >  client/virt/virt_utils.py |  218 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 218 insertions(+), 0 deletions(-)
> > 
> > diff --git a/client/virt/virt_utils.py b/client/virt/virt_utils.py
> > index 5510c89..96b9c84 100644
> > --- a/client/virt/virt_utils.py
> > +++ b/client/virt/virt_utils.py
> > @@ -6,6 +6,7 @@ KVM test utility functions.
> >  
> >  import time, string, random, socket, os, signal, re, logging, commands, cPickle
> >  import fcntl, shelve, ConfigParser, threading, sys, UserDict, inspect
> > +import struct
> >  from autotest_lib.client.bin import utils, os_dep
> >  from autotest_lib.client.common_lib import error, logging_config
> >  import rss_client, aexpect
> > @@ -15,6 +16,20 @@ try:
> >  except ImportError:
> >      KOJI_INSTALLED = False
> >  
> > +# From include/linux/sockios.h
> > +SIOCSIFHWADDR = 0x8924
> > +SIOCGIFHWADDR = 0x8927
> > +SIOCSIFFLAGS = 0x8914
> > +SIOCGIFINDEX = 0x8933
> > +SIOCBRADDIF = 0x89a2
> > +# From linux/include/linux/if_tun.h
> > +TUNSETIFF = 0x400454ca
> > +TUNGETIFF = 0x800454d2
> > +TUNGETFEATURES = 0x800454cf
> > +IFF_UP = 0x1
> > +IFF_TAP = 0x0002
> > +IFF_NO_PI = 0x1000
> > +IFF_VNET_HDR = 0x4000
> >  
> >  def _lock_file(filename):
> >      f = open(filename, "w")
> > @@ -36,6 +51,76 @@ def is_vm(obj):
> >      return obj.__class__.__name__ == "VM"
> >  
> >  
> > +class NetError(Exception):
> > +    pass
> > +
> > +
> > +class TAPModuleError(NetError):
> > +    def __init__(self, devname):
> > +        NetError.__init__(self, devname)
> > +        self.devname = devname
> > +
> > +    def __str__(self):
> > +        return "Can't open %s" % self.devname
> > +
> > +class TAPNotExistError(NetError):
> > +    def __init__(self, ifname):
> > +        NetError.__init__(self, ifname)
> > +        self.ifname = ifname
> > +
> > +    def __str__(self):
> > +        return "Interface %s does not exist" % self.ifname
> > +
> > +
> > +class TAPCreationError(NetError):
> > +    def __init__(self, ifname):
> > +        NetError.__init__(self, ifname)
> > +        self.ifname = ifname
> > +
> > +    def __str__(self):
> > +        return "Cannot create TAP device %s" % self.ifname
> > +
> > +
> > +class TAPBringUpError(NetError):
> > +    def __init__(self, ifname):
> > +        NetError.__init__(self, ifname)
> > +        self.ifname = ifname
> > +
> > +    def __str__(self):
> > +        return "Cannot bring up TAP %s" % self.ifname
> > +
> > +
> > +class BRAddIfError(NetError):
> > +    def __init__(self, ifname, brname, details):
> > +        NetError.__init__(self, ifname, brname, details)
> > +        self.ifname = ifname
> > +        self.brname = brname
> > +        self.details = details
> > +
> > +    def __str__(self):
> > +        return ("Can not add if %s to bridge %s: %s" %
> > +                (self.ifname, self.brname, self.details))
> > +
> > +
> > +class HwAddrSetError(NetError):
> > +    def __init__(self, ifname, mac):
> > +        NetError.__init__(self, ifname, mac)
> > +        self.ifname = ifname
> > +        self.mac = mac
> > +
> > +    def __str__(self):
> > +        return "Can not set mac %s to interface %s" % (self.mac, self.ifname)
> > +
> > +
> > +class HwAddrGetError(NetError):
> > +    def __init__(self, ifname):
> > +        NetError.__init__(self, ifname)
> > +        self.ifname = ifname
> > +
> > +    def __str__(self):
> > +        return "Can not get mac of interface %s" % self.ifname
> > +
> > +
> >  class Env(UserDict.IterableUserDict):
> >      """
> >      A dict-like object containing global objects used by tests.
> > @@ -2307,3 +2392,136 @@ def install_host_kernel(job, params):
> >      else:
> >          logging.info('Chose %s, using the current kernel for the host',
> >                       install_type)
> > +
> > +
> > +def bridge_auto_detect():
> > +    """
> > +    Automatically detect a bridge for tap through brctl.
> > +    """
> > +    try:
> > +        brctl_output = utils.system_output("ip route list",
> > +                                           retain_output=True)
> > +        brname = re.findall("default.*dev (.*) ", brctl_output)[0]
> > +    except:
> > +        raise BRAutoDetectError
> > +    return brname
> > +
> > +
> > +def if_nametoindex(ifname):
> > +    """
> > +    Map an interface name into its corresponding index.
> > +    Returns 0 on error, as 0 is not a valid index
> > +
> > +    @param ifname: interface name
> > +    """
> > +    index = 0
> > +    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
> > +    ifr = struct.pack("16si", ifname, 0)
> > +    r = fcntl.ioctl(ctrl_sock, SIOCGIFINDEX, ifr)
> > +    index = struct.unpack("16si", r)[1]
> > +    ctrl_sock.close()
> > +    return index
> > +
> > +
> > +def vnet_hdr_probe(tapfd):
> > +    """
> > +    Check if the IFF_VNET_HDR is support by tun.
> > +
> > +    @param tapfd: the file descriptor of /dev/net/tun
> > +    """
> > +    u = struct.pack("I", 0)
> > +    r = fcntl.ioctl(tapfd, TUNGETFEATURES, u)
> > +    flags = struct.unpack("I", r)[0]
> > +    if flags & IFF_VNET_HDR:
> > +        return True
> > +    else:
> > +        return False
> 
> # ../../bin/autotest control --verbose --args="kill_vm=yes only=boot iterations=2"
> 
> 05/23 13:24:41 INFO |      test:0286| Test started. Number of iterations: 2
> 05/23 13:24:41 INFO |      test:0289| Executing iteration 1 of 2
> _PASS_
> 
> 05/23 13:25:36 INFO |      test:0289| Executing iteration 2 of 2
> ...
>   File "/project/rh/autotest-upstream/client/virt/virt_env_process.py", line 185, in process
>     vm_func(test, vm_params, env, vm_name)
>   File "/project/rh/autotest-upstream/client/virt/virt_env_process.py", line 82, in preprocess_vm
>     migration_mode=params.get("migration_mode"))
>   File "/project/rh/autotest-upstream/client/common_lib/error.py", line 138, in new_fn
>     return fn(*args, **kwargs)
>   File "/project/rh/autotest-upstream/client/virt/kvm_vm.py", line 643, in create
>     tapfd = virt_utils.open_tap("/dev/net/tun", ifname)
>   File "/project/rh/autotest-upstream/client/virt/virt_utils.py", line 2461, in open_tap
>     raise TAPCreationError(ifname)
> TAPCreationError: Cannot create TAP device t0-081158-LeCM
> 
> 
> Tap device is not closed after iteration.1, iteration.2 fails of creating an existed tap device.
> So we need a 'close_tap()' function, and call it in vm.destroy()

Oh, OK, let's do this.

Thanks for having noticed that, Amos!

> > +def open_tap(devname, ifname, vnet_hdr=True):
> > +    """
> > +    Open a tap device and returns its file descriptor which is used by
> > +    fd=<fd> parameter of qemu-kvm.
> > +
> > +    @param ifname: TAP interface name
> > +    @param vnet_hdr: Whether enable the vnet header
> > +    """
> > +    try:
> > +        tapfd = os.open(devname, os.O_RDWR)
> > +    except OSError:
> > +        raise TAPModuleError(devname)
> > +    flags = IFF_TAP | IFF_NO_PI
> > +    if vnet_hdr and vnet_hdr_probe(tapfd):
> > +        flags |= IFF_VNET_HDR
> > +
> > +    ifr = struct.pack("16sh", ifname, flags)
> > +    try:
> > +        r = fcntl.ioctl(tapfd, TUNSETIFF, ifr)
> > +    except IOError:
> > +        raise TAPCreationError(ifname)
> > +    ifname = struct.unpack("16sh", r)[0].strip("\x00")
> > +    return tapfd
> > +
> > +
> > +def add_to_bridge(ifname, brname):
> > +    """
> > +    Add a TAP device to bridge
> > +
> > +    @param ifname: Name of TAP device
> > +    @param brname: Name of the bridge
> > +    """
> > +    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
> > +    index = if_nametoindex(ifname)
> > +    if index == 0:
> > +        raise TAPNotExistError(ifname)
> > +    ifr = struct.pack("16si", brname, index)
> > +    try:
> > +        r = fcntl.ioctl(ctrl_sock, SIOCBRADDIF, ifr)
> > +    except IOError, details:
> > +        raise BRAddIfError(ifname, brname, details)
> > +    ctrl_sock.close()
> > +
> > +
> > +def bring_up_ifname(ifname):
> > +    """
> > +    Bring up an interface
> > +
> > +    @param ifname: Name of the interface
> > +    """
> > +    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
> > +    ifr = struct.pack("16si", ifname, IFF_UP)
> > +    try:
> > +        fcntl.ioctl(ctrl_sock, SIOCSIFFLAGS, ifr)
> > +    except IOError:
> > +        raise TAPBringUpError(ifname)
> > +    ctrl_sock.close()
> > +
> > +
> > +def if_set_macaddress(ifname, mac):
> > +    """
> > +    Set the mac address for an interface
> > +
> > +    @param ifname: Name of the interface
> > +    @mac: Mac address
> > +    """
> > +    ctrl_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
> > +
> > +    ifr = struct.pack("256s", ifname)
> > +    try:
> > +        mac_dev = fcntl.ioctl(ctrl_sock, SIOCGIFHWADDR, ifr)[18:24]
> > +        mac_dev = ":".join(["%02x" % ord(m) for m in mac_dev])
> > +    except IOError, e:
> > +        raise HwAddrGetError(ifname)
> > +
> > +    if mac_dev.lower() == mac.lower():
> > +        return
> > +
> > +    ifr = struct.pack("16sH14s", ifname, 1,
> > +                      "".join([chr(int(m, 16)) for m in mac.split(":")]))
> > +    try:
> > +        fcntl.ioctl(ctrl_sock, SIOCSIFHWADDR, ifr)
> > +    except IOError, e:
> > +        logging.info(e)
> > +        raise HwAddrSetError(ifname, mac)
> > +    ctrl_sock.close()
> > +
> > -- 
> > 1.7.5.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] KVM test: virt_env_process: Setup private bridge during postprocessing

[Lucas Meneghel Rodrigues]

Call bridge setup at preprocessing and cleanup at postprocessing.
The bridge can be cleaned up when no tap interfaces are using it.

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/virt/virt_env_process.py |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/client/virt/virt_env_process.py b/client/virt/virt_env_process.py
index 625b422..1742a02 100644
--- a/client/virt/virt_env_process.py
+++ b/client/virt/virt_env_process.py
@@ -196,6 +196,11 @@ def preprocess(test, params, env):
     @param env: The environment (a dict-like object).
     """
     error.context("preprocessing")
+
+    if params.get("bridge") == "private":
+        brcfg = virt_test_setup.PrivateBridgeConfig(params)
+        brcfg.setup()
+
     # Start tcpdump if it isn't already running
     if "address_cache" not in env:
         env["address_cache"] = {}
@@ -365,6 +370,10 @@ def postprocess(test, params, env):
                         int(params.get("post_command_timeout", "600")),
                         params.get("post_command_noncritical") == "yes")
 
+    if params.get("bridge") == "private":
+        brcfg = virt_test_setup.PrivateBridgeConfig()
+        brcfg.cleanup()
+
 
 def postprocess_on_error(test, params, env):
     """
-- 
1.7.5.1

[PATCH 4/5] KVM test: setup tap fd and pass it to qemu-kvm

[Lucas Meneghel Rodrigues]

We used to use qemu-ifup to manage the tap which have several
limitations:

1) If we want to specify a bridge, we must create a customized
qemu-ifup file as the default script always match the first bridge.
2) It's hard to add support for macvtap device.

So this patch let kvm subtest control the tap creation and setup then
pass it to qemu-kvm. User could specify the bridge he want to used in
configuration file.

The original autoconfiguration was changed by private bridge setup.

Changes from v1:
* Combine the private bridge config and TAP fd in one patchset,
dropped the "auto" mode

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/tests/kvm/scripts/qemu-ifup |   11 -------
 client/virt/kvm_vm.py              |   57 +++++++++++++++++++++++------------
 2 files changed, 37 insertions(+), 31 deletions(-)
 delete mode 100755 client/tests/kvm/scripts/qemu-ifup

diff --git a/client/tests/kvm/scripts/qemu-ifup b/client/tests/kvm/scripts/qemu-ifup
deleted file mode 100755
index c4debf5..0000000
--- a/client/tests/kvm/scripts/qemu-ifup
+++ /dev/null
@@ -1,11 +0,0 @@
-#!/bin/sh
-
-# The following expression selects the first bridge listed by 'brctl show'.
-# Modify it to suit your needs.
-switch=$(/usr/sbin/brctl show | awk 'NR==2 { print $1 }')
-
-/bin/echo 1 > /proc/sys/net/ipv6/conf/${switch}/disable_ipv6
-/sbin/ifconfig $1 0.0.0.0 up
-/usr/sbin/brctl addif ${switch} $1
-/usr/sbin/brctl setfd ${switch} 0
-/usr/sbin/brctl stp ${switch} off
diff --git a/client/virt/kvm_vm.py b/client/virt/kvm_vm.py
index 57fc61b..d925fe7 100644
--- a/client/virt/kvm_vm.py
+++ b/client/virt/kvm_vm.py
@@ -7,7 +7,7 @@ Utility classes and functions to handle Virtual Machine creation using qemu.
 import time, os, logging, fcntl, re, commands, glob
 from autotest_lib.client.common_lib import error
 from autotest_lib.client.bin import utils
-import virt_utils, virt_vm, kvm_monitor, aexpect
+import virt_utils, virt_vm, virt_test_setup, kvm_monitor, aexpect
 
 
 class VM(virt_vm.BaseVM):
@@ -41,6 +41,7 @@ class VM(virt_vm.BaseVM):
             self.pci_assignable = None
             self.netdev_id = []
             self.device_id = []
+            self.tapfds = []
             self.uuid = None
 
 
@@ -231,19 +232,17 @@ class VM(virt_vm.BaseVM):
                 cmd += ",id='%s'" % device_id
             return cmd
 
-        def add_net(help, vlan, mode, ifname=None, script=None,
-                    downscript=None, tftp=None, bootfile=None, hostfwd=[],
-                    netdev_id=None, netdev_extra_params=None):
+        def add_net(help, vlan, mode, ifname=None, tftp=None, bootfile=None,
+                    hostfwd=[], netdev_id=None, netdev_extra_params=None,
+                    tapfd=None):
             if has_option(help, "netdev"):
                 cmd = " -netdev %s,id=%s" % (mode, netdev_id)
                 if netdev_extra_params:
                     cmd += ",%s" % netdev_extra_params
             else:
                 cmd = " -net %s,vlan=%d" % (mode, vlan)
-            if mode == "tap":
-                if ifname: cmd += ",ifname='%s'" % ifname
-                if script: cmd += ",script='%s'" % script
-                cmd += ",downscript='%s'" % (downscript or "no")
+            if mode == "tap" and tapfd:
+                cmd += ",fd=%d" % tapfd
             elif mode == "user":
                 if tftp and "[,tftp=" in help:
                     cmd += ",tftp='%s'" % tftp
@@ -413,20 +412,22 @@ class VM(virt_vm.BaseVM):
             qemu_cmd += add_nic(help, vlan, nic_params.get("nic_model"), mac,
                                 device_id, netdev_id, nic_params.get("nic_extra_params"))
             # Handle the '-net tap' or '-net user' or '-netdev' part
-            script = nic_params.get("nic_script")
-            downscript = nic_params.get("nic_downscript")
             tftp = nic_params.get("tftp")
-            if script:
-                script = virt_utils.get_path(root_dir, script)
-            if downscript:
-                downscript = virt_utils.get_path(root_dir, downscript)
             if tftp:
                 tftp = virt_utils.get_path(root_dir, tftp)
-            qemu_cmd += add_net(help, vlan, nic_params.get("nic_mode", "user"),
-                                vm.get_ifname(vlan),
-                                script, downscript, tftp,
+            if nic_params.get("nic_mode") == "tap":
+                try:
+                    tapfd = vm.tapfds[vlan]
+                except IndexError:
+                    tapfd = None
+            else:
+                tapfd = None
+            qemu_cmd += add_net(help, vlan,
+                                nic_params.get("nic_mode", "user"),
+                                vm.get_ifname(vlan), tftp,
                                 nic_params.get("bootp"), redirs, netdev_id,
-                                nic_params.get("netdev_extra_params"))
+                                nic_params.get("netdev_extra_params"),
+                                tapfd)
             # Proceed to next NIC
             vlan += 1
 
@@ -549,6 +550,10 @@ class VM(virt_vm.BaseVM):
         @raise VMBadPATypeError: If an unsupported PCI assignment type is
                 requested
         @raise VMPAError: If no PCI assignable devices could be assigned
+        @raise TAPCreationError: If fail to create tap fd
+        @raise BRAddIfError: If fail to add a tap to a bridge
+        @raise TAPBringUpError: If fail to bring up a tap
+        @raise PrivateBridgeError: If fail to bring the private bridge
         """
         error.context("creating '%s'" % self.name)
         self.destroy(free_mac_addresses=False)
@@ -612,12 +617,24 @@ class VM(virt_vm.BaseVM):
                 guest_port = int(redir_params.get("guest_port"))
                 self.redirs[guest_port] = host_ports[i]
 
-            # Generate netdev/device IDs for all NICs
+            # Generate netdev IDs for all NICs and create TAP fd
             self.netdev_id = []
-            self.device_id = []
+            self.tapfds = []
+            vlan = 0
             for nic in params.objects("nics"):
                 self.netdev_id.append(virt_utils.generate_random_id())
                 self.device_id.append(virt_utils.generate_random_id())
+                nic_params = params.object_params(nic)
+                if nic_params.get("nic_mode") == "tap":
+                    ifname = self.get_ifname(vlan)
+                    brname = nic_params.get("bridge")
+                    if brname == "private":
+                        brname = virt_test_setup.PrivateBridgeConfig().brname
+                    tapfd = virt_utils.open_tap("/dev/net/tun", ifname)
+                    virt_utils.add_to_bridge(ifname, brname)
+                    virt_utils.bring_up_ifname(ifname)
+                    self.tapfds.append(tapfd)
+                vlan += 1
 
             # Find available VNC port, if needed
             if params.get("display") == "vnc":
-- 
1.7.5.1

[PATCH 5/5] KVM test: Changing KVM autotest default to private bridge

[Lucas Meneghel Rodrigues]

Rather than the unsupported userspace mode, which still is
an option. This way we are giving users a default setup
much closer to a real world usage scenario, and enable
people to run all the network tests that don't work properly
in user mode.

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/tests/kvm/tests_base.cfg.sample |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
index 78c84c6..cbded0a 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -57,10 +57,13 @@ redirs = remote_shell
 guest_port_remote_shell = 22
 
 # NIC parameters
-nic_mode = user
-#nic_mode = tap
-nic_script = scripts/qemu-ifup
-#nic_script = scripts/qemu-ifup-ipv6
+#nic_mode = user
+nic_mode = tap
+bridge = private
+# You can set bridge to
+# be a specific bridge
+# name, such as 'virbr0'
+#bridge = virbr0
 run_tcpdump = yes
 
 # Misc
@@ -117,7 +120,6 @@ variants:
             # Install guest from cdrom 
             - cdrom:
                 medium = cdrom
-                nic_mode = user
                 redirs += " unattended_install"
             # Install guest from http/ftp url
             - url:
-- 
1.7.5.1