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

Maximum packet size #383

Closed
marten-seemann opened this issue Mar 10, 2017 · 16 comments
Closed

Maximum packet size #383

marten-seemann opened this issue Mar 10, 2017 · 16 comments
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.

Comments

@marten-seemann
Copy link
Contributor

In the error codes there's a QUIC_PACKET_TOO_LARGE error, but there's no maximum packet size specified anywhere in the document.

Does it make sense to define a maximum packet size or must a QUIC implementation be capable of handling packets with the maximum UDP packet size? Google QUIC currently uses 1452 as the maximum packet size (= Ethernet MTU - minimum UDP header size).

@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Mar 10, 2017
@ianswett
Copy link
Contributor

I would propose we remove this error code, unless there's some compelling reason for it to exist.

GQUIC doesn't even use it anymore in practice, though there is some possibility of packets that are too large being dropped in the current implementation.

@martinthomson
Copy link
Member

Dropping a packet that is too large isn't ideal, since that doesn't prevent more packets of that size arriving in future. Do we need a mechanism? Do we need to offer any guidance?

@martinthomson
Copy link
Member

A potential mechanism: TLS has a max_fragment_length extension. This allows endpoints to set a limit on record size, which is especially helpful when the endpoint is memory-constrained. Obviously, a TCP-based protocol offers more opportunity to have large packets, but big UDP packets aren't impossible.

@igorlord
Copy link
Contributor

1452 is certainly too small. QUIC should allow at least PMTU-sized UDP packets (unless negotiated lower). MTU of localhost in quite large. Jumbo packets exist.

@marten-seemann
Copy link
Contributor Author

I'd like to make the argument that if we don't define a maximum packet size here, implementations should be allowed to choose one and drop larger packets.

I have very little experience with PMTUD, but wouldn't consistently dropping these packets look like one element on the path doesn't suppport the higher MTU and cause the peer to reduce the packet size?

Setting a maximum packet size is essentially a DoS defense for an implementation. During the handshake, undecrytable packets have to be queued (because reordering can happen). In quic-go, we're queuing up to 10 packets before sending a Public Reset, which is a reasonably small memory commitment of about 15 kB. Things might look different if these were maximum size UDP packets though.

@Aron-Schats
Copy link

Related: pull request #384 proposes that handshake be completed within 64 KB of stream 1 data.

@ianswett
Copy link
Contributor

@martinthomson Dropping packets that are too large doesn't provide much feedback, but that's a failure mode QUIC, at least in Chrome, has to deal with gracefully. It's much more likely the path drops the packet before it arrives at the server, possibly sending an ICMP packet in the process.

For a while, GQUIC would read packets up to 2000 bytes, but send back an error if the packet was larger than 1460 or something. This path was almost never hit, but created unnecessary code complexity.

So I'd support no max packet size, with dropping being the way to deal with overly large ones, but possibly we want to require all endpoints support 1500 byte packets as a minimum?

@Aron-Schats
Copy link

...but possibly we want to require all endpoints support 1500 byte packets as a minimum?

@ianswett, why?

@ianswett
Copy link
Contributor

I was thinking that a peer would want to know what the max MTU it should try is if we're doing PathMTU, and that most connections would want to use a size a bit larger than 1280.

But possibly this is something that could be sent in the handshake if we thought it was useful, instead of having a mandated size?

@Aron-Schats
Copy link

Aron-Schats commented Mar 10, 2017

Why mandate a minimum size at all? I think it should be up to the server. If the server is willing to accept packets carrying just 10 bytes (for example) of stream data payload, so be it. Certainly it is not a good idea for a web server on the Internet as it would take hundreds of packets to complete a handshake, but who knows how someone would want to use QUIC?

@janaiyengar
Copy link
Contributor

@Aron-Schats: the minimum size is required to avoid amplification attacks (see Packetization and Reliability section in the transport draft.)

I think removing the error code is fine. Anything that an endpoint sends as an untested packet size mid-connection will naturally be a part of PMTUD, in which case, the endpoint will also be prepared to deal with it getting dropped. I would expect detecting server support of an untested packet size to be part of PMTUD.

@martinthomson
Copy link
Member

I agree with the view that this looks a lot like PMTUD, providing that the endpoint is consistent in how it drops larger packets.

Is there any value in signaling a maximum if you are operating a constrained implementation?

@igorlord
Copy link
Contributor

If you are a constrained implementation or you know that you are behind a path with PMTU smaller than usual, it is better to let the peer know rather than have the peer waste a round trip (in the best case) to discover this for itself.

TCP negotiates MSS.

@ianswett
Copy link
Contributor

It may be nice to signal MSS, but this could only be the endpoint value, unless one of the peers had particularly good knowledge of the path. So I'd suggest a handshake negotiated value, to the min of the two peers, since it's assume the max endpoint size is the same for sending and receiving.

@martinthomson
Copy link
Member

Relevant to this discussion: https://datatracker.ietf.org/doc/html/draft-thomson-tls-record-limit - the same reasons for doing that apply here. Interestingly, this would allow an endpoint to specify a limit lower than 1280 for encrypted packets, but I think that's OK. (The degree to which we intend to support constrained implementations determines how relevant this feature might be.)

@mnot mnot added this to Packets in QUIC Apr 28, 2017
martinthomson added a commit that referenced this issue May 4, 2017
This is primarily for constrained implementations.  Based on experience with
TLS, there can be real burden for those implementations just implementing an
AEAD correctly over large records.  This setting would allow an absurdly
constrained device some defense against peers who might otherwise send jumbo
packets.  After all, jumbo packets can be more efficient.

Closes #383.
martinthomson added a commit that referenced this issue Jun 6, 2017
This is primarily for constrained implementations.  Based on experience with
TLS, there can be real burden for those implementations just implementing an
AEAD correctly over large records.  This setting would allow an absurdly
constrained device some defense against peers who might otherwise send jumbo
packets.  After all, jumbo packets can be more efficient.

Closes #383.
@mnot
Copy link
Member

mnot commented Jun 6, 2017

Discussed in Paris; Martin to update PR above:

  • raise minimum to same size as our MTU

@mnot mnot changed the title Do we need to define a maximum packet size? Maximum packet size Jun 21, 2017
@mnot mnot removed this from Packets in QUIC Jun 21, 2017
@mnot mnot added the has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. label Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
None yet
Development

No branches or pull requests

7 participants