fix(iroh-net): improve hole punching when relying on call-me-maybe #1983
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
dignifiedquire
commented
Jan 29, 2024
•
dignifiedquire
commented
- only consider active endpoints for call-me-maybe checks
- insert call me maybe addrs into the peer map
endpoint.close() is too agressive to actually get the behaviour being checked for
|
/netsim branch arqu/direct_derp |
|
|
flub
reviewed
Jan 30, 2024
| @@ -673,7 +672,8 @@ mod tests { | |||
| let mut buf = [0u8, 5]; | |||
| stream.read_exact(&mut buf).await.unwrap(); | |||
| info!("Accepted 1 stream, received {buf:?}. Closing now."); | |||
| ep.close(7u8.into(), b"bye").await.unwrap(); | |||
| // close the stream | |||
| conn.close(7u8.into(), b"bye"); | |||
There was a problem hiding this comment.
by not awaiting this you're never closing this?
There was a problem hiding this comment.
there is no way to actually wait for a connection close asfaik
flub
reviewed
Jan 30, 2024
| @@ -438,7 +438,7 @@ impl Endpoint { | |||
| }) | |||
| .collect(); | |||
| let ping_needed = !pings.is_empty(); | |||
| let have_endpoints = !self.direct_addr_state.is_empty(); | |||
| let have_alive_endpoints = self.direct_addr_state.values().any(|e| e.is_alive()); | |||
There was a problem hiding this comment.
i'm currently convinced this condition is pointless, the ping_needed condition is all that is needed here.
dignifiedquire
changed the title
flub
approved these changes
Feb 1, 2024
dignifiedquire
changed the title
fubuloubu
pushed a commit
to ApeWorX/iroh
that referenced
this pull request
Feb 21, 2024
- only consider active endpoints for call-me-maybe checks - insert call me maybe addrs into the peer map
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.