Skip to content

libproxy: allow read/write buffers to be set#582

Merged
djs55 merged 2 commits intomoby:masterfrom
djs55:buffer
Jun 9, 2022
Merged

libproxy: allow read/write buffers to be set#582
djs55 merged 2 commits intomoby:masterfrom
djs55:buffer

Conversation

@djs55
Copy link
Copy Markdown
Collaborator

@djs55 djs55 commented May 17, 2022

If the link has high latency then it's useful to use larger buffers

Signed-off-by: David Scott dave.scott@docker.com

@djs55 djs55 requested a review from fredericdalleau June 9, 2022 07:09

// SetReadBuffer sets the size of the operating system's receive buffer associated with the connection.
// See similar function https://pkg.go.dev/net#IPConn.SetReadBuffer
func (c *channel) SetReadBuffer(bytes int) error {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The parameter should be unsigned. Otherwise if the user sets -1 here, it becomes havoc

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ha I understand why it was int it can be setsockopt, it's bounded and can fail.

SO_RCVBUF
Sets or gets the maximum socket receive buffer in bytes.
The kernel doubles this value (to allow space for
bookkeeping overhead) when it is set using setsockopt(2),
and this doubled value is returned by getsockopt(2). The
default value is set by the
/proc/sys/net/core/rmem_default file, and the maximum
allowed value is set by the /proc/sys/net/core/rmem_max
file. The minimum (doubled) value for this option is 256.
boucane:~$ cat /proc/sys/net/core/rmem_max
212992


// SetWriteBuffer sets the size of the operating system's write buffer associated with the connection.
// See similar function https://pkg.go.dev/net#IPConn.SetWriteBuffer
func (c *channel) SetWriteBuffer(bytes int) error {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ditto

djs55 added 2 commits June 9, 2022 10:37
If the link has high latency then it's useful to use larger buffers

Signed-off-by: David Scott <dave.scott@docker.com>
Signed-off-by: David Scott <dave@recoil.org>
Copy link
Copy Markdown
Collaborator

@fredericdalleau fredericdalleau left a comment

Choose a reason for hiding this comment

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

LGTM

@djs55 djs55 merged commit f3622e8 into moby:master Jun 9, 2022
@djs55 djs55 deleted the buffer branch June 9, 2022 13:23
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.

2 participants