Skip to content
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

use SO_RCVBUFFORCE to force receive buffer increase on Linux #3804

Merged
merged 7 commits into from
May 8, 2023

Conversation

MarcoPolo
Copy link
Collaborator

@MarcoPolo MarcoPolo commented May 5, 2023

Closes #3795

@@ -17,3 +22,23 @@ const (
)

const batchSize = 8 // needs to smaller than MaxUint8 (otherwise the type of oobConn.readPos has to be changed)

func forceSetReceiveBuffer(c interface{}, bytes int) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no downside to always calling SO_RCVBUFFORCE, is there?

If that's the case, you could just do it as part of setReceiveBuffer, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple a reasons to keep it here:

  1. unix.SO_RCVBUFFORCE is only defined for GOOS=linux. So this would fail if we put it in setReceiveBuffer as is, since that's shared.
  2. Having it as a separate function lets us test only this thing. However, I'm not sure this will actually work since CI runner might not have CAP_NET_ADMIN (I don't when running this as a normal user).

@MarcoPolo MarcoPolo force-pushed the marco/inc-recv-buf-with-gusto branch from 854bd99 to a85cc72 Compare May 5, 2023 21:30
@MarcoPolo MarcoPolo force-pushed the marco/inc-recv-buf-with-gusto branch from a85cc72 to b7f7f78 Compare May 5, 2023 21:31
@codecov
Copy link

codecov bot commented May 6, 2023

Codecov Report

Merging #3804 (baca871) into master (6b74a9a) will increase coverage by 0.17%.
The diff coverage is 100.00%.

❗ Current head baca871 differs from pull request most recent head 9b08488. Consider uploading reports for the commit 9b08488 to get more accurate results

@@            Coverage Diff             @@
##           master    #3804      +/-   ##
==========================================
+ Coverage   83.97%   84.14%   +0.17%     
==========================================
  Files         142      141       -1     
  Lines       14208    14000     -208     
==========================================
- Hits        11930    11779     -151     
+ Misses       1846     1795      -51     
+ Partials      432      426       -6     
Impacted Files Coverage Δ
sys_conn_helper_nonlinux.go 100.00% <100.00%> (ø)
transport.go 64.07% <100.00%> (-1.59%) ⬇️

... and 2 files with indirect coverage changes

@marten-seemann marten-seemann force-pushed the marco/inc-recv-buf-with-gusto branch 3 times, most recently from 9b08488 to 4644cb9 Compare May 8, 2023 09:15
@marten-seemann marten-seemann force-pushed the marco/inc-recv-buf-with-gusto branch from 4644cb9 to 103e683 Compare May 8, 2023 09:20
@marten-seemann marten-seemann marked this pull request as ready for review May 8, 2023 09:29
Copy link
Member

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this @MarcoPolo!

One minor change was still needed: We need to ignore the error from SetReadBuffer. Other than that, I modified the CI setup a bit, so that we can test this (it's even included in Codecov, which is pretty cool).

@marten-seemann
Copy link
Member

marten-seemann commented May 8, 2023

Merging this now, since I need to build on top of it to increase the send buffer as well (#3811), and this is blocking progress on the GSO PR.

@marten-seemann marten-seemann changed the title Add ability to change the receive buffer size using SO_RCVBUFFORCE in Linux use SO_RCVBUFFORCE to force receive buffer increase on Linux May 8, 2023
@marten-seemann marten-seemann merged commit 843b633 into master May 8, 2023
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use SO_RCVBUFFORCE to increase receive buffer size
2 participants