autotest: Refactoring and cleanup for LinuxRouter

In general, this patch tries to remove some of the invisible state and
opaque configuration built into the router.  As we've extended the
router to do more things, concepts like the "phytype" of the router no
longer make sense, because we can be multiple phytypes at once.
Fortunately, the phytype is easily inferred from the type of interface
we hope to configure.  We also had a number of interfaces to configure
various aspects of how the router worked that we never used and added
complexity.

Removed params dict for LinuxRouter/LinuxSystem.
Deprecated self.router field for LinuxRouter.
Removed LinuxRouter.create_wifi_device() since we now have
  hostapd/wpa_supplicant/ibss (phytype) specific method calls.
Removed LinuxRouter.destroy() method in favor of deconfig().

BUG=None
TEST=wifi_matfunc passes with these changes on a spring.

Change-Id: I1d77499b5e544813956e6f8018ca328c466046e8
Reviewed-on: https://chromium-review.googlesource.com/182636
Tested-by: Christopher Wiley <wiley@chromium.org>
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Commit-Queue: Christopher Wiley <wiley@chromium.org>
diff --git a/server/cros/network/wifi_client.py b/server/cros/network/wifi_client.py
index d14b656..c397d36 100644
--- a/server/cros/network/wifi_client.py
+++ b/server/cros/network/wifi_client.py
@@ -198,7 +198,7 @@
         @param result_dir string directory to store test logs/packet caps.
 
         """
-        super(WiFiClient, self).__init__(client_host, {}, 'client',
+        super(WiFiClient, self).__init__(client_host, 'client',
                                          inherit_interfaces=True)
         self._board = None
         self._command_ip = 'ip'
diff --git a/server/cros/network/wifi_test_context_manager.py b/server/cros/network/wifi_test_context_manager.py
index 3af19f9..bac3e25 100644
--- a/server/cros/network/wifi_test_context_manager.py
+++ b/server/cros/network/wifi_test_context_manager.py
@@ -173,13 +173,9 @@
         router_port = int(self._cmdline_args.get(self.CMDLINE_ROUTER_PORT, 22))
         logging.info('Connecting to router at %s:%d',
                      self.router_address, router_port)
-        # TODO(wiley) Simplify the router and make the parameters explicit.
-        router_params = {}
         self._router = site_linux_router.LinuxRouter(
                 hosts.SSHHost(self.router_address, port=router_port),
-                router_params, self._test_name)
-        # If we're testing WiFi, we're probably going to need one of these.
-        self._router.create_wifi_device()
+                self._test_name)
         # The '_server' is a machine which hosts network services, such as
         # OpenVPN or StrongSwan.  Note that we make a separate SSHHost instance
         # here because both the server and the router expect to close() their
diff --git a/server/site_linux_router.py b/server/site_linux_router.py
index 9defce5..669d178 100644
--- a/server/site_linux_router.py
+++ b/server/site_linux_router.py
@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import collections
 import logging
 import random
 import string
@@ -13,6 +14,11 @@
 from autotest_lib.server.cros import wifi_test_utils
 from autotest_lib.server.cros.network import hostap_config
 
+
+StationInstance = collections.namedtuple('StationInstance',
+                                         ['ssid', 'interface', 'dev_type'])
+
+
 class LinuxRouter(site_linux_system.LinuxSystem):
     """Linux/mac80211-style WiFi Router support for WiFiTest class.
 
@@ -44,31 +50,31 @@
         caps = set([self.CAPABILITY_IBSS])
         try:
             self.cmd_send_management_frame = wifi_test_utils.must_be_installed(
-                    self.router, '/usr/bin/send_management_frame')
+                    self.host, '/usr/bin/send_management_frame')
             caps.add(self.CAPABILITY_SEND_MANAGEMENT_FRAME)
         except error.TestFail:
             pass
         return super(LinuxRouter, self).get_capabilities().union(caps)
 
 
-    def __init__(self, host, params, test_name):
+    @property
+    def router(self):
+        """Deprecated.  Use self.host instead.
+
+        @return Host object representing the remote router.
+
+        """
+        return self.host
+
+
+    def __init__(self, host, test_name):
         """Build a LinuxRouter.
 
         @param host Host object representing the remote machine.
-        @param params dict of settings from site_wifitest based tests.
         @param test_name string name of this test.  Used in SSID creation.
 
         """
