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

proposal: x/net/quic: add QUIC implementation #58547

Open
neild opened this issue Feb 15, 2023 · 54 comments
Open

proposal: x/net/quic: add QUIC implementation #58547

neild opened this issue Feb 15, 2023 · 54 comments
Labels
Milestone

Comments

@neild
Copy link
Contributor

neild commented Feb 15, 2023

I propose adding an implementation of the QUIC transport protocol (RFC 9000) in golang.org/x/net/quic. QUIC is the protocol underlying HTTP/3, and QUIC support is a necessary prerequisite for HTTP/3 support.

The proposed API is in https://go.dev/cl/468575. This API does not include support for Early Data, but does not preclude adding that support at a later time.

RFC 9000 does not define a QUIC API, but it does define a set of operations that can be performed on a QUIC connection or stream.

A QUIC connection is shared state between a client and server.

  • open a [client] connection:

    conn, err := quic.Dial(ctx, "udp", "127.0.0.1:8000", &quic.Config{})
  • listen for incoming connections:

    l, err := quic.Listen("udp", "127.0.0.1:8000", &quic.Config{})
    conn, err := l.Accept(ctx)

A QUIC stream is an ordered, reliable byte stream. A connection may have many streams. (A QUIC stream is loosely analogous to a TCP connection.)

  • create streams:

    s, err := conn.NewStream(ctx)
  • accept streams created by the peer:

    s, err := conn.AcceptStream(ctx)
  • read from, write to, and close streams:

    n, err = s.Read(buf)
    n, err = s.Write(buf)
    s.Close()
  • stream operations also have Context-aware variants:

    n, err = s.ReadContext(ctx, buf)
    n, err = s.WriteContext(ctx, buf)
    s.CloseContext(ctx)
  • data written to streams is buffered, and may be explicitly flushed:

    // Sends one datagram, not 100.
    for i := byte(0); i < 100; i++ {
      s.Write([]byte{i})
    }
    // Data will not be sent until a datagram's worth has been accumulated or an explicit flush.
    // No Nagle's algorithm.
    s.Flush()

See https://go.dev/cl/468575 for the detailed API.

@neild neild added the Proposal label Feb 15, 2023
@gopherbot gopherbot added this to the Proposal milestone Feb 15, 2023
@DeedleFake
Copy link

DeedleFake commented Feb 15, 2023

I generally like the use of context.Context over the Deadline() methods of the regular net package, but I think those should still be available. As proposed, quic.Stream doesn't implement net.Conn, for example, though it would probably make sense for it to. Additionally, it probably makes sense for quic.Conn to implement net.Listener as it can accept streams which would then be net.Conns.

Confusingly, it probably does not make sense for quic.Listener to implement net.Listener, as I don't think it would make sense for quic.Conn to implement net.Conn. It could probably be done, though, for example by making the quic.Conn implementation of net.Conn's methods automatically open a single stream on first use. That behavior might be surprising, though.

Edit: Random thought: Might it make sense to introduce new interfaces for the listener -> conn -> stream pattern that QUIC uses? I don't think any other connection scheme is even considering something similar, so maybe not, but it would allow the net package, for example, to provide adapters for things using that pattern, such as the proposed quic package, to let them function as the standard net interfaces in various different ways.

@marten-seemann
Copy link
Contributor

Author of quic-go here 👋
quic-go is a QUIC implementation in Go that's been around since before QUIC was even standardized. It also comes with an HTTP/3 implementation.

If there's interest from the Go team's side, I'd be happy to talk about what would be needed to get (parts of?) quic-go merged into the standard library.

@neild
Copy link
Contributor Author

neild commented Feb 15, 2023

It probably does make sense for quic.Stream to implement net.Conn.

I don't think it's useful for anything in a QUIC implementation to implement net.Listener. A quic.Listener isn't a net.Listener, because it listens for QUIC connections, which are not stream-oriented network connections. A quic.Conn does listen for streams, but while net.Listeners generally accept streams from varying sources a quic.Conn only accepts streams multiplexed over an existing connection with a single entity. I don't think there are many cases where one would want to use a quic.Conn in a place which operates on a net.Listener.

If someone does need to use a quic.Conn as a net.Listener, writing an adapter will be trivial.

@komuw
Copy link
Contributor

komuw commented Feb 16, 2023

Related: #32204 (net/http: support HTTP/3)

@txthinking
Copy link

Is it better to add laddr, same as:

net.DialUDP(network string, laddr, raddr *UDPAddr)
conn, err := quic.DialFunc(ctx, "udp", "laddr", "127.0.0.1:8000", &quic.Config{})

For convenience, there is no need to listen first and then dial :), like here

@rhysh
Copy link
Contributor

rhysh commented Feb 16, 2023

Thank you for sharing this!


