Socket buffer controls to solve bufferbloat. #241
Conversation
|
@cbeuw i have been using this 1 year+ without issue can we get merge ? |
|
I will review this one more time to be 100% absolutely sure |
|
Sorry for leaving this, I will give it a proper look at in the next day or so, testing it together with uTLS updates |
| data, after which the connection will be closed by Cloak. Cloak will not enforce any timeout on TCP connections after it | ||
| is established. | ||
|
|
||
| `LoopbackTcpSendBuffer` is the number of bytes to use for the tcp loopback send buffer. Use a low value like 4096 to reduce upload bufferbloat on client. |
There was a problem hiding this comment.
Given that this parameter controls the send buffer from ck-client to e.g. shadowsocks client, shouldn't this be relevant to download instead of upload? And LoopbackTcpReceiveBuffer below should affect upload
There was a problem hiding this comment.
There's cases where cloak is used in tunnel mode not with ss, so user would upload to cloak and then to cloak server directly, not to ss first. This parameter is made for that case.
For usage with ss, it will be ss local receive that needs clamping, in such case buffer between ss client and ck-client will also need clamping (either from ss send side or ck receive side), I have implemented that in ss-rust in the past and it's part it for some time now already.
There was a problem hiding this comment.
There's cases where cloak is used in tunnel mode not with ss, so user would upload to cloak and then to cloak server directly, not to ss first.
But even in this case, uploaded data from user would fill up the receive buffer in ck-client, rather than send buffer, right?
There was a problem hiding this comment.
Yes, that's why the ck-client tcp receive needs to be clamped. I put controls for all 4 sides, local send/ local receive / remote send / remote receieve for client and server.
The general principle is all the parts that are on the same machine need to have clamped buffers, and only the ends exposed to the remote internet need to have open buffers, otherwise, bufferbloat will happen.
There was a problem hiding this comment.
I understand the need for clamping, my issue is with the line
Use a low value like 4096 to reduce upload bufferbloat on client.
This should describe LoopbackTcpReceiveBuffer, not here.
There was a problem hiding this comment.
It may be better to name these LocalTcpUploadBuffer, LocalTcpDownloadBuffer, RemoteTcpUploadBuffer and RemoteTcpDownloadBuffer
There was a problem hiding this comment.
Yes renaming it makes sense it was very confusing even to me. I will check and update this PR.
There was a problem hiding this comment.
For client the rename may make sense but for the server not. Because loopback for server point of view means any direction of socket that's connected on loopback IP whether it's to the "proxybook" or to the "incoming client"
The consideration of clamping the buffers applies mainly to Loopback IP in any side of the connection. To remove confusion I will implement the 4 properties for the server as well. Where for server local is to the proxy book and remote is to the client. The unification of terminology will make things simpler.
|
I tested my changs again and it works fine |
notsure2
force-pushed
the
tcp-buffer-controls
branch
from
c7d1b5d to
c3cde8d
Compare
2 participants
This adds multiple parameters to config (documented in README.md) which allow precise and control control over socket buffers.
For incoming, buffer must be set on listener socket, and for outgoing it must be set before connection.