From 6f937039c7187c84a45901f29c67ad0a0f6b8192 Mon Sep 17 00:00:00 2001 From: John Smith Date: Sat, 17 Dec 2022 13:02:39 -0500 Subject: [PATCH] better route selection --- external/keyring-manager | 2 +- veilid-core/src/network_manager/mod.rs | 34 ++++++++---- .../src/routing_table/route_spec_store.rs | 55 ++++++++++++++++--- .../src/routing_table/routing_domains.rs | 11 +++- veilid-core/src/veilid_api/types.rs | 1 - 5 files changed, 81 insertions(+), 22 deletions(-) diff --git a/external/keyring-manager b/external/keyring-manager index c153eb30..b127b2d3 160000 --- a/external/keyring-manager +++ b/external/keyring-manager @@ -1 +1 @@ -Subproject commit c153eb3015d6d118e5d467865510d053ddd84533 +Subproject commit b127b2d3c653fea163a776dd58b3798f28aeeee3 diff --git a/veilid-core/src/network_manager/mod.rs b/veilid-core/src/network_manager/mod.rs index d23677f2..8bd6012b 100644 --- a/veilid-core/src/network_manager/mod.rs +++ b/veilid-core/src/network_manager/mod.rs @@ -1379,9 +1379,13 @@ impl NetworkManager { let some_relay_nr = if self.check_client_whitelist(sender_id) { // Full relay allowed, do a full resolve_node - rpc.resolve_node(recipient_id).await.wrap_err( - "failed to resolve recipient node for relay, dropping outbound relayed packet", - )? + match rpc.resolve_node(recipient_id).await { + Ok(v) => v, + Err(e) => { + log_net!(debug "failed to resolve recipient node for relay, dropping outbound relayed packet: {}" ,e); + return Ok(false); + } + } } else { // If this is not a node in the client whitelist, only allow inbound relay // which only performs a lightweight lookup before passing the packet back out @@ -1396,9 +1400,14 @@ impl NetworkManager { if let Some(relay_nr) = some_relay_nr { // Relay the packet to the desired destination log_net!("relaying {} bytes to {}", data.len(), relay_nr); - network_result_value_or_log!(self.send_data(relay_nr, data.to_vec()) - .await - .wrap_err("failed to forward envelope")? => { + network_result_value_or_log!(match self.send_data(relay_nr, data.to_vec()) + .await { + Ok(v) => v, + Err(e) => { + log_net!(debug "failed to forward envelope: {}" ,e); + return Ok(false); + } + } => { return Ok(false); } ); @@ -1411,10 +1420,15 @@ impl NetworkManager { let node_id_secret = routing_table.node_id_secret(); // Decrypt the envelope body - // xxx: punish nodes that send messages that fail to decrypt eventually - let body = envelope - .decrypt_body(self.crypto(), data, &node_id_secret) - .wrap_err("failed to decrypt envelope body")?; + let body = match envelope + .decrypt_body(self.crypto(), data, &node_id_secret) { + Ok(v) => v, + Err(e) => { + log_net!(debug "failed to decrypt envelope body: {}",e); + // xxx: punish nodes that send messages that fail to decrypt eventually + return Ok(false); + } + }; // Cache the envelope information in the routing table let source_noderef = match routing_table.register_node_with_existing_connection( diff --git a/veilid-core/src/routing_table/route_spec_store.rs b/veilid-core/src/routing_table/route_spec_store.rs index e1a1351a..9928fb7c 100644 --- a/veilid-core/src/routing_table/route_spec_store.rs +++ b/veilid-core/src/routing_table/route_spec_store.rs @@ -646,22 +646,43 @@ impl RouteSpecStore { let v = v.unwrap(); // Exclude our relay if we have one - if let Some(relay_id) = opt_relay_id { - if k == relay_id { + if let Some(own_relay_id) = opt_relay_id { + if k == own_relay_id { return false; } } - // Exclude nodes on our local network - let on_local_network = v.with(rti, |_rti, e| { - e.node_info(RoutingDomain::LocalNetwork).is_some() - }); - if on_local_network { + // Exclude nodes we have specifically chosen to avoid + if avoid_node_ids.contains(&k) { return false; } - // Exclude nodes we have specifically chosen to avoid - if avoid_node_ids.contains(&k) { + // Process node info exclusions + let keep = v.with(rti, |_rti, e| { + // Exclude nodes on our local network + if e.node_info(RoutingDomain::LocalNetwork).is_some() { + return false; + } + // Exclude nodes that have no publicinternet signednodeinfo + let Some(sni) = e.signed_node_info(RoutingDomain::PublicInternet) else { + return false; + }; + // Relay check + if let Some(relay_id) = sni.relay_id() { + // Exclude nodes whose relays we have chosen to avoid + if avoid_node_ids.contains(&relay_id.key) { + return false; + } + // Exclude nodes whose relay is our own relay if we have one + if let Some(own_relay_id) = opt_relay_id { + if own_relay_id == relay_id.key { + return false; + } + } + } + return true; + }); + if !keep { return false; } @@ -774,6 +795,22 @@ impl RouteSpecStore { return None; } + // Ensure the route doesn't contain both a node and its relay + let mut seen_nodes: HashSet = HashSet::new(); + for n in permutation { + let node = nodes.get(*n).unwrap(); + if !seen_nodes.insert(node.node_id.key) { + // Already seen this node, should not be in the route twice + return None; + } + if let Some(relay_id) = node.signed_node_info.relay_id() { + if !seen_nodes.insert(relay_id.key) { + // Already seen this node, should not be in the route twice + return None; + } + } + } + // Ensure this route is viable by checking that each node can contact the next one if directions.contains(Direction::Outbound) { let mut previous_node = &our_peer_info; diff --git a/veilid-core/src/routing_table/routing_domains.rs b/veilid-core/src/routing_table/routing_domains.rs index d52365c8..21fcfefe 100644 --- a/veilid-core/src/routing_table/routing_domains.rs +++ b/veilid-core/src/routing_table/routing_domains.rs @@ -294,8 +294,10 @@ impl RoutingDomainDetail for PublicInternetRoutingDomainDetail { // Get the target's inbound relay, it must have one or it is not reachable if let Some(node_b_relay) = peer_b.signed_node_info.relay_info() { let node_b_relay_id = peer_b.signed_node_info.relay_id().unwrap(); + // Note that relay_peer_info could be node_a, in which case a connection already exists - // and we shouldn't have even gotten here + // and we only get here if the connection had dropped, in which case node_a is unreachable until + // it gets a new relay connection up if node_b_relay_id.key == peer_a.node_id.key { return ContactMethod::Existing; } @@ -375,6 +377,13 @@ impl RoutingDomainDetail for PublicInternetRoutingDomainDetail { // If the node B has no direct dial info, it needs to have an inbound relay else if let Some(node_b_relay) = peer_b.signed_node_info.relay_info() { let node_b_relay_id = peer_b.signed_node_info.relay_id().unwrap(); + + // Note that relay_peer_info could be node_a, in which case a connection already exists + // and we only get here if the connection had dropped, in which case node_a is unreachable until + // it gets a new relay connection up + if node_b_relay_id.key == peer_a.node_id.key { + return ContactMethod::Existing; + } // Can we reach the full relay? if first_filtered_dial_info_detail( diff --git a/veilid-core/src/veilid_api/types.rs b/veilid-core/src/veilid_api/types.rs index 36e9c608..922b77cf 100644 --- a/veilid-core/src/veilid_api/types.rs +++ b/veilid-core/src/veilid_api/types.rs @@ -1988,7 +1988,6 @@ impl SignedNodeInfo { SignedNodeInfo::Relayed(r) => r.timestamp, } } - pub fn node_info(&self) -> &NodeInfo { match self { SignedNodeInfo::Direct(d) => &d.node_info,