fix(iroh): Poll all AsyncUdpSocket sources fairly #2996
Merged
Conversation
This file contains hidden or 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
The existing AsyncUdpSocket::poll_recv polled the UDP IPv4 socket first, only if that has no datagrams would it poll UDP IPv6 and only also that had no datagrams to deliver was the relay socket polled. This highly favoured IPv4 relay traffic, potentially starving the relay traffic. This change makes it prefer a different source on each poll. Meaning if multiple sockets have data to deliver a fairer delivery happens.
flub
requested review from
matheus23,
ramfox,
dignifiedquire and
divagant-martian
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2996/docs/iroh/ Last updated: 2024-12-03T15:38:17Z |
matheus23
reviewed
Dec 3, 2024
matheus23
reviewed
Dec 3, 2024
matheus23
reviewed
Dec 3, 2024
matheus23
approved these changes
Dec 3, 2024
Co-authored-by: Philipp Krüger <[email protected]>
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.
Description
The existing AsyncUdpSocket::poll_recv polled the UDP IPv4 socket
first, only if that has no datagrams would it poll UDP IPv6 and only
also that had no datagrams to deliver was the relay socket polled.
This highly favoured IPv4 relay traffic, potentially starving the
relay traffic.
This change makes it prefer a different source on each poll. Meaning
if multiple sockets have data to deliver a fairer delivery happens.
Closes #2981.
Breaking Changes
Notes & open questions
Using Ordering::Relaxed is probably fine. I believe normally
this polling would only happen from a single task (the Quinn
endpoint driver) and even if it was polled concurrently from
different threads it would not be that bad and still an improvement
on the current order.
I'm not particularly fond of the implementation, but the macros is
the best I could come up with. Maybe there's something cleverer
which can be done.
Change checklist