Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(iroh-net): merge related fields regarding incoming pings #2236

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 19 additions & 30 deletions iroh-net/src/magicsock/node_map/node_state.rs
Original file line number Diff line number Diff line change
@@ -1099,18 +1099,10 @@ pub(super) struct PathState {
/// The last (outgoing) ping time.
last_ping: Option<Instant>,

// TODO: merge last_got_ping and last_got_ping_tx_id into one field and one Option
/// If non-zero, means that this was an endpoint
/// that we learned about at runtime (from an incoming ping)
/// and that is not in the network map. If so, we keep the time
/// updated and use it to discard old candidates.
last_got_ping: Option<Instant>,

/// Contains the TxID for the last incoming ping. This is
/// used to de-dup incoming pings that we may see on both the raw disco
/// socket on Linux, and UDP socket. We cannot rely solely on the raw socket
/// disco handling due to <https://github.com/tailscale/tailscale/issues/7078>.
last_got_ping_tx_id: Option<stun::TransactionId>,
/// If non-zero, means that this was an endpoint that we learned about at runtime (from an
/// incoming ping). If so, we keep the time updated and use it to discard old candidates.
// NOTE: tx_id Originally added in tailscale due to <https://github.com/tailscale/tailscale/issues/7078>.
last_got_ping: Option<(Instant, stun::TransactionId)>,

/// If non-zero, is the time this endpoint was advertised last via a call-me-maybe disco message.
call_me_maybe_time: Option<Instant>,
@@ -1131,8 +1123,7 @@ impl PathState {

pub(super) fn with_ping(tx_id: stun::TransactionId, now: Instant) -> Self {
PathState {
last_got_ping: Some(now),
last_got_ping_tx_id: Some(tx_id),
last_got_ping: Some((now, tx_id)),
..Default::default()
}
}
@@ -1163,6 +1154,11 @@ impl PathState {
.unwrap_or(false)
}

/// Returns the instant the last incoming ping was received.
pub(super) fn last_incoming_ping(&self) -> Option<&Instant> {
self.last_got_ping.as_ref().map(|(time, _tx_id)| time)
}

/// Reports the last instant this path was considered alive.
///
/// Alive means the path is considered in use by the remote endpoint. Either because we
@@ -1180,7 +1176,7 @@ impl PathState {
.into_iter()
.chain(self.last_payload_msg.as_ref())
.chain(self.call_me_maybe_time.as_ref())
.chain(self.last_got_ping.as_ref())
.chain(self.last_incoming_ping())
.max()
.copied()
}
@@ -1195,8 +1191,7 @@ impl PathState {
.as_ref()
.map(|call_me| (*call_me, ControlMsg::CallMeMaybe));
let last_ping = self
.last_got_ping
.as_ref()
.last_incoming_ping()
.map(|ping| (*ping, ControlMsg::Ping));

last_pong
@@ -1235,28 +1230,22 @@ impl PathState {
}

fn handle_ping(&mut self, tx_id: stun::TransactionId, now: Instant) -> PingRole {
if Some(tx_id) == self.last_got_ping_tx_id {
if Some(&tx_id) == self.last_got_ping.as_ref().map(|(_t, tx_id)| tx_id) {
PingRole::Duplicate
} else {
self.last_got_ping_tx_id.replace(tx_id);
let last = self.last_got_ping.replace(now);
match last {
None => PingRole::Reactivate,
Some(last) => {
if now.duration_since(last) <= HEARTBEAT_INTERVAL {
PingRole::LikelyHeartbeat
} else {
PingRole::Reactivate
}
let prev = self.last_got_ping.replace((now, tx_id));
match prev {
Some((prev_time, _tx)) if now.duration_since(prev_time) <= HEARTBEAT_INTERVAL => {
PingRole::LikelyHeartbeat
}
_ => PingRole::Reactivate,
}
}
}

fn clear(&mut self) {
self.last_ping = None;
self.last_got_ping = None;
self.last_got_ping_tx_id = None;
self.call_me_maybe_time = None;
self.recent_pong = None;
}
@@ -1269,7 +1258,7 @@ impl PathState {
if let Some(ref pong) = self.recent_pong {
write!(w, "pong-received({:?} ago)", pong.pong_at.elapsed())?;
}
if let Some(ref when) = self.last_got_ping {
if let Some(when) = self.last_incoming_ping() {
write!(w, "ping-received({:?} ago) ", when.elapsed())?;
}
if let Some(ref when) = self.last_ping {