Fuchsia: Use SSH multiplexing for forwarded test server connections.

SSH multiplexing makes SSH-based forwarding much more performant
and reliable versus using separately negotiated connection.

Connection overhead for runner_v2 SSH operations is also reduced.

* Fix deployment path used to push the test server configuration
  to the target device.
* Documentation fixes.

Bug: 798851
Change-Id: I4d97204b0e3420e0ae8ce0ca8bcf6ca137dce46f
Reviewed-on: https://chromium-review.googlesource.com/1018364
Commit-Queue: Kevin Marshall <kmarshall@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552157}

build/config/fuchsia/BUILD.gn

@@ -71,15 +71,18 @@ action("blobstore_extended_fvm") {
   # limitation in the future.
   _extend_size = "524288000"  # 500MB = 500 * 1024 * 1024
 
+  if (current_cpu == "arm64") {
+    _target_dir = "//third_party/fuchsia-sdk/sdk/target/aarch64"
+  } else if (current_cpu == "x64") {
+    _target_dir = "//third_party/fuchsia-sdk/sdk/target/x86_64"
+  }
+
   script = "//build/config/fuchsia/extend_fvm.py"
 
-  if (current_cpu == "arm64") {
-    _blobstore_src_path = "//third_party/fuchsia-sdk/sdk/target/aarch64/fvm.blk"
-  } else if (current_cpu == "x64") {
-    _blobstore_src_path = "//third_party/fuchsia-sdk/sdk/target/x86_64/fvm.blk"
-  }
   inputs = [
-    _blobstore_src_path,
+    "${_target_dir}/fvm.blk",
+    "${_target_dir}/bootdata-blob.bin",
+    "${_target_dir}/zircon.bin",
   ]
   outputs = [
     blobstore_extended_path,
@@ -87,7 +90,7 @@ action("blobstore_extended_fvm") {
 
   args = [
     rebase_path("${fuchsia_sdk}/tools/fvm"),
-    rebase_path(_blobstore_src_path),
+    rebase_path("${_target_dir}/fvm.blk"),
     rebase_path(blobstore_extended_path),
     _extend_size,
   ]

build/fuchsia/runner_v2/boot_data.py

@@ -18,13 +18,15 @@ Host *
   StrictHostKeyChecking no
   ForwardAgent no
   ForwardX11 no
-  GSSAPIDelegateCredentials no
   UserKnownHostsFile {known_hosts}
   User fuchsia
   IdentitiesOnly yes
   IdentityFile {identity}
   ServerAliveInterval 1
-  ServerAliveCountMax 1"""
+  ServerAliveCountMax 1
+  ControlMaster auto
+  ControlPersist 1m
+  ControlPath /tmp/ssh-%r@%h:%p"""
 
 
 def _TargetCpuToSdkBinPath(target_arch):

build/fuchsia/runner_v2/net_test_server.py

@@ -26,10 +26,12 @@ def _ConnectPortForwardingTask(target, local_port):
   """Establishes a port forwarding SSH task to a localhost TCP endpoint hosted
   at port |local_port|. Blocks until port forwarding is established.
 
-  Returns a tuple containing the remote port and the SSH task Popen object."""
+  Returns the remote port number."""
 
-  forwarding_flags = ['-NT',  # Don't execute command; don't allocate terminal.
-                      '-R', '0:localhost:%d' % local_port]
+  forwarding_flags = ['-O', 'forward',  # Send SSH mux control signal.
+                      '-R', '0:localhost:%d' % local_port,
+                      '-v',   # Get forwarded port info from stderr.
+                      '-NT']  # Don't execute command; don't allocate terminal.
   task = target.RunCommandPiped([],
                                 ssh_args=forwarding_flags,
                                 stderr=subprocess.PIPE)
@@ -50,13 +52,15 @@ def _ConnectPortForwardingTask(target, local_port):
         break
       line += next_char
       if line.endswith('\n'):
-        line.strip()
+        line = line[:-1]
+        logging.debug('ssh: ' + line)
         matched = PORT_MAP_RE.match(line)
         if matched:
           device_port = int(matched.group('port'))
           logging.debug('Port forwarding established (local=%d, device=%d)' %
                         (local_port, device_port))
-          return (device_port, task)
+          task.wait()
+          return device_port
         line = ''
 
   raise Exception('Could not establish a port forwarding connection.')
@@ -68,8 +72,7 @@ class SSHPortForwarder(chrome_test_server_spawner.PortForwarder):
   def __init__(self, target):
     self._target = target
 
-    # Maps the host (server) port to a tuple consisting of the device port
-    # number and a subprocess.Popen object for the forwarding SSH process.
+    # Maps the host (server) port to the device port number.
     self._port_mapping = {}
 
   def Map(self, port_pairs):
@@ -79,13 +82,22 @@ class SSHPortForwarder(chrome_test_server_spawner.PortForwarder):
           _ConnectPortForwardingTask(self._target, host_port)
 
   def GetDevicePortForHostPort(self, host_port):
-    return self._port_mapping[host_port][0]
+    return self._port_mapping[host_port]
 
   def Unmap(self, device_port):
     for host_port, entry in self._port_mapping.iteritems():
-      if entry[0] == device_port:
-        entry[1].terminate()
-        entry[1].wait()
+      if entry == device_port:
+        forwarding_args = [
+            '-NT', '-O', 'cancel', '-R',
+            '%d:localhost:%d' % (self._port_mapping[host_port], host_port)]
+        task = self._target.RunCommandPiped([],
+                                            ssh_args=forwarding_args,
+                                            stderr=subprocess.PIPE)
+        task.wait()
+        if task.returncode != 0:
+          raise Exception(
+              'Error %d when unmapping port %d' % (task.returncode,
+                                                   device_port))
         del self._port_mapping[host_port]
         return
 
@@ -95,13 +107,12 @@ class SSHPortForwarder(chrome_test_server_spawner.PortForwarder):
 def SetupTestServer(target, test_concurrency):
   """Provisions a forwarding test server and configures |target| to use it.
 
-  Returns a tuple with a Popen opbject for the test server process,
-  and a Popen object for the port forwarding connection."""
+  Returns a Popen object for the test server process."""
 
   logging.debug('Starting test server.')
   spawning_server = chrome_test_server_spawner.SpawningServer(
       0, SSHPortForwarder(target), test_concurrency)
-  forwarded_port, forwarding_process = _ConnectPortForwardingTask(
+  forwarded_port = _ConnectPortForwardingTask(
       target, spawning_server.server_port)
   spawning_server.Start()
 
@@ -119,6 +130,6 @@ def SetupTestServer(target, test_concurrency):
   }))
 
   config_file.flush()
-  target.PutFile(config_file.name, '/system/net-test-server-config')
+  target.PutFile(config_file.name, '/data/net-test-server-config')
 
-  return spawning_server, forwarding_process
+  return spawning_server

build/fuchsia/runner_v2/test_runner.py

@@ -98,7 +98,7 @@ def main():
 
     forwarder = None
     if args.enable_test_server:
-      test_server, forwarder = SetupTestServer(target, test_concurrency)
+      test_server = SetupTestServer(target, test_concurrency)
 
     returncode = RunPackage(args.output_directory, target, args.package,
                             args.package_name, child_args,

net/test/spawned_test_server/remote_test_server_config.cc

@@ -16,6 +16,10 @@
 #include "build/build_config.h"
 #include "url/gurl.h"
 
+#if defined(OS_FUCHSIA)
+#include "base/base_paths_fuchsia.h"
+#endif
+
 namespace net {
 
 namespace {
@@ -25,7 +29,14 @@ base::FilePath GetTestServerConfigFilePath() {
 #if defined(OS_ANDROID)
   PathService::Get(base::DIR_ANDROID_EXTERNAL_STORAGE, &dir);
 #elif defined(OS_FUCHSIA)
-  dir = base::FilePath("/system");
+  // TODO(https://crbug.com/805057): Remove conditional after bootfs turndown.
+  if (base::GetPackageRoot().empty()) {
+    // Bootfs runs.
+    dir = base::FilePath("/system");
+  } else {
+    // Packaged runs.
+    dir = base::FilePath("/data");
+  }
 #else
   PathService::Get(base::DIR_TEMP, &dir);
 #endif