Skip to content

Commit

Permalink
refactor(iroh-net): make region ids always be u16
Browse files Browse the repository at this point in the history
  • Loading branch information
dignifiedquire committed Jun 28, 2023
1 parent c1b674f commit 685b9aa
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 31 deletions.
6 changes: 3 additions & 3 deletions iroh-net/src/hp/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub struct NetInfo {
/// connected to multiple DERP servers (to send to other nodes)
/// but PreferredDERP is the instance number that the node
/// subscribes to traffic at. Zero means disconnected or unknown.
pub preferred_derp: usize,
pub preferred_derp: u16,

/// LinkType is the current link type, if known.
pub link_type: Option<LinkType>,
Expand Down Expand Up @@ -116,7 +116,7 @@ impl NetInfo {
pub enum LinkType {
Wired,
Wifi,
//LTE, 4G, 3G, etc
/// LTE, 4G, 3G, etc
Mobile,
}

Expand All @@ -137,7 +137,7 @@ pub struct PingResult {
/// The ip:port if direct UDP was used. It is not currently set for TSMP pings.
pub endpoint: Option<SocketAddr>,
/// Non-zero DERP region ID if DERP was used. It is not currently set for TSMP pings.
pub derp_region_id: Option<usize>,
pub derp_region_id: Option<u16>,
/// The three-letter region code corresponding to derp_region_id. It is not currently set for TSMP pings.
pub derp_region_code: Option<String>,
/// Whether the ping request error is due to it being a ping to the local node.
Expand Down
8 changes: 4 additions & 4 deletions iroh-net/src/hp/derp/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ use url::Url;
/// Configuration of all the Derp servers that can be used.
#[derive(Debug, Default, Clone, PartialEq, Eq)]
pub struct DerpMap {
pub regions: HashMap<usize, DerpRegion>,
pub regions: HashMap<u16, DerpRegion>,
}

impl DerpMap {
/// Returns the sorted region IDs.
pub fn region_ids(&self) -> Vec<usize> {
pub fn region_ids(&self) -> Vec<u16> {
let mut ids: Vec<_> = self.regions.keys().copied().collect();
ids.sort();
ids
Expand Down Expand Up @@ -69,7 +69,7 @@ impl fmt::Display for DerpMap {
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct DerpRegion {
/// A unique integer for a geographic region.
pub region_id: usize,
pub region_id: u16,
pub nodes: Vec<DerpNode>,
pub avoid: bool,
pub region_code: String,
Expand All @@ -78,7 +78,7 @@ pub struct DerpRegion {
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct DerpNode {
pub name: String,
pub region_id: usize,
pub region_id: u16,
pub host_name: Url,
pub stun_only: bool,
pub stun_port: u16,
Expand Down
10 changes: 5 additions & 5 deletions iroh-net/src/hp/magicsock/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ impl Inner {
pub(super) async fn get_derp_region(&self, region: u16) -> Option<DerpRegion> {
match &*self.derp_map.read().await {
None => None,
Some(ref derp_map) => derp_map.regions.get(&usize::from(region)).cloned(),
Some(ref derp_map) => derp_map.regions.get(&region).cloned(),
}
}

Expand Down Expand Up @@ -1524,7 +1524,7 @@ impl Actor {
ni.preferred_derp = self.pick_derp_fallback().await;
}

if !self.set_nearest_derp(ni.preferred_derp.try_into()?).await {
if !self.set_nearest_derp(ni.preferred_derp).await {
ni.preferred_derp = 0;
}

Expand Down Expand Up @@ -1557,7 +1557,7 @@ impl Actor {
.as_ref()
.expect("already checked")
.regions
.get(&usize::from(derp_num))
.get(&derp_num)
{
Some(dr) => {
info!("home is now derp-{} ({})", derp_num, dr.region_code);
Expand All @@ -1580,7 +1580,7 @@ impl Actor {
/// Returns a non-zero but deterministic DERP node to
/// connect to. This is only used if netcheck couldn't find the nearest one
/// For instance, if UDP is blocked and thus STUN latency checks aren't working
async fn pick_derp_fallback(&self) -> usize {
async fn pick_derp_fallback(&self) -> u16 {
let ids = {
let derp_map = self.conn.derp_map.read().await;
if derp_map.is_none() {
Expand Down Expand Up @@ -1609,7 +1609,7 @@ impl Actor {

let my_derp = self.conn.my_derp();
if my_derp > 0 {
return my_derp.into();
return my_derp;
}

let mut rng = rand::rngs::StdRng::seed_from_u64(0);
Expand Down
11 changes: 3 additions & 8 deletions iroh-net/src/hp/magicsock/derp_actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,10 @@ impl DerpActor {
continue;
}
}
if u16::try_from(rid).expect("region too large") == self.conn.my_derp() {
if rid == self.conn.my_derp() {
self.conn.set_my_derp(0);
}
to_close.push(u16::try_from(rid).expect("too large"));
to_close.push(rid);
}
}
}
Expand Down Expand Up @@ -249,12 +249,7 @@ impl DerpActor {
warn!("DERP is disabled");
return;
}
if !derp_map
.as_ref()
.unwrap()
.regions
.contains_key(&usize::from(region_id))
{
if !derp_map.as_ref().unwrap().regions.contains_key(&region_id) {
warn!("unknown region id {}", region_id);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion iroh-net/src/hp/magicsock/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ impl Endpoint {

if !self.pending_cli_pings.is_empty() {
let ep = sp.to;
let region_id = usize::from(ep.port());
let region_id = ep.port();
// FIXME: this creates a deadlock as it needs to interact with the run loop in the conn::Actor
// let region_code = self.get_derp_region(region_id).await.map(|r| r.region_code);

Expand Down
18 changes: 9 additions & 9 deletions iroh-net/src/hp/netcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,13 @@ pub struct Report {
/// None means not checked.
pub pcp: Option<bool>,
/// or 0 for unknown
pub preferred_derp: usize,
pub preferred_derp: u16,
/// keyed by DERP Region ID
pub region_latency: HashMap<usize, Duration>,
pub region_latency: HashMap<u16, Duration>,
/// keyed by DERP Region ID
pub region_v4_latency: HashMap<usize, Duration>,
pub region_v4_latency: HashMap<u16, Duration>,
/// keyed by DERP Region ID
pub region_v6_latency: HashMap<usize, Duration>,
pub region_v6_latency: HashMap<u16, Duration>,
/// ip:port of global IPv4
pub global_v4: Option<SocketAddr>,
/// `[ip]:port` of global IPv6
Expand Down Expand Up @@ -298,7 +298,7 @@ async fn measure_https_latency(_reg: &DerpRegion) -> Result<(Duration, IpAddr)>
/// return a "204 No Content" response and checking if that's what we get.
///
/// The boolean return is whether we think we have a captive portal.
async fn check_captive_portal(dm: &DerpMap, preferred_derp: Option<usize>) -> Result<bool> {
async fn check_captive_portal(dm: &DerpMap, preferred_derp: Option<u16>) -> Result<bool> {
// If we have a preferred DERP region with more than one node, try
// that; otherwise, pick a random one not marked as "Avoid".
let preferred_derp = if preferred_derp.is_none()
Expand Down Expand Up @@ -1031,7 +1031,7 @@ fn probe_would_help(report: &Report, probe: &Probe, node: &DerpNode) -> bool {
false
}

fn update_latency(m: &mut HashMap<usize, Duration>, region_id: usize, d: Duration) {
fn update_latency(m: &mut HashMap<u16, Duration>, region_id: u16, d: Duration) {
let prev = m.entry(region_id).or_insert(d);
if d < *prev {
*prev = d;
Expand All @@ -1049,7 +1049,7 @@ fn named_node<'a>(dm: &'a DerpMap, node_name: &str) -> Option<&'a DerpNode> {
None
}

fn max_duration_value(m: &HashMap<usize, Duration>) -> Duration {
fn max_duration_value(m: &HashMap<u16, Duration>) -> Duration {
m.values().max().cloned().unwrap_or_default()
}

Expand Down Expand Up @@ -1918,7 +1918,7 @@ mod tests {
let mut report = Report::default();
for (s, d) in a {
assert!(s.starts_with('d'), "invalid derp server key");
let region_id: usize = s[1..].parse().unwrap();
let region_id: u16 = s[1..].parse().unwrap();
report
.region_latency
.insert(region_id, Duration::from_secs(d));
Expand All @@ -1935,7 +1935,7 @@ mod tests {
name: &'static str,
steps: Vec<Step>,
/// want PreferredDERP on final step
want_derp: usize,
want_derp: u16,
// wanted len(c.prev)
want_prev_len: usize,
}
Expand Down
2 changes: 1 addition & 1 deletion iroh-net/src/hp/stun.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ pub mod test {
let mut m = DerpMap::default();

for (i, addr) in stun.enumerate() {
let region_id = i + 1;
let region_id = (i + 1) as u16;
let host = addr.ip();
let port = addr.port();

Expand Down

0 comments on commit 685b9aa

Please sign in to comment.