Skip to content

Commit 24bf214

Browse files
doughertydaCQ Bot
authored andcommitted
[ffx] Add some advanced tuntap checks
Previously, the tun/tap networking mode validation was limited to "is an interface listed on the system". No verification that it was the correct interface, or that it was in a usable state was performed. This CL reworks the "tap_available" utility method to glean more information from the call to "ip". It will now look for the interface of the appropriate name, verify that it is in fact a Tun/Tap interface, that it is enabled, and that isn't being used by another process. The error messages from the method provide context and remedial actions the user can take, where available. There are now two distinct front-end functions to call for this feature: tap_available() checks that the interface is present and correctly configured and unused, while tap_ready() also checks whether it's in the "UP" state. tap_available() is meant for resolving the --net auto flag, while tap_ready() is meant for network validation. Bug: 103180 Bug: 103725 Change-Id: I54c86494eadfcf4f5ef25aeb9ba6eb30261132ad Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/705304 Commit-Queue: Daniel Dougherty <doughertyda@google.com> Reviewed-by: Clayton Wilkinson <wilkinsonclay@google.com>
1 parent 2bf6acc commit 24bf214

File tree

7 files changed

+645
-38
lines changed

7 files changed

+645
-38
lines changed

src/developer/ffx/plugins/emulator/commands/start/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ if (is_host) {
1111
with_unit_tests = true
1212
args_deps = [
1313
"//src/developer/ffx/config:lib",
14+
"//src/developer/ffx/plugins/emulator/common:ffx_emulator_common",
1415
"//src/developer/ffx/plugins/emulator/configuration:ffx_emulator_config",
1516
"//third_party/rust_crates:anyhow",
1617
]

src/developer/ffx/plugins/emulator/commands/start/src/args.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use argh::FromArgs;
66
use ffx_config::FfxConfigBacked;
77
use ffx_core::ffx_command;
8+
use ffx_emulator_common::host_is_mac;
89
use ffx_emulator_config::{AccelerationMode, GpuType, NetworkingMode};
910
use std::path::PathBuf;
1011

@@ -81,7 +82,7 @@ pub struct StartCommand {
8182
pub headless: bool,
8283

8384
/// enable pixel scaling on HiDPI devices. Defaults to true for MacOS, false otherwise.
84-
#[argh(option, default = "std::env::consts::OS == \"macos\"")]
85+
#[argh(option, default = "host_is_mac()")]
8586
pub hidpi_scaling: bool,
8687

8788
/// experimental(for https://fxbug.dev/95278). Passes the given string to the emulator

src/developer/ffx/plugins/emulator/commands/start/src/pbm.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ use anyhow::{bail, Context, Result};
88
use ffx_emulator_common::instances::get_instance_dir;
99
use ffx_emulator_common::{
1010
config::{FfxConfigWrapper, EMU_UPSCRIPT_FILE, KVM_PATH},
11-
split_once, tap_available,
11+
split_once,
12+
tuntap::tap_available,
1213
};
1314
use ffx_emulator_config::{
1415
convert_bundle_to_configs, AccelerationMode, ConsoleType, EmulatorConfiguration, LogLevel,
@@ -121,9 +122,14 @@ async fn apply_command_line_options(
121122
}
122123

123124
if emu_config.host.networking == NetworkingMode::Auto {
124-
if tap_available() {
125+
let available = tap_available();
126+
if available.is_ok() {
125127
emu_config.host.networking = NetworkingMode::Tap;
126128
} else {
129+
tracing::debug!(
130+
"Falling back on user-mode networking: {}",
131+
available.as_ref().unwrap_err()
132+
);
127133
emu_config.host.networking = NetworkingMode::User;
128134
}
129135
}

src/developer/ffx/plugins/emulator/common/BUILD.gn

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ if (is_host) {
1414
"src/lib.rs",
1515
"src/process.rs",
1616
"src/target.rs",
17+
"src/tuntap.rs",
1718
]
1819
deps = [
1920
"//sdk/fidl/fuchsia.developer.ffx:fuchsia.developer.ffx_rust",
@@ -22,7 +23,11 @@ if (is_host) {
2223
"//src/developer/ffx/lib/timeout:lib",
2324
"//src/lib/fidl/rust/fidl",
2425
"//third_party/rust_crates:anyhow",
26+
"//third_party/rust_crates:cfg-if",
27+
"//third_party/rust_crates:mockall",
2528
"//third_party/rust_crates:nix",
29+
"//third_party/rust_crates:serde",
30+
"//third_party/rust_crates:serde_json",
2631
"//third_party/rust_crates:shared_child",
2732
"//third_party/rust_crates:signal-hook",
2833
"//third_party/rust_crates:tracing",

src/developer/ffx/plugins/emulator/common/src/lib.rs

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,19 @@ use std::{
1111
fs::File,
1212
io::{BufRead, Write},
1313
path::PathBuf,
14-
process::Command,
1514
};
1615

1716
// Provides access to ffx_config properties.
1817
pub mod config;
1918
pub mod instances;
2019
pub mod process;
2120
pub mod target;
21+
pub mod tuntap;
22+
23+
/// A utility function for checking whether the host OS is MacOS.
24+
pub fn host_is_mac() -> bool {
25+
std::env::consts::OS == "macos"
26+
}
2227

2328
/// A utility function for splitting a string at a single point and converting it into a tuple.
2429
/// Returns an Err(anyhow) if it can't do the split.
@@ -32,21 +37,6 @@ pub fn split_once(text: &str, pattern: &str) -> Result<(String, String)> {
3237
Ok((first.to_string(), second.to_string()))
3338
}
3439

35-
/// A utility function for testing if a Tap interface is up and available. Assumes the existence
36-
/// of the "ip" program for finding the interface, which is usually preinstalled on Linux hosts
37-
/// but not MacOS hosts. Conservatively assumes any error indicates Tap is unavailable.
38-
pub fn tap_available() -> bool {
39-
if std::env::consts::OS != "linux" {
40-
return false;
41-
}
42-
let output = Command::new("ip").args(["tuntap", "show"]).output();
43-
// Output contains the interface name, if it exists. Tap is considered available if the call
44-
// succeeds and output is non-empty.
45-
// TODO(fxbug.dev/87464): Check for the expected interface name, to make sure we have the right
46-
// one if there are multiple taps possible (i.e. for supporting multiple emu instances).
47-
return output.is_ok() && output.unwrap().stdout.len() > 0;
48-
}
49-
5040
/// A utility function to dump the contents of a file to the terminal.
5141
pub fn dump_log_to_out<W: Write>(log: &PathBuf, out: &mut W) -> Result<()> {
5242
let mut out_handle = std::io::BufWriter::new(out);

0 commit comments

Comments
 (0)