autotest: Make WiFiClient an instance of LinuxSystem

This lets us consolidate some packet capture code and enables
us to use dual radio stumpies as both packet capture machines
and WiFiClients at the same time.

BUG=chromium:308214
TEST=wifi_matfunc still passes with these changes.
Taking packet captures with this modified WiFiClient still works.

Change-Id: Id143357f63be5ef2c8914cf0ff295408ab9b143b
Reviewed-on: https://chromium-review.googlesource.com/180097
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Christopher Wiley <wiley@chromium.org>
Commit-Queue: Christopher Wiley <wiley@chromium.org>
diff --git a/client/common_lib/cros/network/iw_runner.py b/client/common_lib/cros/network/iw_runner.py
index 85895b9..817646d 100644
--- a/client/common_lib/cros/network/iw_runner.py
+++ b/client/common_lib/cros/network/iw_runner.py
@@ -32,6 +32,8 @@
 IwBand = collections.namedtuple('Band', ['num', 'frequencies', 'mcs_indices'])
 IwBss = collections.namedtuple('IwBss', ['bss', 'frequency', 'ssid', 'security',
                                          'ht'])
+IwNetDev = collections.namedtuple('IwNetDev', ['phy', 'if_name', 'if_type'])
+
 # The fields for IwPhy are as follows:
 #   name: string name of the phy, such as "phy0"
 #   bands: list of IwBand objects.
@@ -156,10 +158,25 @@
         """@return list of string WiFi interface names on device."""
         output = self._run('%s dev' % self._command_iw).stdout
         interfaces = []
+        phy = None
+        if_name = None
+        if_type = None
         for line in output.splitlines():
+            m = re.match('phy#([0-9]+)', line)
+            if m:
+                phy = 'phy%d' % int(m.group(1))
             m = re.match('[\s]*Interface (.*)', line)
             if m:
-                interfaces.append(m.group(1))
+                if_name = m.group(1)
+            # Common values for type are 'managed', 'monitor', and 'IBSS'.
+            m = re.match('[\s]*type ([a-zA-Z]+)', line)
+            if m:
+                if_type = m.group(1)
+            if phy and if_name and if_type:
+                interfaces.append(IwNetDev(phy=phy, if_name=if_name,
+                                           if_type=if_type))
+                # One phy may have many interfaces, so don't reset it.
+                if_name = if_type = None
 
         return interfaces
 
diff --git a/client/common_lib/cros/network/shill_xmlrpc_server.py b/client/common_lib/cros/network/shill_xmlrpc_server.py
index 0077bb7..0f01b4d 100755
--- a/client/common_lib/cros/network/shill_xmlrpc_server.py
+++ b/client/common_lib/cros/network/shill_xmlrpc_server.py
@@ -181,7 +181,7 @@
 
             if len(interfaces) > 1:
                 logging.error('Defaulting to first interface of %r', interfaces)
