-
Notifications
You must be signed in to change notification settings - Fork 121
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
Connect UDP sockets. #4270
Merged
kaidokert
merged 2 commits into
youtube:25.lts.1+
from
jellefoks:http3_opts_part_one_b_connect
Oct 16, 2024
Merged
Connect UDP sockets. #4270
kaidokert
merged 2 commits into
youtube:25.lts.1+
from
jellefoks:http3_opts_part_one_b_connect
Oct 16, 2024
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
jellefoks
force-pushed
the
http3_opts_part_one_b_connect
branch
3 times, most recently
from
October 15, 2024 23:14
3037478
to
db57bfe
Compare
jellefoks
requested review from
y4vor,
kaidokert,
andrewsavage1 and
aee-google
October 15, 2024 23:19
kaidokert
reviewed
Oct 15, 2024
kaidokert
reviewed
Oct 15, 2024
jellefoks
force-pushed
the
http3_opts_part_one_b_connect
branch
from
October 15, 2024 23:31
db57bfe
to
3161990
Compare
kaidokert
reviewed
Oct 15, 2024
kaidokert
approved these changes
Oct 15, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm - questions /nitpicks nonwithstanding. Great improvement if this proves out !
This comment was marked as resolved.
This comment was marked as resolved.
jellefoks
force-pushed
the
http3_opts_part_one_b_connect
branch
from
October 15, 2024 23:41
3161990
to
d58a033
Compare
kaidokert
reviewed
Oct 15, 2024
Allow Cobalt to connect UDP sockets. This saves kernel CPU time. In the download benchmark page, this results in ~7% higher achieved bandwidth and ~5% less total CPU usage.
jellefoks
force-pushed
the
http3_opts_part_one_b_connect
branch
from
October 16, 2024 00:10
d58a033
to
f873293
Compare
Submitting - the test failures are due to IPv6 and prior build breakage on devices. We'll need to fix those separately |
jellefoks
added a commit
to jellefoks/cobalt
that referenced
this pull request
Oct 16, 2024
Since youtube#4270, UDP socket connections are no longer silently ignored. Starboard can not return ERR_ADDRESS_UNREACHABLE and will instead return ERR_FAILED. This changes UDPSocketTest.ClientGetLocalPeerAddresses to allow that return value for ipv6 connections. b/373726636 b/205134049
jellefoks
added a commit
that referenced
this pull request
Oct 16, 2024
Since #4270, UDP socket connections are no longer silently ignored. Starboard can not return ERR_ADDRESS_UNREACHABLE and will instead return ERR_FAILED. This changes UDPSocketTest.ClientGetLocalPeerAddresses to allow that return value for ipv6 connections. b/373726636 b/205134049
madhurajayaraman
added a commit
that referenced
this pull request
Oct 29, 2024
This reverts commit d955af2.
jellefoks
added a commit
that referenced
this pull request
Oct 31, 2024
Starboard builds that don't include #4270 hard assert when sending a UDP packet without a destination address, even on connected sockets. This fixes that by supplying the remote address, unless the `"dev.cobalt.extension.SocketReceiveMultiMsg"` extension from #4321 is also implemented for the platform. As a result, platforms that implement the extension will still get the full performance benefit of relying on connected UDP sockets. b/374155470 b/373726636 b/205134049
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.
Allow Cobalt to connect UDP sockets. This saves kernel CPU time. In the download
benchmark page, this results in ~7% higher achieved bandwidth and ~5% less total
CPU usage.
Since this will also detect unreachable IP networks sooner, it may also help with connection latency in some networks.
b/373726636
b/205134049