Skip to content

Commit

Permalink
fix(iroh-net): Clear the recent pong time when pong is lost (#2743)
Browse files Browse the repository at this point in the history
## Description

When we do not receive a pong in the timeframe we should have, we remove
the last ping time and clear the best_addr. However the path will be
re-selected from the candidates as long as the pong time is set.

This additionally clears the pong time so that there is no longer an
indication this path works. While the DISCO ping-pong is not
loss-protected there is already an existing check that ensures this
clear is not triggered when there are still other signs of life from the
remote node.

## Breaking Changes

None

## Notes & open questions

Note that when sending call-me-maybe the pong times are also cleared. So
while I'm not a particular fan of this, it is the intended way to use
this. Ideally the PathState would be clearer about this data and store
this as separately. But the PathState data needs a general cleanup and I
do not want to mix in extra state for now.

Fixes a bug found by @zh522130 

## Change checklist

- [x] Self-review.
- ~~[ ] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.~~
- ~~[ ] Tests if relevant.~~
- ~~[ ] All breaking changes documented.~~
  • Loading branch information
flub authored Sep 24, 2024
1 parent cafdc08 commit 8fb92f3
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions iroh-net/src/magicsock/node_map/node_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,13 +400,16 @@ impl NodeState {
SendAddr::Udp(addr) => {
if let Some(path_state) = self.udp_paths.paths.get_mut(&addr.into()) {
path_state.last_ping = None;
// only clear the best address if there was no sign of life from this path
// within the time the pong should have arrived
let consider_alive = path_state
.last_alive()
.map(|last_alive| last_alive.elapsed() <= PING_TIMEOUT_DURATION)
.unwrap_or(false);
if !consider_alive {
// If there was no sign of life from this path during the time
// which we should have received the pong, clear best addr and
// pong. Both are used to select this path again, but we know
// it's not a usable path now.
path_state.recent_pong = None;
self.udp_paths.best_addr.clear_if_equals(
addr,
ClearReason::PongTimeout,
Expand Down

0 comments on commit 8fb92f3

Please sign in to comment.