quic: initial experimental quic implementation

[jasnell]

This is the initial QUIC implementation. It is experimental, it is still unfinished, and it's complicated.

To get started with this, use ./configure --experimental-quic or vcbuild experimental-quic to build with the quic support enabled.

EXPECT BUGS!!

This adds two new dependencies: ngtcp2 and nghttp3.

This also patches openssl to add the new boringssl quic apis. This is a floated patch that hopefully we'll be able to drop later.

It is semver-major because of the additional top level module and the changes to openssl (there are no functional changes to openssl if quic is not being used)

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[jasnell]

Some initial details to get started...

This PR adds two new additional dependencies: ngtcp2 and nghttp3, and patches openssl for quic support.

• ngtcp2 implements the core of the QUIC protocol on our behalf. There are extremely complex state management requirements, protocol flow, serialization, etc that this library handles for us.
• nghttp3 implements the http3 specific bits for us. It is similar to nghttp2, and there are some areas of overlap, but it is designed specifically to work with ngtcp2. Where there is overall, we have implemented some utility wrappers that can be used to abstract away the differences.

For OpenSSL... QUIC includes built in support for TLS 1.3 but the timing of key generation is different for QUIC than it is for TCP. The BoringSSL project has implemented a new set of QUIC specific APIs that are currently being actively backported to OpenSSL 3.x. The PR, however, has not yet landed in OpenSSL. Since we are running OpenSSL 1.1.1, I further backported the APIs from that open OpenSSL 3 PR to our instance of 1.1.1. Whee! Once we are able to move to OpenSSL 3.x, will be able to avoid floating those patches. When QUIC is not being used, there are no changes to OpenSSL function at runtime. One practical limitation of this is that it means we can only use the bundled version of OpenSSL and cannot use the shared version.

Additional information in a later update.

[ronag]

Shouldn't this happen in _destroy or inside an event listener for finish & end?

e.g.

function onEndFinish () {
  if (this.writableFinished && this.readableEnded) {
    this[kHandle].resetStream(code, family);
  }
}
this.on('end', onEndFinish);
this.on('finish', onEndFinish);

Or something along those lines...

[ronag]

Why is this read() call required?

[jasnell]

@ronag ... some important bits to think about with regards to the QuicStream implementation...

1. The lifespan of the QuicStream Duplex is a bit different than other streams instances. Specifically, we cannot use autoDestroy because we cannot destroy the QuicStream immediately when the writable and readable sides end -- we must first wait for confirmation from the underlying C++ layer that the underlying Stream data has been successfully processed and ngtcp2 is done with it. A separate signal will be triggered that causes the QuicStream to be destroyed.

2. As with all of the QUIC objects, calling destroy() immediately and synchronously causes the destruction of the underlying C++ handle objects. Unlike elsewhere throughout core where destruction of the handle is often deferred to nextTick or setImmediate, everything in QUIC is implemented to destroy immediately.

3. The way the current JS Streams + StreamBase API is currently implemented puts an artificial speed limit on how fast the QUIC implementation can work. Specifically, to avoid memcpy operations at the C++ layer, we take the array of uv_buf_t instances given to us by writev and stick those directly into a linked list internally. We are required by QUIC to keep those in memory until we receive an acknowledgement from the remote peer and ngtcp2 tells us that it's ok to free those. Because we are not copying, we cannot invoke the callback on the WriteReq until the acknowledgement is received. The current stream.Writable implementation forbids us from having more than one outstanding callback at a time, which means at any given time, the QUIC implementation can only have a maximum of one outstanding WriteReq. This can be fixed by adding an option to stream.Writable that permits a non-buffered immediate pass-through mode, which I will be working on soon after we get the core QUIC functionality landed.

4. QUIC streams are either Bidirectional or Unidirectional. A Bidirectional stream is writable and readable in both directions regardless of which peer initiated it. A unidirectional stream is writable only by the peer that initiated it and readable only by the non-initiating peer. QUIC streams may, at any time, be half open and may, at any time, be abruptly closed.

[ronag]

@jasnell:

1. Isn't that the purpose of _final? And on the readable side can't we delay the 'end' event? Or do you want to avoid delaying the 'finish'/'end' events? Is the extra complexity worth that latency reduction? Maybe we could start with the "simple" non-optimized version and later improve streams in a generic way?

2. Can't that be an implementation detail of _destroy? I'm not sure why this is a problem or needs to be given special consideration?

3. Fixing Writable sounds like a great idea. I'm more than happy help where I can. I think we could use a passthrough mode for OutgoingMessage in http as well. Something I've also been considering.

forbids us from having more than one outstanding callback at a time

Is it possible at least partly get around that with writev? Again, I would personally prefer the "slower" (within reason) way until a generic solution as you suggest could be found in "standard" streams.

4. How does that differ from regular net.Socket? Can we make it behave the same?