-        params = params.copy()
-        params.update({
-            'phy_bus_preference': {
-                'monitor': 'usb',
-                'managed': 'pci'
-            }})
-        site_linux_system.LinuxSystem.__init__(self, host, params, 'router')
-
-        # Router host.
-        self.router = host
+        super(LinuxRouter, self).__init__(host, 'router')
 
         self.cmd_dhcpd = '/usr/sbin/dhcpd'
         self.cmd_hostapd = wifi_test_utils.must_be_installed(
@@ -91,15 +97,9 @@
         self.ssid_prefix += '_'
 
         self._total_hostapd_instances = 0
-        self.station = {
-            'configured': False,
-            'config_file': "/tmp/wpa-supplicant-test-%s.conf",
-            'log_file': "/tmp/wpa-supplicant-test-%s.log",
-            'pid_file': "/tmp/wpa-supplicant-test-%s.pid",
-            'conf': {},
-        }
         self.local_servers = []
         self.hostapd_instances = []
+        self.station_instances = []
         self.dhcp_low = 1
         self.dhcp_high = 128
 
@@ -113,42 +113,8 @@
 
     def close(self):
         """Close global resources held by this system."""
-        self.destroy()
-        super(LinuxRouter, self).close()
-
-
-    def create_wifi_device(self, device_type='hostap'):
-        """Create a wifi device of the specified type.
-
-        Defaults to creating a hostap managed device.
-
-        @param device_type string device type.
-
-        """
-        #
-        # AP mode is handled entirely by hostapd so we only
-        # have to setup others (mapping the bsd type to what
-        # iw wants)
-        #
-        # map from bsd types to iw types
-        self.apmode = device_type in ('ap', 'hostap')
-        if not self.apmode:
-            self.station['type'] = device_type
-        self.phytype = {
-            'sta'       : 'managed',
-            'monitor'   : 'monitor',
-            'adhoc'     : 'adhoc',
-            'ibss'      : 'ibss',
-            'ap'        : 'managed',     # NB: handled by hostapd
-            'hostap'    : 'managed',     # NB: handled by hostapd
-            'mesh'      : 'mesh',
-            'wds'       : 'wds',
-        }[device_type]
-
-
-    def destroy(self):
-        """Destroy a previously created device."""
         self.deconfig()
+        super(LinuxRouter, self).close()
 
 
     def has_local_server(self):
@@ -166,9 +132,7 @@
         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, 'managed')
 
         conf_file = self.HOSTAPD_CONF_FILE_PATTERN % interface
         log_file = self.HOSTAPD_LOG_FILE_PATTERN % interface
@@ -299,7 +263,7 @@
 
         """
         if multi_interface is None and (self.hostapd_instances or
-                                        self.station['configured']):
+                                        self.station_instances):
             self.deconfig()
         # Start with the default hostapd config parameters.
         conf = self.__get_default_hostap_config()
@@ -373,21 +337,19 @@
         @param config HostapConfig object.
 
         """
-        if self.station['configured'] or self.hostapd_instances:
+        if self.station_instances or self.hostapd_instances:
             self.deconfig()
-        interface = self.get_wlanif(config.frequency, self.phytype,
-                                    config.hw_mode)
-        self.station['conf']['ssid'] = (config.ssid or
-                                        self._build_ssid(config.ssid_suffix))
+        interface = self.get_wlanif(config.frequency, 'ibss')
+        ssid = (config.ssid or self._build_ssid(config.ssid_suffix))
         # Connect the station
         self.router.run('%s link set %s up' % (self.cmd_ip, interface))
-        self.iw_runner.ibss_join(
-                interface, self.station['conf']['ssid'], config.frequency)
+        self.iw_runner.ibss_join(interface, ssid, config.frequency)
         # Always start a local server.
         self.start_local_server(interface)
         # Remember that this interface is up.
-        self.station['configured'] = True
-        self.station['interface'] = interface
+        self.station_instances.append(
+                StationInstance(ssid=ssid, interface=interface,
+                                dev_type='ibss'))
 
 
     def local_server_address(self, index):
@@ -420,8 +382,10 @@
         """Get the MAC address of the peer interface.
 
         @return string MAC address of the peer interface.
+
         """
-        iface = interface.Interface(self.station['interface'], self.router)
+        iface = interface.Interface(self.station_instances[0].interface,
+                                    self.router)
         return iface.mac_address
 
 
@@ -560,7 +524,7 @@
                 the DUT.
 
         """
-        if not self.hostapd_instances and not self.station['configured']:
+        if not self.hostapd_instances and not self.station_instances:
             return
 
         if self.hostapd_instances:
@@ -597,18 +561,19 @@
                 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']:
+        if self.station_instances:
             local_servers = self.local_servers
             self.local_servers = []
-            if self.station['type'] == 'ibss':
-                self.iw_runner.ibss_leave(self.station['interface'])
-            elif self.station['type'] == 'supplicant':
+            instance = self.station_instances.pop()
+            if instance.dev_type == 'ibss':
+                self.iw_runner.ibss_leave(instance.interface)
+            elif instance.dev_type == 'managed':
                 self._kill_process_instance('wpa_supplicant',
-                                            self.station['interface'])
+                                            instance.interface)
             else:
-                self.iw_runner.disconnect_station(self.station['interface'])
-            self.router.run("%s link set %s down" % (self.cmd_ip,
-                                                     self.station['interface']))
+                self.iw_runner.disconnect_station(instance.interface)
+            self.router.run('%s link set %s down' %
+                            (self.cmd_ip, instance.interface))
 
         for server in local_servers:
             self.stop_dhcp_server(server['interface'])
@@ -616,8 +581,6 @@
                             (self.cmd_ip, server['ip_params']),
                              ignore_status=True)
 
-        self.station['configured'] = False
-
 
     def confirm_pmksa_cache_use(self, instance=0):
         """Verify that the PMKSA auth was cached on a hostapd instance.
@@ -644,10 +607,10 @@
         if self.hostapd_instances:
             return self.hostapd_instances[instance]['ssid']
 
-        if not 'ssid' in self.station['conf']:
-            raise error.TestFail('Requested ssid of an unconfigured AP.')
+        if self.station_instances:
+            return self.station_instances[0].ssid
 
-        return self.station['conf']['ssid']
+        raise error.TestFail('Requested ssid of an unconfigured AP.')
 
 
     def deauth_client(self, client_mac):
@@ -729,18 +692,14 @@
         if not self.hostapd_instances:
             raise error.TestFail('Hostapd is not configured.')
 
-        if self.station['configured']:
+        if self.station_instances:
             raise error.TestFail('Station is already configured.')
 
-        client_conf = self.station['conf']
-        client_conf['ssid'] = self.get_ssid(instance)
-
+        ssid = self.get_ssid(instance)
         hostap_conf = self.hostapd_instances[instance]['config_dict']
         frequency = hostap_config.HostapConfig.get_frequency_for_channel(
                 hostap_conf['channel'])
-        interface = self.get_wlanif(
-                frequency, 'managed', hostap_conf['hw_mode'])
-        client_conf['interface'] = interface
+        interface = self.get_wlanif(frequency, 'managed')
 
         # TODO(pstew): Configure other bits like PSK, 802.11n if tests
         # require them...
@@ -748,7 +707,7 @@
                 'network={\n'
                 '  ssid="%(ssid)s"\n'
                 '  key_mgmt=NONE\n'
-                '}\n' % client_conf
+                '}\n' % {'ssid': ssid}
         )
 
         conf_file = self.STATION_CONF_FILE_PATTERN % interface
@@ -786,6 +745,6 @@
         self.router.run('echo 1 > /proc/sys/net/ipv4/conf/%s/arp_ignore' %
                         hostap_conf['interface'])
 
-        self.station['configured'] = True
-        self.station['type'] = 'supplicant'
-        self.station['interface'] = interface
+        self.station_instances.append(
+                StationInstance(ssid=ssid, interface=interface,
+                                dev_type='managed'))
diff --git a/server/site_linux_system.py b/server/site_linux_system.py
index 1a286eb..d20457f 100644
--- a/server/site_linux_system.py
+++ b/server/site_linux_system.py
@@ -44,30 +44,21 @@
         return self._capabilities
 
 
-    def __init__(self, host, params, role, inherit_interfaces=False):
+    def __init__(self, host, role, inherit_interfaces=False):
         # Command locations.
         cmd_iw = wifi_test_utils.must_be_installed(
-                host, params.get('cmd_iw', '/usr/sbin/iw'))
+                host, '/usr/sbin/iw')
         self.cmd_ip = wifi_test_utils.must_be_installed(
-                host, params.get('cmd_ip', '/usr/sbin/ip'))
+                host, '/usr/sbin/ip')
         self.cmd_readlink = '%s -l' % wifi_test_utils.must_be_installed(
-                host, params.get('cmd_readlink', '/bin/ls'))
-
-        self.phy_bus_preference = params.get('phy_bus_preference', {})
-        self.phydev2 = params.get('phydev2', None)
-        self.phydev5 = params.get('phydev5', None)
+                host, '/bin/ls')
 
         self.host = host
         self.role = role
 
