Skip to content

Commit

Permalink
fix(iroh-net): poll_send should drop transmits that we dont have a …
Browse files Browse the repository at this point in the history
…`dest` for (#2393)

## Description

We've learned that if we return anything other than `Ok` in `poll_send`,
we will be blocking the endpoint and ruining all connections. We should
not return an error just because we don't have a proper address for the
`dest`, instead we should log an error and wait for the connection to
timeout. By returning `Poll::Ready(Ok(n))`, we are telling quinn to drop
the `n` transmits that are associated with `dest`. When quinn doesn't
receive any `ACK`s for those transmits, quinn end the connection with a
timeout error.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
  • Loading branch information
ramfox authored Jun 21, 2024
1 parent 600ba8c commit aba70ea
Showing 1 changed file with 13 additions and 4 deletions.
17 changes: 13 additions & 4 deletions iroh-net/src/magicsock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,11 +612,20 @@ impl MagicSock {
Poll::Ready(Ok(transmits_sent))
}
None => {
// Returning an error here would lock up the entire `Endpoint`.
//
// If we returned `Poll::Pending`, the waker driving the `poll_send` will never get woken up.
//
// Our best bet here is to log an error and return `Poll::Ready(Ok(n))`.
//
// `n` is the number of consecutive transmits in this batch that are meant for the same destination (a destination that we have no node state for, and so we can never actually send).
//
// When we return `Poll::Ready(Ok(n))`, we are effectively dropping those n messages, by lying to QUIC and saying they were sent.
// (If we returned `Poll::Ready(Ok(0))` instead, QUIC would loop to attempt to re-send those messages, blocking other traffic.)
//
// When `QUIC` gets no `ACK`s for those messages, the connection will eventually timeout.
error!(dst=%dest, "no node_state for mapped address");
Poll::Ready(Err(io::Error::new(
io::ErrorKind::NotConnected,
"trying to send to unknown node",
)))
Poll::Ready(Ok(n))
}
}
}
Expand Down

0 comments on commit aba70ea

Please sign in to comment.