Btw no hurry with responding to my comments. I don't mind if we take this whenever you feel is appropriate in terms of time and where you are currently focusing your attention.

I would very much like to avoid making "hacks" around streams and where possible either make it work with current streams or update streams to better support the cases you require here.

[jasnell]

@nodejs/collaborators @nodejs/tsc @nodejs/quic ... this is ready for detailed review. Note that changes are not tested in CI yet because there is no configuration yet that uses the --experimental-quic compile flag (/cc @nodejs/build)

[jasnell]

@nodejs/collaborators @nodejs/tsc ... to help make reviewing easier, I'm happy to jump on a vscode liveshare session with folks to give them a walkthrough.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/29785/
CI (--experimental-quic): https://ci.nodejs.org/job/node-test-commit/36493/

[jasnell]

Couple of relevant failures in CI.

• test-code-cache - fix quic: fixup test code cache quic#359
• test-fs-chown - fix quic: fixup test-fs-chown quic#360

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/29815/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/29871/

[jasnell]

Ping @nodejs/collaborators @nodejs/tsc ... could definitely use some review on this one. Would very much like to get it landed before the 14.0.0 cut off. I know it's a massive PR and difficult to review so I'm happy to jump on the phone with anyone who wants to go through it in detail.

[sam-github]

It is semver-major because of the additional top level module and the changes to openssl (there are no functional changes to openssl if quic is not being used)

Why would a new feature be semver-major?

Why would an openssl change that has no impact on non-quics use-cases be semver-major?

[sam-github]

Can you restructure these PRs so they are more easily reviewed? It wouldn't substantially increase the complexity of managing, you would still have one branch you can rebase, but you might, occaisonally, need to force-push the two deps branches to sync them with the main one.

Example (not to be commented on, just to show how much more reviewable it becomes):

• quics: openssl sam-github/node#4 - deps: openssl
• quics: ngtcp2 and nghttp3 sam-github/node#5 - deps: ngtcp2 + nghttp3
• quics: comment here sam-github/node#6 - quics, the actual code

[jasnell]

Why would a new feature be semver-major?

Adds a new top level module that shadows an existing module published on the npm registry.

Why would an openssl change that has no impact on non-quics use-cases be semver-major?

Just marked defensively. With the compile time flag in place now it should be fine actually. But see above for why it's still semver-major

Can you restructure these PRs so they are more easily reviewed

Yes, but the concern there is that doing in a separate fork repo pulls the review conversation away from here. Perhaps if folks don't mind I can put the PR branches under this repo so we can keep the conversation here?

[sam-github]

Yes, but the concern there is that doing in a separate fork repo pulls the review conversation away from here. Perhaps if folks don't mind I can put the PR branches under this repo so we can keep the conversation here?

I agree, and to be clear, that is exactly my suggestion/request.

Where the branches are doesn't matter to the PR structure (though putting them all in nodejs/node sounds fine to me), but all 3 PRs would have to be against nodejs/node so the conversation can be here.

The deps:openssl and deps:ng PRs could be "draft" (though the openssl in particular could land before the next 2, or not, whatever ends up being convenient).

[sam-github]

I assume its https://www.npmjs.com/package/quic that is the problem, not https://www.npmjs.com/package/http3, because the latter sounds like they would be willing to give it to Node.js.

Exporting as net/quic would solve this, as would adding a no-op quic module into Node.js right now, so its there in v14.0.0 and would prevent https://www.npmjs.com/package/quic from being importable (the semver-major part). The feature could become available later, at our leisure.

I'm just concerned that v14.0.0 not unnecessarily become a deadline for landing.

[jasnell]

Given that the quic module itself really is just a single export, I've been considering forgoing the new top level module entirely and just exporting it from net ... e.g. const createQuicSocket = require('net')

[sam-github]

Plausible, though I assume there are a raft of methods associated with the returned object, the top-level module also lends itself to organizing docs, net is pretty full already.

[jasnell]

The main implementation is separated out under lib/internal/quic so there's very little directly in the require('quic') module. And I think it would still be good to separate it out into it's own quic.md even if it is in net.js. It's a departure from the normal pattern but worthwhile.

[addaleax]

Given that the quic module itself really is just a single export, I've been considering forgoing the new top level module entirely and just exporting it from net ... e.g. const createQuicSocket = require('net')

I like that.

[jasnell]

Closing this PR in favor of three separate PRs that will (hopefully) make this whole thing easier to review (cc @sam-github )

• Part 1: OpenSSL changes: quic: part 1... quic-modified openssl #32377
• Part 2: ngtcp2/nghttp3: quic: part 2... ngtcp2 and nghttp3 #32378
• Part 3: QUIC implementation: quic: part 3... initial quic implementation #32379

Please move discussion to those PRs.

[Manny27nyc]

Please help if there is any problems.