-            wifi_if = interfaces[0]
+            wifi_if = interfaces[0].if_name
         if not self._wifi_proxy.configure_bgscan(
                 wifi_if,
                 method=params.bgscan_config.method,
diff --git a/server/cros/network/wifi_client.py b/server/cros/network/wifi_client.py
index db20041..bdf380d 100644
--- a/server/cros/network/wifi_client.py
+++ b/server/cros/network/wifi_client.py
@@ -9,19 +9,17 @@
 
 from autotest_lib.client.common_lib import error
 from autotest_lib.client.common_lib.cros.network import interface
-from autotest_lib.client.common_lib.cros.network import iw_runner
 from autotest_lib.client.common_lib.cros.network import ping_runner
 from autotest_lib.client.cros import constants
 from autotest_lib.server import autotest
 from autotest_lib.server import site_linux_system
 from autotest_lib.server.cros import remote_command
 from autotest_lib.server.cros import wifi_test_utils
-from autotest_lib.server.cros.network import packet_capturer
 from autotest_lib.server.cros.network import wpa_cli_proxy
 from autotest_lib.server.hosts import adb_host
 
 
-class WiFiClient(object):
+class WiFiClient(site_linux_system.LinuxSystem):
     """WiFiClient is a thin layer of logic over a remote DUT in wifitests."""
 
     XMLRPC_BRINGUP_TIMEOUT_SECONDS = 60
@@ -86,18 +84,6 @@
 
 
     @property
-    def capabilities(self):
-        """@return list of WiFi capabilities as parsed by LinuxSystem."""
-        return self._capabilities
-
-
-    @property
-    def host(self):
-        """@return host object representing the remote DUT."""
-        return self._host
-
-
-    @property
     def shill(self):
         """@return shill RPCProxy object."""
         return self._shill_proxy
@@ -212,7 +198,8 @@
         @param result_dir string directory to store test logs/packet caps.
 
         """
-        super(WiFiClient, self).__init__()
+        super(WiFiClient, self).__init__(client_host, {}, 'client',
+                                         inherit_interfaces=True)
         self._board = None
         self._command_ip = 'ip'
         self._command_iptables = 'iptables'
@@ -226,9 +213,8 @@
         self._ping_runner = ping_runner.PingRunner(host=self.host)
         self._ping_thread = None
         self._result_dir = result_dir
-        self._iw_runner = iw_runner.IwRunner(remote_host=self._host)
         # Look up the WiFi device (and its MAC) on the client.
-        devs = self._iw_runner.list_interfaces()
+        devs = self.iw_runner.list_interfaces()
         if not devs:
             raise error.TestFail('No wlan devices found on %s.' %
                                  self.host.hostname)
@@ -236,7 +222,7 @@
         if len(devs) > 1:
             logging.warning('Warning, found multiple WiFi devices on %s: %r',
                             self.host.hostname, devs)
-        self._wifi_if = devs[0]
+        self._wifi_if = devs[0].if_name
         self._interface = interface.Interface(self._wifi_if, host=self.host)
         if isinstance(self.host, adb_host.ADBHost):
             self._shill_proxy = wpa_cli_proxy.WpaCliProxy(
@@ -256,21 +242,11 @@
             # These commands aren't known to work with ADB hosts.
             self._command_ifconfig = 'ifconfig'
             self._raise_logging_level()
-        # Used for packet captures.
-        self._packet_capturer = packet_capturer.get_packet_capturer(
-                self.host, host_description='client', ignore_failures=True)
         self._result_dir = result_dir
 
         self._firewall_rules = []
         # Turn off powersave mode by default.
         self.powersave_switch(False)
-        # It is tempting to make WiFiClient a type of LinuxSystem, but most of
-        # the functionality there only makes sense for systems that want to
-        # manage their own WiFi interfaces.  On client devices however, shill
-        # does that work.
-        system = site_linux_system.LinuxSystem(self.host, {}, 'client')
-        self._capabilities = system.capabilities
-
         # All tests that use this object assume the interface starts enabled.
         self.set_device_enabled(self._wifi_if, True)
 
@@ -402,31 +378,6 @@
         self._firewall_rules = []
 
 
-    def start_capture(self, snaplen=None):
-        """Start a packet capture.
-
-        Attempt to start a host based OTA capture.  If the driver stack does
-        not support creating monitor interfaces, fall back to managed interface
-        packet capture.  Only one ongoing packet capture is supported at a time.
-
-        @param snaplen int number of byte to retain per captured frame.
-
-        """
-        self.stop_capture()
-        devname = self._packet_capturer.create_managed_monitor(self.wifi_if)
-        if devname is None:
-            logging.warning('Failure creating monitor interface; doing '
-                            'managed packet capture instead.')
-            devname = self.wifi_if
-        self._packet_capturer.start_capture(devname, self._result_dir,
-                                            snaplen=snaplen)
-
-
-    def stop_capture(self):
-        """Stop a packet capture and copy over the results."""
-        self._packet_capturer.close()
-
-
     def sync_host_times(self):
         """Set time on our DUT to match local time."""
         epoch_seconds = time.time()
@@ -450,8 +401,8 @@
 
 
     def get_iw_link_value(self, iw_link_key, ignore_failures=False):
-        return self._iw_runner.get_link_value(self.wifi_if, iw_link_key,
-                                              ignore_failures=ignore_failures)
+        return self.iw_runner.get_link_value(self.wifi_if, iw_link_key,
+                                             ignore_failures=ignore_failures)
 
 
     def powersave_switch(self, turn_on):
@@ -482,7 +433,7 @@
         """
         start_time = time.time()
         while time.time() - start_time < timeout_seconds:
-            bss_list = self._iw_runner.scan(
+            bss_list = self.iw_runner.scan(
                     self.wifi_if, frequencies=frequencies, ssids=ssids)
             if bss_list is not None:
                 break
diff --git a/server/cros/network/wifi_test_context_manager.py b/server/cros/network/wifi_test_context_manager.py
index 6a53705..3af19f9 100644
--- a/server/cros/network/wifi_test_context_manager.py
+++ b/server/cros/network/wifi_test_context_manager.py
@@ -157,7 +157,8 @@
             self.router.hostap_configure(configuration_parameters,
                                          multi_interface=multi_interface)
         if self._enable_client_packet_captures:
-            self.client.start_capture(snaplen=self._packet_capture_snaplen)
+            self.client.start_capture(configuration_parameters.frequency,
+                                      snaplen=self._packet_capture_snaplen)
         if self._enable_router_packet_captures:
             self.router.start_capture(
                     configuration_parameters.frequency,
diff --git a/server/site_linux_router.py b/server/site_linux_router.py
index 70b6655..9ca886f 100644
--- a/server/site_linux_router.py
+++ b/server/site_linux_router.py
@@ -66,7 +66,6 @@
                 'managed': 'pci'
             }})
         site_linux_system.LinuxSystem.__init__(self, host, params, 'router')
-        self._remove_interfaces()
 
         # Router host.
         self.router = host
@@ -167,9 +166,9 @@
         logging.info('Starting hostapd with parameters: %r',
                      hostapd_conf_dict)
         # Figure out the correct interface.
-        interface = self._get_wlanif(configuration.frequency,
-                                     self.phytype,
-                                     configuration.hw_mode)
+        interface = self.get_wlanif(configuration.frequency,
+                                    self.phytype,
+                                    configuration.hw_mode)
 
         conf_file = self.HOSTAPD_CONF_FILE_PATTERN % interface
         log_file = self.HOSTAPD_LOG_FILE_PATTERN % interface
@@ -376,8 +375,8 @@
         """
         if self.station['configured'] or self.hostapd_instances:
             self.deconfig()
-        interface = self._get_wlanif(config.frequency, self.phytype,
-                                     config.hw_mode)
+        interface = self.get_wlanif(config.frequency, self.phytype,
+                                    config.hw_mode)
         self.station['conf']['ssid'] = (config.ssid or
                                         self._build_ssid(config.ssid_suffix))
         # Connect the station
@@ -587,7 +586,7 @@
                 if silent:
                     # Deconfigure without notifying DUT.  Remove the interface
                     # hostapd uses to send beacon and DEAUTH packets.
-                    self._remove_interface(instance['interface'], True)
+                    self.remove_interface(instance['interface'])
 
                 self.kill_hostapd_instance(instance['conf_file'])
                 if wifi_test_utils.is_installed(self.host,
@@ -599,7 +598,7 @@
                 else:
                     logging.error('Did not collect hostapd log file because '
                                   'it was missing.')
-                self._release_wlanif(instance['interface'])
+                self.release_interface(instance['interface'])
 #               self.router.run("rm -f %(log_file)s %(conf_file)s" % instance)
             self._total_hostapd_instances += 1
         if self.station['configured']:
@@ -676,11 +675,11 @@
 
         """
         hostap_interface = self.hostapd_instances[instance]['interface']
-        interface = self._get_wlanif(0, 'monitor', same_phy_as=hostap_interface)
+        interface = self.get_wlanif(0, 'monitor', same_phy_as=hostap_interface)
         self.router.run("%s link set %s up" % (self.cmd_ip, interface))
         self.router.run('%s %s %s' %
                         (self.cmd_send_management_frame, interface, frame_type))
-        self._release_wlanif(interface)
+        self.release_interface(interface)
 
 
     def detect_client_deauth(self, client_mac, instance=0):
@@ -743,7 +742,7 @@
         hostap_conf = self.hostapd_instances[instance]['config_dict']
         frequency = hostap_config.HostapConfig.get_frequency_for_channel(
                 hostap_conf['channel'])
-        interface = self._get_wlanif(
+        interface = self.get_wlanif(
                 frequency, 'managed', hostap_conf['hw_mode'])
         client_conf['interface'] = interface
 
diff --git a/server/site_linux_system.py b/server/site_linux_system.py
index 4e015fb..1a286eb 100644
--- a/server/site_linux_system.py
+++ b/server/site_linux_system.py
@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 import datetime
+import collections
 import logging
 import time
 
@@ -11,6 +12,9 @@
 from autotest_lib.server.cros import wifi_test_utils
 from autotest_lib.server.cros.network import packet_capturer
 
+NetDev = collections.namedtuple('NetDev',
+                                ['inherited', 'phy', 'if_name', 'if_type'])
+
 class LinuxSystem(object):
     """Superclass for test machines running Linux.
 
@@ -40,7 +44,7 @@
         return self._capabilities
 
 
-    def __init__(self, host, params, role):
+    def __init__(self, host, params, role, inherit_interfaces=False):
         # Command locations.
         cmd_iw = wifi_test_utils.must_be_installed(
                 host, params.get('cmd_iw', '/usr/sbin/iw'))
@@ -56,8 +60,6 @@
         self.host = host
         self.role = role
 
-        self.capture_channel = None
-        self.capture_ht_type = None
         cmd_netdump = wifi_test_utils.get_install_path(
                 host, params.get('cmd_netdump', '/usr/sbin/tcpdump'))
         cmd_ifconfig = wifi_test_utils.get_install_path(
@@ -70,8 +72,18 @@
 
         self._phy_list = None
         self.phys_for_frequency, self.phy_bus_type = self._get_phy_info()
-        self.wlanifs_in_use = []
-        self.wlanifs = {}
+        self._interfaces = []
+        for interface in self.iw_runner.list_interfaces():
+            if inherit_interfaces:
+                self._interfaces.append(NetDev(inherited=True,
+                                               if_name=interface.if_name,
+                                               if_type=interface.if_type,
+                                               phy=interface.phy))
+            else:
+                self.iw_runner.remove_interface(interface.if_name)
+
+        self._wlanifs_in_use = []
+        self._capture_interface = None
         # Some uses of LinuxSystem don't use the interface allocation facility.
         # Don't force us to remove all the existing interfaces if this facility
         # is not desired.
@@ -129,41 +141,32 @@
         return phys_for_frequency, phy_bus_type
 
 
-    def _remove_interface(self, interface, remove_monitor):
+    def remove_interface(self, interface):
         """Remove an interface from a WiFi device.
 
         @param interface string interface to remove (e.g. wlan0).
-        @param remove_monitor bool True if we should also remove a monitor.
 
         """
+        self.release_interface(interface)
         self.host.run('%s link set %s down' % (self.cmd_ip, interface))
         self.iw_runner.remove_interface(interface)
-        if remove_monitor:
-            # Some old hostap implementations create a 'mon.<interface>' to
-            # handle management frame transmit/receive.
-            self.host.run('%s link set mon.%s down' % (self.cmd_ip, interface),
-                          ignore_status=True)
-            self.iw_runner.remove_interface('mon.%s' % interface,
-                                             ignore_status=True)
-        for phytype in self.wlanifs:
-            for phy in self.wlanifs[phytype]:
-                if self.wlanifs[phytype][phy] == interface:
-                    self.wlanifs[phytype].pop(phy)
-                    break
-
-
-    def _remove_interfaces(self):
-        """Remove all WiFi devices."""
-        for interface in self.iw_runner.list_interfaces():
-            self.iw_runner.remove_interface(interface)
-        self.wlanifs = {}
-        self._wlanifs_initialized = True
+        for net_dev in self._interfaces:
+            if net_dev.if_name == interface:
+                self._interfaces.remove(net_dev)
+                break
 
 
     def close(self):
         """Close global resources held by this system."""
         logging.debug('Cleaning up host object for %s', self.role)
         self._packet_capturer.close()
+        # Release and remove any interfaces that we create.
+        for net_dev in self._wlanifs_in_use:
+            self.release_interface(net_dev.if_name)
+        for net_dev in self._interfaces:
+            if net_dev.inherited:
+                continue
+            self.remove_interface(net_dev.if_name)
         self.host.close()
         self.host = None
 
@@ -185,36 +188,6 @@
         return caps
 
 
-    def start_capture_params(self, params):
-        """Start a packet capture.
-
-        Note that in |params|, 'channel' refers to the frequency of the WiFi
-        channel (e.g. 2412), not the channel number.
-
-        @param params dict of site_wifitest parameters.
-
-        """
-        if 'channel' in params:
-            self.capture_channel = int(params['channel'])
-        for arg in ('ht20', 'ht40+', 'ht40-'):
-            if arg in params:
-                self.capture_ht_type = arg.upper()
-
-        if not self.capture_channel:
-            raise error.TestError('No capture channel specified.')
-
-        self.start_capture(self.capture_channel, ht_type=self.capture_ht_type)
-
-
-    def stop_capture_params(self, params):
-        """Stop a packet capture.
-
-        @param params dict unused, but required by our dispatch method.
-
-        """
-        return self.stop_capture()
-
-
     def start_capture(self, frequency, ht_type=None, snaplen=None):
         """Start a packet capture.
 
@@ -225,14 +198,18 @@
         """
         if self._packet_capturer.capture_running:
             self.stop_capture()
-        # LinuxSystem likes to manage the phys on its own, so let it.
-        self.capture_interface = self._get_wlanif(frequency, 'monitor')
-        # But let the capturer configure the interface.
-        self._packet_capturer.configure_raw_monitor(self.capture_interface,
-                                                    frequency,
-                                                    ht_type=ht_type)
+        self._capture_interface = self.get_wlanif(frequency, 'monitor')
+        full_interface = [net_dev for net_dev in self._interfaces
+                          if net_dev.if_name == self._capture_interface][0]
+        # If this is the only interface on this phy, we ought to configure
+        # the phy with a channel and ht_type.  Otherwise, inherit the settings
+        # of the phy as they stand.
+        if len([net_dev for net_dev in self._interfaces
+                if net_dev.phy == full_interface.phy]) == 1:
+            self._packet_capturer.configure_raw_monitor(
+                    self._capture_interface, frequency, ht_type=ht_type)
         # Start the capture.
-        self._packet_capturer.start_capture(self.capture_interface, './debug/',
+        self._packet_capturer.start_capture(self._capture_interface, './debug/',
                                             snaplen=snaplen)
 
 
@@ -247,9 +224,8 @@
             return
         results = self._packet_capturer.stop_capture(
                 local_save_dir=save_dir, local_pcap_filename=save_filename)
-        self.host.run('%s link set %s down' % (self.cmd_ip,
-                                               self.capture_interface))
-        self._release_wlanif(self.capture_interface)
+        self.release_interface(self._capture_interface)
+        self._capture_interface = None
         return results
 
 
@@ -261,6 +237,7 @@
         self.host.run('date -u --set=@%s 2>/dev/null || date -u %s' %
                       (epoch_seconds, busybox_date))
 
+
     def _get_phy_for_frequency(self, frequency, phytype):
         """Get a phy appropriate for a frequency and phytype.
 
@@ -276,7 +253,7 @@
         """
         phys = self.phys_for_frequency[frequency]
 
-        busy_phys = set(phy for phy, wlanif, phytype in self.wlanifs_in_use)
+        busy_phys = set(net_dev.phy for net_dev in self._wlanifs_in_use)
         idle_phys = [phy for phy in phys if phy not in busy_phys]
         phys = idle_phys or phys
 
@@ -288,13 +265,9 @@
         return phys[0]
 
 
-    def _get_wlanif(self, frequency, phytype, mode = None, same_phy_as = None):
+    def get_wlanif(self, frequency, phytype, mode=None, same_phy_as=None):
         """Get a WiFi device that supports the given frequency, mode, and type.
 
-        This function is used by inherited classes, so we use the single '_'
-        convention rather than the '__' we usually use for non-scriptable
-        commands, since these cannot be inherited by subclasses.
-
         We still support the old "phydevN" parameters, but this code is
         smart enough to do without it.
 
@@ -306,9 +279,10 @@
 
         """
         if same_phy_as:
-            for phy, wlanif_i, phytype_i in self.wlanifs_in_use:
-                if wlanif_i == same_phy_as:
-                     break
+            for net_dev in self._interfaces:
+                if net_dev.if_name == same_phy_as:
+                    phy = net_dev.phy
+                    break
             else:
                 raise error.TestFail('Unable to find phy for interface %s' %
                                      same_phy_as)
@@ -322,31 +296,39 @@
             raise error.TestFail('Unable to find phy for frequency %d mode %s' %
                                  (frequency, mode))
 
-        if not self._wlanifs_initialized:
-            self._remove_interfaces()
-        if phytype not in self.wlanifs:
-            self.wlanifs[phytype] = {}
-        elif phy in self.wlanifs[phytype]:
-            return self.wlanifs[phytype][phy]
+        # If we have a suitable unused interface sitting around on this
+        # phy, reuse it.
+        for net_dev in set(self._interfaces) - set(self._wlanifs_in_use):
+            if net_dev.phy == phy and net_dev.if_type == phytype:
+                self._wlanifs_in_use.append(net_dev)
+                return net_dev.if_name
 
-        wlanif = '%s%d' % (phytype, len(self.wlanifs[phytype].keys()))
-        self.wlanifs[phytype][phy] = wlanif
-
-        self.iw_runner.add_interface(phy, wlanif, phytype)
-        self.wlanifs_in_use.append((phy, wlanif, phytype))
-
-        return wlanif
+        # Because we can reuse interfaces, we have to iteratively find a good
+        # interface name.
+        name_exists = lambda name: bool([net_dev
+                                         for net_dev in self._interfaces
+                                         if net_dev.if_name == name])
+        if_name = lambda index: '%s%d' % (phytype, index)
+        if_index = len(self._interfaces)
+        while name_exists(if_name(if_index)):
+            if_index += 1
+        net_dev = NetDev(phy=phy, if_name=if_name(if_index), if_type=phytype,
+                         inherited=False)
+        self._interfaces.append(net_dev)
+        self._wlanifs_in_use.append(net_dev)
+        self.iw_runner.add_interface(phy, net_dev.if_name, phytype)
+        return net_dev.if_name
 
 
-    def _release_wlanif(self, wlanif):
-        """Release a device allocated throuhg _get_wlanif().
+    def release_interface(self, wlanif):
+        """Release a device allocated throuhg get_wlanif().
 
         @param wlanif string name of device to release.
 
         """
-        for phy, wlanif_i, phytype in self.wlanifs_in_use:
-            if wlanif_i == wlanif:
-                 self.wlanifs_in_use.remove((phy, wlanif, phytype))
+        for net_dev in self._wlanifs_in_use:
+            if net_dev.if_name == wlanif:
+                 self._wlanifs_in_use.remove(net_dev)
 
 
     def require_capabilities(self, requirements, fatal_failure=False):