Open
Conversation
This code is very closely based from the chrome QUIC C++ code (aka google/proto-quic), Thankfully this code base is very similar in it's layout to proto-quic making it a easy-ish port. For some of the missing holes, the Linux Kernel was looked at. It is worth pointing out that there are some core differences between this implementation and proto-quic, due to the fact that proto-quic has a lot more things passed down to it than this version has. To enable ease of merge and intergration I have decided to avoid doing that and instead (ab)use other bits of info that quic-go gives to it's congestion control code. In the end a bandwidth sampler needed to be implemented, and a good window filter was done as well. I *think* there already is another windowfilter in quic-go but it was not usable at the time. This commit does not impact quic-go in anyway since there is no intergration to it yet. This commit simply serves as the root commit to the BBR implementation.
You can now request to use BBR with the quic.Config{} section.
Since the config section is not passed down to the function that
makes the Y/N call on what congestion (there was only one until
now) code to use, I passed down the bool only. Depending on how
people feel about things might mean it's better to simply pass
down the whole config section. But that feels to me excessive.
Not holding any strong views though, up for doing it either way tbh.
... Otherwise BBR will never get setup correctly.
Found using delve and a core dump of a production PID:
```
(dlv) print s
*github.com/lucas-clemente/quic-go.session {
handshakeDestConnID: github.com/lucas-clemente/quic-go/internal/protocol.ConnectionID len: 4, cap: 4, [83,254,26,34],
origDestConnID: github.com/lucas-clemente/quic-go/internal/protocol.ConnectionID len: 0, cap: 0, nil,
retrySrcConnID: *github.com/lucas-clemente/quic-go/internal/protocol.ConnectionID nil,
srcConnIDLen: 4,
perspective: PerspectiveServer (1),
initialVersion: 0,
version: VersionTLS (4278190109),
config: *github.com/lucas-clemente/quic-go.Config {
Versions: []github.com/lucas-clemente/quic-go/internal/protocol.VersionNumber len: 1, cap: 1, [VersionTLS (4278190109)],
ConnectionIDLength: 4,
HandshakeTimeout: net/http.http2prefaceTimeout (10000000000),
MaxIdleTimeout: google.golang.org/grpc/credentials/google.tokenRequestTimeout (30000000000),
AcceptToken: github.com/lucas-clemente/quic-go.glob..func1,
TokenStore: github.com/lucas-clemente/quic-go.TokenStore nil,
MaxReceiveStreamFlowControlWindow: 6291456,
MaxReceiveConnectionFlowControlWindow: 15728640,
MaxIncomingStreams: 1000,
MaxIncomingUniStreams: 100,
StatelessResetKey: []uint8 len: 0, cap: 0, nil,
KeepAlive: false,
UseBBR: false,
```
However in the root goroutine:
```
(dlv) print p
*github.com/getlantern/http-proxy-lantern/v2.Proxy {
TestingLocal: true,
HTTPAddr: (unreadable could not read string at 0x4 due to error while reading spliced memory at 0x4: EOF),
HTTPMultiplexAddr: "",
HTTPUTPAddr: "\x00\x00\x00\x00",
BordaReportInterval: 4,
BordaSamplePercentage: 0,
...
QUICUseBBR: true,
OQUICAddr: "",
```
Me and forkner tracked this down in DM's, This should fix the issue
Forgot to take these out before submitting the "final" POC PR. Woops.
Crosse
reviewed
Aug 29, 2022
Crosse
left a comment
Crosse
left a comment
There was a problem hiding this comment.
I assume this is a straight-up merge from upstream, so if this compiles and does useful things, it looks good to me.
clicks Approve, runs away
Author
|
It works but I don't think the workflows should have remained. I don't know why deletes aren't retained across merges and I get an empty file. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
No description provided.