-        cmd_netdump = wifi_test_utils.get_install_path(
-                host, params.get('cmd_netdump', '/usr/sbin/tcpdump'))
-        cmd_ifconfig = wifi_test_utils.get_install_path(
-                host, params.get('cmd_ifconfig', 'ifconfig'))
         self._packet_capturer = packet_capturer.get_packet_capturer(
-                self.host, host_description=role, cmd_ifconfig=cmd_ifconfig,
-                cmd_ip=self.cmd_ip, cmd_iw=cmd_iw, cmd_netdump=cmd_netdump,
-                ignore_failures=True)
+                self.host, host_description=role, cmd_ip=self.cmd_ip,
+                cmd_iw=cmd_iw, ignore_failures=True)
         self.iw_runner = iw_runner.IwRunner(remote_host=host, command_iw=cmd_iw)
 
         self._phy_list = None
@@ -257,7 +248,7 @@
         idle_phys = [phy for phy in phys if phy not in busy_phys]
         phys = idle_phys or phys
 
-        preferred_bus = self.phy_bus_preference.get(phytype)
+        preferred_bus = {'monitor': 'usb', 'managed': 'pci'}.get(phytype)
         preferred_phys = [phy for phy in phys
                           if self.phy_bus_type[phy] == preferred_bus]
         phys = preferred_phys or phys
@@ -265,15 +256,11 @@
         return phys[0]
 
 
-    def get_wlanif(self, frequency, phytype, mode=None, same_phy_as=None):
-        """Get a WiFi device that supports the given frequency, mode, and type.
-
-        We still support the old "phydevN" parameters, but this code is
-        smart enough to do without it.
+    def get_wlanif(self, frequency, phytype, same_phy_as=None):
+        """Get a WiFi device that supports the given frequency and type.
 
         @param frequency int WiFi frequency to support.
         @param phytype string type of phy (e.g. 'monitor').
-        @param mode string 'a' 'b' or 'g'.
         @param same_phy_as string create the interface on the same phy as this.
         @return string WiFi device.
 
@@ -286,15 +273,11 @@
             else:
                 raise error.TestFail('Unable to find phy for interface %s' %
                                      same_phy_as)
-        elif mode in ('b', 'g') and self.phydev2 is not None:
-            phy = self.phydev2
-        elif mode == 'a' and self.phydev5 is not None:
-            phy = self.phydev5
         elif frequency in self.phys_for_frequency:
             phy = self._get_phy_for_frequency(frequency, phytype)
         else:
-            raise error.TestFail('Unable to find phy for frequency %d mode %s' %
-                                 (frequency, mode))
+            raise error.TestFail('Unable to find phy for frequency %d' %
+                                 frequency)
 
         # If we have a suitable unused interface sitting around on this
         # phy, reuse it.
diff --git a/server/site_tests/network_WiFi_IBSS/network_WiFi_IBSS.py b/server/site_tests/network_WiFi_IBSS/network_WiFi_IBSS.py
index cb6e3ac..07d3e80 100644
--- a/server/site_tests/network_WiFi_IBSS/network_WiFi_IBSS.py
+++ b/server/site_tests/network_WiFi_IBSS/network_WiFi_IBSS.py
@@ -17,7 +17,6 @@
         """Body of the test."""
         self.context.router.require_capabilities(
                 [site_linux_system.LinuxSystem.CAPABILITY_IBSS])
-        self.context.router.create_wifi_device(device_type='ibss')
         configuration = hostap_config.HostapConfig(
                 frequency=2412, mode=hostap_config.HostapConfig.MODE_11B)
         self.context.configure(configuration, is_ibss=True)
diff --git a/server/site_tests/network_WiFi_SecChange/network_WiFi_SecChange.py b/server/site_tests/network_WiFi_SecChange/network_WiFi_SecChange.py
index 6d03a99..5de39d2 100644
--- a/server/site_tests/network_WiFi_SecChange/network_WiFi_SecChange.py
+++ b/server/site_tests/network_WiFi_SecChange/network_WiFi_SecChange.py
@@ -36,9 +36,8 @@
         self.context.assert_connect_wifi(assoc_params)
         self.context.assert_ping_from_dut()
         self.context.client.shill.disconnect(assoc_params.ssid)
-        # This destroy erases the state stored in the router around WPA.
-        self.context.router.destroy()
-        self.context.router.create_wifi_device()
+        # This deconfig erases the state stored in the router around WPA.
+        self.context.router.deconfig()
         # Now we change the same SSID to be an open network.
         ap_config = hostap_config.HostapConfig(
                     ssid=self.TEST_SSID,