mobile: Add an option to set the UDP socket send and receive buffer sizes #35190
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
|
/retest |
2 similar comments
|
/retest |
|
/retest |
fredyw
force-pushed
the
send_buffer_size
branch
7 times, most recently
from
b1b56de to
2d5bf8e
Compare
|
/retest |
fredyw
force-pushed
the
send_buffer_size
branch
13 times, most recently
from
87c8057 to
a084d8d
Compare
|
/retest |
1 similar comment
|
/retest |
fredyw
force-pushed
the
send_buffer_size
branch
2 times, most recently
from
a0b0820 to
5b5f1c1
Compare
fredyw
changed the title
Signed-off-by: Fredy Wijaya <[email protected]>
|
/retest |
fredyw
requested review from
RyanTheOptimist,
alyssawilk and
abeyad
as code owners
|
/assign @alyssawilk |
alyssawilk
reviewed
Jul 25, 2024
| udp_snd_buf_sock_opt->set_name(SO_SNDBUF); | ||
| udp_snd_buf_sock_opt->set_level(SOL_SOCKET); | ||
| udp_snd_buf_sock_opt->set_int_value(udp_socket_send_buffer_size_); | ||
| udp_snd_buf_sock_opt->mutable_type()->mutable_datagram(); |
There was a problem hiding this comment.
Mind adding a comment
// Make sure this option is only applied to UDP sockets and not TCP
mobile/library/cc/engine_builder.cc
Outdated
| @@ -734,6 +739,15 @@ std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> EngineBuilder::generate | |||
| rcv_buf_sock_opt->set_int_value(socket_receive_buffer_size_); | |||
There was a problem hiding this comment.
change this to datagram only?
There was a problem hiding this comment.
Initially I wanted to do it in a separate PR, but the change is small enough that I can do it in this PR. Done.
Signed-off-by: Fredy Wijaya <[email protected]>
fredyw
changed the title
alyssawilk
approved these changes
Jul 25, 2024
martinduke
pushed a commit
to martinduke/envoy
that referenced
this pull request
Aug 8, 2024
…izes (envoyproxy#35190) This PR adds an option in the `EngineBuilder` to set the UDP socket send buffer size. By default it will use the same value as the one set in Chromium: https://source.chromium.org/chromium/chromium/src/+/main:net/quic/quic_session_pool.cc;l=790-793;drc=7f04a8e033c23dede6beae129cd212e6d4473d72 This PR also updates the code to only apply the default receive buffer size to UDP and not TCP given that the code in https://source.chromium.org/chromium/chromium/src/+/main:net/quic/quic_session_pool.cc;l=766;drc=7f04a8e033c23dede6beae129cd212e6d4473d72 only applies the socket option to UDP only. Risk Level: low Testing: unit tests Docs Changes: n/a Release Notes: n/a Platform Specific Features: mobile --------- Signed-off-by: Fredy Wijaya <[email protected]> Signed-off-by: Martin Duke <[email protected]>
asingh-g
pushed a commit
to asingh-g/envoy
that referenced
this pull request
Aug 20, 2024
…izes (envoyproxy#35190) This PR adds an option in the `EngineBuilder` to set the UDP socket send buffer size. By default it will use the same value as the one set in Chromium: https://source.chromium.org/chromium/chromium/src/+/main:net/quic/quic_session_pool.cc;l=790-793;drc=7f04a8e033c23dede6beae129cd212e6d4473d72 This PR also updates the code to only apply the default receive buffer size to UDP and not TCP given that the code in https://source.chromium.org/chromium/chromium/src/+/main:net/quic/quic_session_pool.cc;l=766;drc=7f04a8e033c23dede6beae129cd212e6d4473d72 only applies the socket option to UDP only. Risk Level: low Testing: unit tests Docs Changes: n/a Release Notes: n/a Platform Specific Features: mobile --------- Signed-off-by: Fredy Wijaya <[email protected]> Signed-off-by: asingh-g <[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.
This PR adds an option in the
EngineBuilderto set the UDP socket send buffer size. By default it will use the same value as the one set in Chromium: https://source.chromium.org/chromium/chromium/src/+/main:net/quic/quic_session_pool.cc;l=790-793;drc=7f04a8e033c23dede6beae129cd212e6d4473d72This PR also updates the code to only apply the default receive buffer size to UDP and not TCP given that the code in https://source.chromium.org/chromium/chromium/src/+/main:net/quic/quic_session_pool.cc;l=766;drc=7f04a8e033c23dede6beae129cd212e6d4473d72 only applies the socket option to UDP only.
Risk Level: low
Testing: unit tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: mobile