type Config struct {
	// TLSConfig is the endpoint's TLS configuraiton.
	// It must be non-nil and include at least one certificate or else set GetCertificate.
	TLSConfig *tls.Config

Does an endpoint that acts as a client need to set a certificate? If so, I'd guess that it's even OK to call quic.Listen without a certificate, if the quic.Listener is only used for Dialing (such as for a high-volume client that doesn't want to use an OS file descriptor for each outbound connection).


func (s *Stream) ReadContext(ctx context.Context, b []byte) (n int, err error) {
func (s *Stream) WriteContext(ctx context.Context, b []byte) (n int, err error) {
func (s *Stream) CloseContext(ctx context.Context) error {

These methods don't allow using normal io packages in a Context-aware way; anything that takes an io.Reader/io.Writer to do buffering, (de)compression, (un)marshaling, etc and wants to also use Context will need a wrapper. And dealing with a Context value in each call looks like it will be expensive (as crypto/tls's implementation of Read vs HandshakeContext saw in #50657).

What are the tradeoffs of this versus a method like func (*Stream) WithContext(context.Context) io.ReadWriteCloser (or func (*Stream) WithContext(context.Context) *ContextStream, to allow adding more methods later)?


This revision of the API doesn't give visibility into flow control. How much is required to build a reliable QPACK implementation? From https://www.rfc-editor.org/rfc/rfc9204.html#section-2.1.3

To avoid these deadlocks, an encoder SHOULD NOT write an instruction unless sufficient stream and connection flow-control credit is available for the entire instruction.


// Close waits for the peer to acknowledge the connection has been closed.
func (c *Conn) Close(ctx context.Context) error {

// Abort closes the connection and returns immediately.
func (c *Conn) Abort(err error) {

Conn.Close and Conn.Abort look similar and not quite orthogonal. I'm not sure if there's a good reason for an app to close a connection with an error and then also wait for the peer to acknowledge, but I don't see how they'd do that with this pair of methods. Maybe call Abort and then Close? Or Abort and Wait (but maybe Abort immediately discards all state)? What's the reason to not have Abort and Close be one method with signature func(context.Context, error) error?


// Dial creates and returns a connection to a network address.
func (l *Listener) Dial(ctx context.Context, network, address string) (*Conn, error) {
	return nil, errors.New("not implemented")
}

Only because you mentioned future possibility of Early Data: Would Dialing with Early Data need a separate quic.Listener method (and package-level function)? Probably one with a signature like this, but which returns a *Conn that is not yet connected, and which needs a subsequent Connect(ctx) method call once the client has created the Early Data streams and filled them with the Early Data. I'm not sure it's important to have a full design now (or even soon), but it wasn't immediately clear to me where the extension point would be.

@mholt
Copy link

mholt commented Feb 16, 2023

@neild I just want to make sure @marten-seemann's comment above was seen. Given the years of effort that have already been spent implementing and optimizing QUIC in Go, it would probably make sense to take advantage of that rather than start over from scratch, even if the exported APIs are a little different.

Please consider using quic-go as the basis for this effort.

@james-lawrence
Copy link
Contributor

james-lawrence commented Feb 21, 2023

@mholt quic-go never tried to integrate with stdlib net packages in a natural way. while I'm sure some of the internal structures might be reusable. the overall library itself wasn't terribly appealing to me personally due to the incompatibilities with the wider ecosystem.

edit: as a result I strongly recommend against using quic-go as a base because of that decision. quic-go focused on getting quic http support available. golang's stdlib implementation should be focused on compatibility with the wide ecosystem at the transport level. the fundmental driving forces for the API design are very different.

@james-lawrence
Copy link
Contributor

james-lawrence commented Feb 21, 2023

@paralin I think the adapter code all of them had to implement to handle quic-go speaks for itself. golang stdlib needs to figure out how to interopt at the transport level. similar to the packet conn vs stream conns it already has.

if I have a stream oriented protocol I shouldn't care if I receive a quic, tcp, or unix transport. this is the problem we need to resolve which quic-go explicitly decided to ignore

@mdlayher
Copy link
Member

This is an active proposal and no decisions have been made as of yet.

Please hold off on speculation about the implementation, and take conversations about quic-go elsewhere.

@james-lawrence
Copy link
Contributor

@mdlayher agreed, but my points about the interopt in stdlib for quic stand. they're important even if we ignore quic-go.

@rsc
Copy link
Contributor

rsc commented Mar 9, 2023

It is good for proposals to focus on API, but implementation is explicitly on topic at least for large proposals, given that it's one of the sections listed in the design doc template.

As for quic-go, I completely agree that it would be good to take advantage of the expertise that @marten-seemann has built up over his years of development of quic-go. We would certainly welcome his help. At the same time, reusing quic-go directly is probably not the right path forward, for a few reasons:

  • As already noted on this issue, there are API questions about how best to present QUIC, and we may well want an API that is different in important ways from quic-go. In particular the quic-go API is fairly low level compared to what we are contemplating.
  • QUIC having been a moving target, it is almost certain that quic-go contains compatibility code for older versions that is no longer needed. A new implementation can target the RFC and modern implementations only.
  • The implementation strategy and approach (some would say code style but I mean something deeper about the code) differs from that of the standard library in a few key respects. In particular it tends toward much more indirection and mocks, while standard library code tends to be more direct. The strategy matters because the Go team will be maintaining the standard QUIC implementation into the distant future.
  • The quic-go tests depend on test frameworks that we cannot depend on in the standard library, so those would need rewriting.
  • The quic-go code has not been reviewed, so we would still have to do a careful line-by-line review as part of bringing it in. Reviewing and revising 75,000+ lines of code is quite possibly more work than writing 75,000 lines from scratch. And a fresh implementation without the history of keeping up with QUIC during its instability may well end up smaller.

For all these reasons, the path forward is almost certainly not to adopt quic-go directly.

@marten-seemann, as I said before, we certainly appreciate your work implementing QUIC to date as well as the expertise you have amassed, and if you would be interested to share that with us in the development and review of a fresh implementation, you'd certainly be welcome. On the other hand, if you would rather focus on quic-go and not a different implementation, we'd understand that too.

@james-lawrence
Copy link
Contributor

main things I'm interested in w/ a quic implementation are exposing ALPN and seamless interopt w/ other standard transports. aka shouldn't have to make extra calls to server http over a quic transport. just setup the quic listener and pass it to http.Serve. when the quic implementation gets to a workable state I'm 100% down to start using it in some of my applications and provide feedback on the API.

@marten-seemann
Copy link
Contributor

marten-seemann commented Mar 10, 2023

@rsc, thank you for your detailed post. It seems like a decision has already been made, but nevertheless, here are my 2c. Happy to share some of my insights from almost 8 years of developing / maintaining a QUIC stack and from having been a member of the IETF QUIC working group since the very beginning.

Building a performant QUIC stack is an absolutely massive endeavor. Getting the handshake to work is nothing more than a tiny first step. When we started the project, it only took us one or two weeks to download a small file from a quic-go server using Chrome via what was back then called H2/QUIC, and most of that time was spent on implementing the bespoke QUIC Crypto.
Implementing the other mandatory parts of the 4 QUIC RFCs (please don't do it the CloudFlare way of not implementing mandatory parts of the RFC) is an enormous amount of work, including the implementation of flow control (both on the stream and the connection level), packet scheduler, a congestion controller and loss detection and the various loss recovery strategies.

This results in a spec-compliant QUIC stack, but in no way an optimized / performant one. You'd probably want to implement

  • Flow control window auto-tuning: without this, a QUIC transfer will never be able to make use of the BDP of common connections on the internet
  • Packet pacing: Absolutely crucial. Without pacing, sending a full cwnd of packets overflows the queues of routers, leads to (avoidable) packet loss and a subsequent collapse of the cwnd
  • The QUIC ACK-frequency extension. Preliminary measurements show that this extension will allow quic-go to make much better use the bandwidth available
  • Optimized syscalls to read and write multiple packets from / to the socket in a single syscall (By the way, can we get net: add UDPMsg, (*UDPConn).ReadUDPMsgs, (*UDPConn).WriteUDPMsgs #45886 some time soon 🙏? This is currently the biggest bottleneck for high-BDP throughput performance in quic-go.)
  • qlog: it will be super hard to do any serious debugging / performance analysis without awesome tools like qvis
  • DPLPMTUD
  • maybe: ECN support, especially as L4S is picking up steam

Maybe not absolutely necessary, but highly desirable:

  • The ability to run multiple QUIC connections (ingoing and outgoing) on the same net.PacketConn. This is quite a powerful feature since you only need a single FD for all your connections, and one of the main reasons the IPFS project started investing in quic-go.
  • QUIC Unreliable Datagrams
  • WebTransport support. The hooks WebTransport requires are no fun.
  • 0-RTT handshakes (not sure if that's part of the plan here)

Aside from all of these features above, we've spent significant engineering efforts on performance optimizations (e.g. reducing the number of allocs) and DoS defense (you're keeping track of a lot of things, e.g. sent and received frames, sent and received packets, etc., and all of these data structures are potentially attackable). As I see it, there's little point in just providing a spec-compliant QUIC implementation, if it can't (at the very least) compete with TCP's performance. And performance work on quic-go is far from done at this point.


Let me briefly comment on the points you made.

  • As already noted on this issue, there are API questions about how best to present QUIC, and we may well want an API that is different in important ways from quic-go. In particular the quic-go API is fairly low level compared to what we are contemplating.

Not sure in what sense quic-go is too low-level. However, quic-go is still on a v0.x version, and we're happy to consider well-motivated API changes. That statement stands independent of the discussion on this issue, and we're happy about proposals how to make the quic-go API work better (please open an issue in quic-go, happy to discuss there!).

  • QUIC having been a moving target, it is almost certain that quic-go contains compatibility code for older versions that is no longer needed. A new implementation can target the RFC and modern implementations only.

We've removed support for QUIC crypto a long time ago. The only compatibility code that we're still maintaining is for draft-29, which we're planning to remove some time in summer this year. The additional code for draft-29 is pretty limited to begin with (mostly just using different labels in the various HKDF expansions), and with one tiny exception doesn't leak beyond the handshake package.

One thing I'm really happy about is finally cleaning up the API between quic-go and crypto/tls. I can't wait to start using the new API (I already have a branch). The API that my crypto/tls fork has accumulated over the years is indeed suboptimal (to say the least). Cleaning it up was always complicated by the fact that I had to maintain two separate forks (for the most recent 2 Go versions) at the same time.

Other than that, I don't think there's any code around that only exists for historical reasons. While minimizing the LOC was never a design target (code clarity and testability was), I don't think there's a lot you can remove without removing features or sacrificing performance.

  • The quic-go tests depend on test frameworks that we cannot depend on in the standard library, so those would need rewriting.

Indeed. In hindsight, that was a bad decision we made when we started the project. Migrating would probably take somewhere around 2 weeks of work to rewrite the tests. Pretty sure that this would still be orders of magnitude less work than rewriting an implementation from scratch.

  • The quic-go code has not been reviewed, so we would still have to do a careful line-by-line review as part of bringing it in. Reviewing and revising 75,000+ lines of code is quite possibly more work than writing 75,000 lines from scratch. And a fresh implementation without the history of keeping up with QUIC during its instability may well end up smaller.

All code has been reviewed by a Googler, @lucas-clemente (not on the Go team though). I don't mean to be nit-picky here, but it's just 24,000 LOC if you exclude tests (and 63,000 if you don't) (counted using cloc). The test suite is indeed quite comprehensive, which allowed me to discover a number of problems (including deadlocks) in the QUIC specification itself during the standardization process, as well as countless bugs in our own code.

I'd also like to point out that quic-go is widely used in production, for example by Caddy (using the HTTP/3 implementation it comes with) and accounts for ~80-90% of all connections in the IPFS network (using just the quic package, without HTTP/3). See here for a (very much incomplete) list of other projects that use it.

It's also tested against a long list of other QUIC implementations using the QUIC Interop Runner which we built a few years ago to facilitate automated interop testing in the QUIC working group.

@marten-seemann, as I said before, we certainly appreciate your work implementing QUIC to date as well as the expertise you have amassed, and if you would be interested to share that with us in the development and review of a fresh implementation, you'd certainly be welcome. On the other hand, if you would rather focus on quic-go and not a different implementation, we'd understand that too.

Happy to help, in one way or the other. You know where to find me :)

@neild
Copy link
Contributor Author

neild commented Mar 10, 2023

@rhysh

Does an endpoint that acts as a client need to set a certificate?

No; fixed the documentation for Config.TLSConfig.


func (s *Stream) ReadContext(ctx context.Context, b []byte) (n int, err error) {
func (s *Stream) WriteContext(ctx context.Context, b []byte) (n int, err error) {
func (s *Stream) CloseContext(ctx context.Context) error {

These methods don't allow using normal io packages in a Context-aware way; anything that takes an io.Reader/io.Writer to do buffering, (de)compression, (un)marshaling, etc and wants to also use Context will need a wrapper. And dealing with a Context value in each call looks like it will be expensive (as crypto/tls's implementation of Read vs HandshakeContext saw in #50657).

There are three types of API for cancellable read/write operations in common use that I know of:

  1. The net.Conn style of a type which implements io.ReadWriter with a separate Set(Read|Write)Deadline.
  2. Functions that accept a context.Context.
  3. A function that curries a context, returning an io.ReadWriter: s.WithContext(ctx).Read(p).

I believe we need to support the first one for compatibility with net.Conn. We should also support context-based cancellation, so that means at least two overlapping APIs.

I don't have a strong opinion about which of the latter two options is best (s.ReadContext(ctx, p) vs s.WithContext(ctx).Read(p)), but ReadContext is a bit less indirect and it's simple to write a context-currying adapter in terms of it.

Another possibility if #57928 is accepted (not strictly necessary, but necessary to implement this efficiently) might be:

// SetCancelContext arranges for operations on the stream to be interrupted if the provided context is canceled.
// After the context is canceled, calls to I/O methods such as Read and Write will return the context error.
// A a nil value for ctx means operations will not be interrupted.
func (s *Stream) SetCancelContext(ctx context.Context) {}

This revision of the API doesn't give visibility into flow control. How much is required to build a reliable QPACK implementation?

This is an excellent question. I left flow control out of the initial proposal because I'm not completely satisfied with any of the ideas I've had so far.

My current inclination is to have a per-stream configuration option that makes writes to the stream effectively atomic--a write will block until flow control is available to send the entire write, sending either the entire chunk of data or none of it.

s.SetAtomicWrites()
n, err := s.Write(data)
// If err is nil, n==len(data).
// If err is non-nil, n==0.

I'd be interested to hear other ideas.


Conn.Close and Conn.Abort look similar and not quite orthogonal. I'm not sure if there's a good reason for an app to close a connection with an error and then also wait for the peer to acknowledge, but I don't see how they'd do that with this pair of methods. Maybe call Abort and then Close? Or Abort and Wait (but maybe Abort immediately discards all state)? What's the reason to not have Abort and Close be one method with signature func(context.Context, error) error?

To close a connection with an error and wait for the peer to acknowledge:

c.Abort(ConnClosedError{Code: code})
err := c.Wait(ctx)

I think you might be right that combining Abort and Close into a single func(context.Context, error) error method is better. I'll think about that some more.


Only because you mentioned future possibility of Early Data: Would Dialing with Early Data need a separate quic.Listener method (and package-level function)?

I think we can do Early Data almost entirely within the proposed API.

  • We add a Config.EnableEarlyData option.
  • When EnableEarlyData is on, creating a new client stream doesn't immediately start the handshake if 0-RTT state is available. The user can create new streams and write to them as usual, with data buffered locally. When the early data buffer fills or when the user explicitly flushes the connection, we send the handshake and pending data in 0-RTT packets. If the server rejects early data, we resend the discarded 0-RTT data in 1-RTT.
  • On the server side, when EnableEarlyData is on, accepted client streams may include early data. There will be a method for querying a stream to see if its receiving early data, and possibly a method the user needs to call to explicitly acknowledge that they're receiving early data before they can read from the stream.

But I haven't tried to implement this, and might be missing something.

@gopherbot
Copy link

Change https://go.dev/cl/475435 mentions this issue: quic: add various useful common constants and types

@gopherbot
Copy link

Change https://go.dev/cl/468402 mentions this issue: quic: add internal/quic package

@gopherbot
Copy link

Change https://go.dev/cl/475437 mentions this issue: quic: basic packet operations

@gopherbot
Copy link

Change https://go.dev/cl/475436 mentions this issue: quic: packet number encoding/decoding

@gopherbot
Copy link

Change https://go.dev/cl/475438 mentions this issue: quic: packet protection

@neild
Copy link
Contributor Author

neild commented Mar 10, 2023

just setup the quic listener and pass it to http.Serve.

To be clear: This will not work. QUIC is not TCP. A quic.Listener listens for QUIC connections, where a QUIC connection can multiplex any number of TCP-like streams. There isn't any concept in QUIC which corresponds well to a net.Listener.

In addition, while it would be possible to run HTTP/1 over QUIC streams, nobody (so far as I know) does this. HTTP/3 uses QUIC as an underlying transport, but HTTP/3 is not just HTTP/1 with TCP swapped out for QUIC.

@rhysh
Copy link
Contributor

rhysh commented Mar 10, 2023

There are three types of API for cancellable read/write operations in common use that I know of:

  1. The net.Conn style of a type which implements io.ReadWriter with a separate Set(Read|Write)Deadline.
  2. Functions that accept a context.Context.
  3. A function that curries a context, returning an io.ReadWriter: s.WithContext(ctx).Read(p).

I believe we need to support the first one for compatibility with net.Conn. We should also support context-based cancellation, so that means at least two overlapping APIs.

I don't have a strong opinion about which of the latter two options is best (s.ReadContext(ctx, p) vs s.WithContext(ctx).Read(p)), but ReadContext is a bit less indirect and it's simple to write a context-currying adapter in terms of it.

Yes, ReadContext is more direct when it's called directly, and writing a context-currying adapter is simple. But users of io.Copy, io.ReadFull, gzip.NewReader, fmt.Fprintf, etc who also want to use context will need to write and use that adaptor every time, or (I've found) will end up with project-specific implementations like myio.ReadFullWithContext.

The adaptor is simple to write in either direction (2 to 3 or 3 to 2). The io.Reader and io.Writer interfaces are extremely widespread, and currying means any interaction with the context (including looking up values) can be amortized across more calls and more bytes.

It seems that API 1 can also be built into API 3, where the value that WithContext returns implements net.Conn, but doesn't allow setting a deadline farther out than the deadline that was attached to the context. Users who don't want to deal with context at all can use the result of s.WithContext(context.Background()).

For what it's worth, I'd also expect multiple calls to WithContext with different arguments to return independent results, to allow independent control of Read versus Write deadlines. That would be hard to express in an API like SetCancelContext.

I agree though that it's not clear which of these APIs is best. Most of all I'd like one that aligns with performance: where it's easy to implement and use in an efficient way and hard to end up with a bunch of great code that can't be made to go fast.


My current inclination is to have a per-stream configuration option that makes writes to the stream effectively atomic--a write will block until flow control is available to send the entire write, sending either the entire chunk of data or none of it.

That sounds pretty simple to use, nice!

If it allows writes that are larger than a single packet, and some pacing is active, and the user cancels the write via SetWriteDeadline or a context—that would leak through the abstraction, which is probably fine but would take some careful doc-writing to explain.

I'd be interested to hear other ideas.

IIUC there are three protocol-level limits that determine whether a particular STREAM frame is allowed on the wire: stream-level flow control, connection-level flow control, and connection-level congestion control (which seems closely related to any pacing in place). It looks like QPACK specifies a need for interacting with the first two. I expect it's unusual to need to interact with the third, but I have an application that takes advantage of visibility into that: it prioritizes which data to send, and sometimes whether to send any data at all, based on how soon it expects the QUIC stack will be able to send it to the peer. (Maybe some of that is better left to an integration with the packet scheduler.)

I wonder if there's room for an API along the lines of https://pkg.go.dev/golang.org/x/time/rate#Limiter.ReserveN that gives an app a higher level of visibility into and control of those windows, through an API that returns a struct with methods that allow inspecting and manipulating (and canceling) the reservation. It's definitely an "other idea"; I don't know whether it's better than your SetAtomicWrites proposal.

// TryReserve reserves up to n bytes of stream- and connection-level flow control, ear-marking
// it for use with s. If fewer than n bytes are available, TryReserve claims them all.
func (s *Stream) TryReserve(n int64) *Reservation

type Reservation struct

// Value returns the number of bytes of stream- and connection-level flow control this Reservation holds.
// Writes to the stream will reduce this value.
func (r *Reservation) Value() int64

// Cancel returns the indicated number of bytes of stream- and connection-level flow control to the
// general pool. It panics if the math is wrong (n is negative, or n is larger than the current Value).
func (r *Reservation) Cancel(n int64)

@leafbebop
Copy link

While this might be off topic as the unreliable datagram extension is not part of quic rfc, I would like to express interest in the extension, for implementing http 3 datagrams and udp-connect methods.
I think that an API imitating net.PacketConn should be fine.

gopherbot pushed a commit to golang/net that referenced this issue May 25, 2023
The type of a QUIC packet can be identified by inspecting its first
byte, and the destination connection ID can be determined without
decrypting and parsing the entire packet.

For golang/go#58547

Change-Id: Ie298c0f6c0017343168a0974543e37ab7a569b0f
Reviewed-on: https://go-review.googlesource.com/c/net/+/475437
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matt Layher <mdlayher@gmail.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/net that referenced this issue May 25, 2023
Encrypt and decrypt QUIC packets according to RFC 9001.

For golang/go#58547

Change-Id: Ib7f824cf08f8520400bd38d3b3ab89e8a968114e
Reviewed-on: https://go-review.googlesource.com/c/net/+/475438
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/net that referenced this issue May 25, 2023
A rangeset is an ordered list of non-overlapping int64 ranges.
This type will be used for tracking which packet numbers need to be
acknowledged and which parts of a stream have been sent/received.

For golang/go#58547

Change-Id: Ia4ab3a47e82d0e7aea738a0f857b2129d4ea1f63
Reviewed-on: https://go-review.googlesource.com/c/net/+/478295
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/net that referenced this issue May 25, 2023
Functions to encode and decode QUIC variable-length integers
(RFC 9000, Section 16), as well as a few other common operations.

For golang/go#58547

Change-Id: I2a738e8798b8013a7b13d7c1e1385bf846c6c2cd
Reviewed-on: https://go-review.googlesource.com/c/net/+/478258
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/net that referenced this issue May 25, 2023
Constants for the transport error codes in RFC 9000 Section 20,
types representing transport errors sent to or received from the peer,
and a type representing application protocol errors.

For golang/go#58547

Change-Id: Ib4325e1272f6e0984f233ef494827a1799d7dc26
Reviewed-on: https://go-review.googlesource.com/c/net/+/495235
Reviewed-by: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
gopherbot pushed a commit to golang/net that referenced this issue May 25, 2023
When we send a packet, we need to remember its contents until it has
been acked or detected as lost.

For golang/go#58547

Change-Id: I8c18f7ca1730a3ce460cd562d060dd6c7cfa9ffb
Reviewed-on: https://go-review.googlesource.com/c/net/+/495236
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit to golang/net that referenced this issue May 25, 2023
Frame encoding is handled by the packetWriter type.
The packetWriter also takes responsibility for recording the contents
of constructed packets in a sentPacket structure.

Frame decoding is handled by consume*Frame functions, which generally
return the frame contents. ACK frames, which have complex contents,
are provided to the caller via callback function.

In addition to the above functions, used in the serving path, this
CL includes per-frame types that implement a common debugFrame
interface.  These types are used for tests and debug logging, but
not in the serving path where we want to avoid allocations from
storing values in an interface.

For golang/go#58547

Change-Id: I03ce11210aa9aa6ac749a5273b2ba9dd9c6989cf
Reviewed-on: https://go-review.googlesource.com/c/net/+/495355
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit to golang/net that referenced this issue May 25, 2023
Any given datum communicated to the peer follows a state machine:

  - We do not need to send the this datum.
  - We need to send it, but have not done so.
  - We have sent it, but the peer has not acknowledged it.
  - We have sent it and the peer has acknowledged it.

Data transitions between states in a consistent fashion; for example,
loss of the most recent packet containing a HANDSHAKE_DONE frame
means we should resend the frame in a new packet.

Add a sentVal type which tracks this state machine.

For golang/go#58547

Change-Id: I9de0ef5e482534b8733ef66363bac8f6c0fd3173
Reviewed-on: https://go-review.googlesource.com/c/net/+/498295
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/499284 mentions this issue: quic: parameterize rangeset

@gopherbot
Copy link

Change https://go.dev/cl/499285 mentions this issue: quic: add a data structure for tracking lists of sent packets

@gopherbot
Copy link

Change https://go.dev/cl/499283 mentions this issue: quic: add go1.21 build constraint

@gopherbot
Copy link

Change https://go.dev/cl/499286 mentions this issue: quic: add RTT estimator

@gopherbot
Copy link

Change https://go.dev/cl/499287 mentions this issue: quic: add packet pacer

@marten-seemann
Copy link
Contributor

I’d like to reach out once again, with a new proposal (see below). There have been several positive developments in quic-go in recent months:

  1. crypto/tls gained QUIC support, incl. a 0-RTT API. This means that starting with the next Go release, quic-go finally won’t have to fork crypto/tls anymore (work in progress PR). Special thanks to @FiloSottile and @neild for making it happen!
  2. quic-go just released v0.35.0, containing the biggest API change in years: Following a productive discussion and substantial backing from the quic-go community, we introduced the biggest API change in years: the introduction of a quic.Transport.
  3. We made further performance improvements, finally allowing the sending of significantly more than 1 Gbit/s per CPU core. This is mostly thanks to GSO, but also a large number of smaller changes, especially by cutting down on allocs. We’re now at the point where >90% of allocations happen in the UDP path in the standard library, and we could probably go a few 100 Mbit/s faster if we had the proposed new UDP API. More improvements to come in the near future!

Heres’s my proposal:

Have the standard library add HTTP/3 support, without exposing any QUIC API. As @ianswett pointed out, probably 90% of the use cases of QUIC in the standard library actually only need HTTP/3.

I believe this could easily be achieved by vendoring quic-go to an internal location in the standard-library, and using it from the net/http package. It might be possible to use bundle to create a quic-go bundle, similar to how the HTTP/2 implementation is bundled into the standard library.

The 10% of users that need raw QUIC (without HTTP/3) could then import quic-go directly. Importing a 3rd party package that implements additional functionality is a very common pattern in the Go ecosystem.

While quic-go does offer an http3 package, there’d be a big usability win if the standard library http.Server could speak HTTP/3 natively (enabled with a config flag?), instead of having to embed an existing http.Server into quic-go’s http3.Server (and similarly for the client side).

At frequent intervals (before a new Go release?) the Go team could decide if they want to include an updated quic-go version.

This approach would have a number of advantages:

  1. It would allow Go users to use a performance-optimized, battle-tested QUIC stack in their HTTP applications ~today.
  2. The quic-go community maintains the responsibility for the enhancement and maintenance of quic-go (including further performance improvements, which are totally on our roadmap!). Since no QUIC API is exposed, quic-go remains the ability to evolve its API.
  3. The fact that QUIC support is provided by quic-go becomes an implementation detail. It could be replaced with another QUIC implementation in the future without breaking Go’s backwards compatibility guarantee.

@kgersen
Copy link

kgersen commented May 31, 2023

similar to how the HTTP/2 implementation is bundled into the standard library

I'm not a fan of how HTTP/2 was bundled into the std lib. usinggo generate for the go bundle tool. It's not clean and not very future proof / extensible (and cause issues when debugging/testing http2 changes for instance because we have to regenerate/bundle and/or rebuild whole the std lib). That been said I'm aware I don't propose an alternative ;)

Also the way crypto/tls was specialized for net/http isn't clean either (see #46310 and #59734). It feels more like an emergency patch than something carefully designed.
The use of a callback Config.NegotiateALPN for instance would have been better than hard-coding the http1.1 fallback and now adding quic stuff directly into crypto/tls too. imho crypto/tls should remain agnostic of other protocols as much as possible.

gopherbot pushed a commit to golang/net that referenced this issue May 31, 2023
This package will add on crypto/tls features added in Go 1.21,
so use a build constraint to restrict ourselves to that version.

Unlocks the ability to use other features from Go versions more recent
than what's in x/net's go.mod file.

For golang/go#58547

Change-Id: I14011c7506b047e389d9b3e995c0bafcd5e74d44
Reviewed-on: https://go-review.googlesource.com/c/net/+/499283
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
@Jorropo
Copy link
Contributor

Jorropo commented May 31, 2023

@kgersen you bring up two issues, bundling and how HTTP2 ALPN negociation isn't very pretty, however thoses issues also applies to using a QUIC / HTTP3 implementation that lives in golang.org/x/net/internal/quic, this isn't a differentiation factor.
A differentiation factors I see are that quic-go and quic-go's HTTP3 implementation are well tested and used in the wild already (caddy for example).

@ianswett
Copy link

Thanks for the update Marten that's excellent progress and impressive throughput performance.

@ydnar
Copy link

ydnar commented May 31, 2023

Per @marten-seemann’s proposal:

  • The quic-go tests depend on test frameworks that we cannot depend on in the standard library, so those would need rewriting.

Raising my hand to help port the quic-go tests to standard library style.

@Jorropo
Copy link
Contributor

Jorropo commented May 31, 2023

The quic-go tests depend on test frameworks that we cannot depend on in the standard library, so those would need rewriting.

Raising my hand to help port the quic-go tests to standard library style.

❤️

Presumably if this is go internal vendoring or bundle route I don't see why quic-go's test suite would need to be runned inside the std, the http2 test suite is exclusively ran in golang.org/x/net/http2.
The http2 tests I see in the std are strictly new tests that validates the http2 integration inside net/http.

gopherbot pushed a commit to golang/net that referenced this issue May 31, 2023
Make the rangeset type parameterized, so it can be used for
either packet number or byte ranges without type conversions.

For golang/go#58547

Change-Id: I764913a33ba58222dcfd36f94de01c2249d73551
Reviewed-on: https://go-review.googlesource.com/c/net/+/499284
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/499640 mentions this issue: quic: add congestion controller

@gopherbot
Copy link

Change https://go.dev/cl/499641 mentions this issue: quic: loss detection

@thinkerou
Copy link

Per @marten-seemann’s proposal:

  • The quic-go tests depend on test frameworks that we cannot depend on in the standard library, so those would need rewriting.

Raising my hand to help port the quic-go tests to standard library style.

also need to remove third package?

@marten-seemann
Copy link
Contributor

Per @marten-seemann’s proposal:

  • The quic-go tests depend on test frameworks that we cannot depend on in the standard library, so those would need rewriting.

Raising my hand to help port the quic-go tests to standard library style.

also need to remove third package?

I assume you're referring to gojay? That's a 3rd party JSON encoder only used in the qlog package. It speeds up qlog encoding (by a lot), compared to the standard library, which is needed if you want to use qlog to debug QUIC connections at high throughputs.

For my proposal, quic-go could be vendored / bundled without qlog (quic-go doesn't depend on the qlog package, qlog is just an implementation of quic-go's tracer interface).

Excluding tests and qlog, this means that quic-go doesn't have any 3rd-party dependencies.

@mpx
Copy link
Contributor

mpx commented Jun 1, 2023

@marten-seemann wrote:

The 10% of users that need raw QUIC (without HTTP/3) could then import quic-go directly. Importing a 3rd party package that implements additional functionality is a very common pattern in the Go ecosystem.

This may be a reasonable transition plan to provide HTTP/3 sooner, but I think it's important to have a plan to land QUIC in the standard library as a peer transport option alongside TCP and UDP. If the code is trusted for HTTP/3, it should be available for other uses too. Using QUIC as a transport has a lot of advantages. In future, the idea of not shipping QUIC could be similar to not including TCP/UDP. With a solid implementation available many more applications are likely to be developed.

@marten-seemann
Copy link
Contributor

@marten-seemann wrote:

The 10% of users that need raw QUIC (without HTTP/3) could then import quic-go directly. Importing a 3rd party package that implements additional functionality is a very common pattern in the Go ecosystem.

This may be a reasonable transition plan to provide HTTP/3 sooner, but I think it's important to have a plan to land QUIC in the standard library as a peer transport option alongside TCP and UDP. If the code is trusted for HTTP/3, it should be available for other uses too. Using QUIC as a transport has a lot of advantages. In future, the idea of not shipping QUIC could be similar to not including TCP/UDP. With a solid implementation available many more applications are likely to be developed.

@mpx You're totally right, and as a member of the IETF QUIC working group from the very first day, I couldn't agree more. Can't wait for more and more internet traffic to shift to QUIC :)

My proposal is to decouple exposing of a HTTP/3 API (probably super easy, likely just a few bools in the net/http package) from exposing a QUIC API (which will be a huge API surface).
We can get the first by vendoring / bundling quic-go with the next 1.22 release, if desired. We can get the second independently of that by either wrapping the API of the vendored / bundled quic-go, or by molding the quic-go API into something that works for the standard library.

Neither requires writing a complete RFC 9000-compliant stack from scratch (which is a big task in itself), and the work required to make it an actually usable and performant stack (as I described in #58547 (comment)), which is an even bigger task, if you want to do it right.

gopherbot pushed a commit to golang/net that referenced this issue Jun 1, 2023
Store in-flight packets in a ring buffer.

For golang/go#58547

Change-Id: I825c4e600bb496c2f8f6c195085aaae3e847445e
Reviewed-on: https://go-review.googlesource.com/c/net/+/499285
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
@neild
Copy link
Contributor Author

neild commented Jun 1, 2023

I'm not the person who makes the decision here, but it's extremely unlikely that we would vendor a large, complex, third-party dependency into the standard library.

The standard library consists of code maintained by the Go team. We have common processes for code review, issue tracking, release cycles, and so forth. Adding a dependency which doesn't use those processes would be a very large change with a great deal of impact. As one example, coordinating security fixes for a third-party dependency would be tremendously complicated, and would need an entirely new and different process from what we use today.

There are some examples of code in the standard library that originate with third-party sources (e.g., crypto/internal/nistec/fiat), but these are small and change rarely.

Users can already import quic-go directly and use its HTTP/3 support. Perhaps there's more that can be done to make quic-go's HTTP/3 support integrate transparently with net/http, but that doesn't require bringing it into the standard library itself. Perhaps we should trust to the existing third-party implementation and not have HTTP/3 and QUIC in the standard library at all, or perhaps we should reconsider using quic-go as the basis for a standard library package, but adding it as a dependency is a lot of cost for a dubious benefit.

Regarding the situation with HTTP/2 support in x/net/http2, this is actually a tremendous pain to support and a strong argument against attempting to do the same with HTTP/3, or at least an argument that if we do we need to figure out how to avoid the pain points of the approach used for HTTP/2. To the contrary, I hope to move the HTTP/2 implementation out of x/net/http2 and into std at some point in the not too distant future.

As for providing a QUIC API vs. HTTP/3: It's absolutely the case that almost all uses for QUIC will be via HTTP/3. Perhaps the endpoint of this issue will be that we should have a QUIC implementation, but initially only expose it via HTTP/3 to avoid prematurely committing to a publicly-facing API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests