Skip to content

Commit

Permalink
Parse ip%scope in CLI arguments
Browse files Browse the repository at this point in the history
Currently, the Humility CLI accepts IP addresses as a `String`, and then
attempts to parse them as an `Ipv6Addr` and scope. This is a bit
unfortunate, as when argument parsing is performed by `clap`, the CLI
will emit a somewhat nicer error message when parsing fails (indicating
which argument the error occurred while parsing, including the invalid
value, and suggesting the help text). On the other hand, when we accept
the argument as a `String`, `clap` doesn't add that useful context to
the error message.

This branch adds a `ScopedV6Addr` newtype, consisting of an `Ipv6Addr`
and a scope identifier, and adds a `FromStr` implementation for that
type. Now, we can use `ScopedV6Addr` as the type of the CLI argument,
and `clap` performs the parsing for us, getting us somewhat nicer error
messages. Compare the error output from master:

```console
$ cargo run -- --ip ::%fakeiface0 probe
    Finished dev [unoptimized + debuginfo] target(s) in 0.13s
     Running `target/debug/humility --ip '::%fakeiface0' probe`
humility probe failed: Could not find interface for fakeiface0

$ cargo run -- --ip :: probe
    Finished dev [unoptimized + debuginfo] target(s) in 0.13s
     Running `target/debug/humility --ip '::' probe`
humility probe failed: Missing scope id in IP (e.g. '%en0')

$ cargo run -- --ip barf%eth0 probe
    Finished dev [unoptimized + debuginfo] target(s) in 0.13s
     Running `target/debug/humility --ip 'barf%eth0' probe`
humility probe failed: invalid Zip archive: Invalid zip header
```

with the error output on this branch:

```console
$ cargo run -- --ip :: probe
    Finished dev [unoptimized + debuginfo] target(s) in 0.14s
     Running `target/debug/humility --ip '::' probe`
error: Invalid value "::" for '--ip <IP>': missing scope ID (e.g. "%en0") in IPv6 address

For more information try --help

$ cargo run -- --ip ::%fakeiface0 probe
    Finished dev [unoptimized + debuginfo] target(s) in 8.45s
     Running `target/debug/humility --ip '::%fakeiface0' probe`
error: Invalid value "::%fakeiface0" for '--ip <IP>': Could not find interface for fakeiface0

For more information try --help

$ cargo run -- --ip barf%eth0 probe
    Finished dev [unoptimized + debuginfo] target(s) in 12.87s
     Running `target/debug/humility --ip 'barf%eth0' probe`
error: Invalid value "barf%eth0" for '--ip <IP>': "barf" is not a valid IPv6 address

For more information try --help
```

Additionally, using the `ScopedIpv6Addr` type allows us to provide a
`fmt::Display` implementation that combines the IP address and scope ID
when formatting. This lets us get rid of a little bit of repeated logic
for formatting an address with the delimiter, which seems kinda nice to
me.
  • Loading branch information
hawkw committed Jan 31, 2024
1 parent 136f158 commit 1cb9bf3
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 18 deletions.
6 changes: 3 additions & 3 deletions humility-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod env;
use anyhow::Result;
use clap::{AppSettings, ArgGroup, ArgMatches, Parser};
use env::Environment;
use humility::{core::Core, hubris::HubrisArchive, msg, warn};
use humility::{core::Core, hubris::HubrisArchive, msg, net, warn};

#[derive(Parser, Debug, Clone)]
#[clap(
Expand Down Expand Up @@ -61,7 +61,7 @@ pub struct Cli {
/// HUMILITY_IP environment variable. Run "humility doc" for more
/// information on running Humility over a network.
#[clap(long, short, group = "hubris")]
pub ip: Option<String>,
pub ip: Option<net::ScopedV6Addr>,

/// Hubris environment file. Thie may also be set via the
/// HUMILITY_ENVIRONMENT environment variable. Run "humility doc" for
Expand Down Expand Up @@ -158,7 +158,7 @@ impl ExecutionContext {
if let Ok(e) = env::var("HUMILITY_PROBE") {
cli.probe = Some(e);
} else if let Ok(e) = env::var("HUMILITY_IP") {
cli.ip = Some(e);
cli.ip = Some(e.parse()?);
} else if let Ok(e) = env::var("HUMILITY_TARGET") {
cli.target = Some(e);
} else if let Ok(e) = env::var("HUMILITY_DUMP") {
Expand Down
2 changes: 1 addition & 1 deletion humility-cmd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub fn attach_dump(
}

pub fn attach_net(args: &Cli, hubris: &HubrisArchive) -> Result<Box<dyn Core>> {
if let Some(ip) = &args.ip {
if let Some(ip) = args.ip {
let timeout = Duration::from_millis(args.timeout as u64);
humility_net_core::attach_net(ip, hubris, timeout)
} else {
Expand Down
42 changes: 41 additions & 1 deletion humility-core/src/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,47 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use anyhow::{bail, Result};
use anyhow::{bail, Context, Result};
use std::{fmt, net::Ipv6Addr, str::FromStr};

#[derive(Copy, Clone, Eq, PartialEq)]
pub struct ScopedV6Addr {
pub ip: Ipv6Addr,
pub scopeid: u32,
}

const SCOPE_DELIM: char = '%';

impl FromStr for ScopedV6Addr {
type Err = anyhow::Error;

fn from_str(s: &str) -> Result<Self> {
let s = s.trim();
let Some((ip, scope)) = s.rsplit_once(SCOPE_DELIM) else {
bail!(
"missing scope ID (e.g. \"{SCOPE_DELIM}en0\") in IPv6 address"
)
};
let ip = ip
.parse()
.with_context(|| format!("{ip:?} is not a valid IPv6 address"))?;
let scopeid = decode_iface(scope)?;
Ok(ScopedV6Addr { ip, scopeid })
}
}

impl fmt::Display for ScopedV6Addr {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { ip, scopeid } = self;
write!(f, "{ip}{SCOPE_DELIM}{scopeid}")
}
}

impl fmt::Debug for ScopedV6Addr {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(self, f)
}
}

pub fn decode_iface(iface: &str) -> Result<u32> {
#[cfg(not(windows))]
Expand Down
18 changes: 5 additions & 13 deletions humility-net-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use humility::{
core::{Core, NetAgent},
hubris::{HubrisArchive, HubrisFlashMap, HubrisRegion, HubrisTask},
msg,
net::decode_iface,
net::ScopedV6Addr,
};
use humility_arch_arm::ARMRegister;
use humility_dump_agent::{
Expand Down Expand Up @@ -46,21 +46,13 @@ pub struct NetCore {

impl NetCore {
fn new(
ip: &str,
addr: ScopedV6Addr,
hubris: &HubrisArchive,
timeout: Duration,
) -> Result<Self> {
let mut iter = ip.split('%');
let ip = iter.next().expect("ip address is empty");
let iface = iter
.next()
.ok_or_else(|| anyhow!("Missing scope id in IP (e.g. '%en0')"))?;

let scopeid = decode_iface(iface)?;

let udprpc_socket = if hubris.lookup_task("udprpc").is_some() {
// See oxidecomputer/oana for standard Hubris UDP ports
let target = format!("[{}%{}]:998", ip, scopeid);
let target = format!("[{addr}]:998");

let dest = target.to_socket_addrs()?.collect::<Vec<_>>();
let udprpc_socket = UdpSocket::bind("[::]:0")?;
Expand Down Expand Up @@ -88,7 +80,7 @@ impl NetCore {
//
// See oxidecomputer/oana for standard Hubris UDP ports
let dump_agent_socket = if has_dump_agent {
let target = format!("[{}%{}]:11113", ip, scopeid);
let target = format!("[{addr}]:11113");
let dest = target.to_socket_addrs()?.collect::<Vec<_>>();
let dump_agent_socket = UdpSocket::bind("[::]:0")?;
dump_agent_socket.set_read_timeout(Some(timeout))?;
Expand Down Expand Up @@ -419,7 +411,7 @@ impl Core for NetCore {
}

pub fn attach_net(
ip: &str,
ip: ScopedV6Addr,
hubris: &HubrisArchive,
timeout: Duration,
) -> Result<Box<dyn Core>> {
Expand Down

0 comments on commit 1cb9bf3

Please sign in to comment.