Subject: [quicwg/base-drafts] Change max_packet_size to max_datagram_size (#3471)

From: Kazuho Oku <[email-masked]>
Date: Thu, 20 Feb 2020 06:12:22 -0800

At the moment, the max_packet_size Transport Parameter is a size limit of "a packet," and we seem to interpret that it means "a QUIC packet." I am not sure if that has been our intent.

IIUC, the reason we have this TP is to tell the peer the maximum size of a "UDP datagram" that the endpoint can handle. Not the maximum size of a "QUIC packet."

The problem with having this limit being defined at QUIC packet-level is that it allows the sender to send a coalesced packet that consists of multiple QUIC packets, each of them consuming max_packet_size bytes.

For example, when responding to a client-sent Initial, a server is allowed to send a coalesced packet containing one Initial, one Handshake, one 1-RTT packet, each of them in full size, and expect the client to not drop the coalesced packet, assuming that the network can handle jumbo packets<sup>1</sup>.

This means that a QUIC stack needs to be prepared of handling UDP datagrams at most `3 * max_packet_size` bytes.

I do not think we would want to do that. Therefore, the proposal is to change the definition of the TP from max_packet_size to max_datagram_size.

As an alternative, we can change it to max_ip_packet_size, but considering the BSD socket API, I think that max_datagram_size would be a more natural fit.

[1] Note that there is a performance incentive to configure QUIC endpoints to send jumbo packets.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471

---

From: ianswett <[email-masked]>
Date: Thu, 20 Feb 2020 06:27:08 -0800

This seems like a reasonable change to me.

I assume this requires changing some of the surrounding text, not just the TP name, which would make it a design change I believe?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589077604

---

From: Nick Banks <[email-masked]>
Date: Thu, 20 Feb 2020 06:58:30 -0800

I agree. This seems like a good change, though I had already interpreted this value as "max UDP payload size" in the msquic code. I went back and looked at the text, and it's not 100% clear whether it was supposed to be for a QUIC packet or UDP datagram. It does say 'packet", not "datagram", but that word has been used interchangeably in the past...
```
max_packet_size (0x0003):
The maximum packet size parameter is an integer value that limits the size
of packets that the endpoint is willing to receive. This indicates that packets
larger than this limit will be dropped. The default for this parameter is the
maximum permitted UDP payload of 65527. Values below 1200 are invalid.
This limit only applies to protected packets (Section 12.1).
```
You might argue that it was always supposed to be UDP datagram payload length and call this editorial to clarify that, but calling it design is probably best.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589092395

---

From: David Schinazi <[email-masked]>
Date: Thu, 20 Feb 2020 09:15:26 -0800

I agree that clarifying this would be helpful. However, I think the term "datagram" is already very overloaded (for example we have a [DATAGRAM frame](https://tools.ietf.org/html/draft-pauly-quic-datagram)), how about something very explicit such as `max_udp_payload_size`?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589206024

---

From: Junho Choi <[email-masked]>
Date: Thu, 20 Feb 2020 12:31:22 -0800

👍  I like this change too. My understanding was TP max_packet_size is max payload size the receiver can accept, and max_datagram_size is a congestion control parameter for the sender (need to be found by PMTU discovery). To avoid confusion it's better to have a consistent name.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589294196

---

From: martinduke <[email-masked]>
Date: Thu, 20 Feb 2020 14:10:13 -0800

I like the change, but would strongly encourage us to just make it editorial. This is a very clear case of "we know what we meant."

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589371615

---

From: ianswett <[email-masked]>
Date: Thu, 20 Feb 2020 14:11:11 -0800

@martinduke want to write a PR and that should clarify whether it's editorial or not?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589373414

---

From: Lucas Pardue <[email-masked]>
Date: Thu, 20 Feb 2020 14:25:03 -0800

To highlight David's point, the DATAGRAM draft has s TP named `max_datagram_frame_size` - these two in coexistence would be unfortunate.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589382228

---

From: Martin Thomson <[email-masked]>
Date: Thu, 20 Feb 2020 20:22:39 -0800

This was never intended to be a limit on datagram size.  That is, it was intended to be a limit on the length of ciphertext passed to the AEAD.

That said, this is probably the right approach.  The size of the buffers you need to catch a UDP packet are probably more relevant than the size of the AEAD inputs.

I do note, however, that the construction of the Retry packet is such that the inputs to the AEAD might be greater in length than the packet itself.  And Retry has to be sent without even looking at the client's limits.  Maybe we need to say that Retry needs to be no more than 1179 bytes in length (to allow for the largest possible connection ID to be added).  That shouldn't be a problem in practice, as there is strong motivation for the Retry token to be much shorter than that - to the point that an 1179-byte Retry probably will have a token that doesn't allow for any data in an Initial packet - but we should be clearer about our limits.

Please don't claim that this is an editorial change.  It is not.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589491305

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 20 Feb 2020 21:18:05 -0800

If we are to impose a limit on the maximum size of Retry packets, I think a value much smaller than 1179 should be chosen. This is because a Retry token that is as large as that would leave almost no space for payload in Initial packet sent in response to the Retry packet.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589502662

---

From: martinduke <[email-masked]>
Date: Fri, 21 Feb 2020 09:24:43 -0800

@martinthomson I always assumed this was the equivalent of the TCP MSS option, rather than something to do with AEAD. Even if we decide to do nothing else, we should probably add some more text about what this is communicating and how to set it. But now the last sentence about unprotected packets make sense, so thanks for the insight.

But it seems we're converging on a TCP MSS-like purpose instead.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589751586

---

From: ianswett <[email-masked]>
Date: Fri, 21 Feb 2020 09:28:21 -0800

FWIW, I had assumed it was UDP payload size, since I need to ensure receive buffers are large enough.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589752947

---

From: David Schinazi <[email-masked]>
Date: Fri, 21 Feb 2020 09:56:20 -0800

I really don't think we should place a limit on retry token size. This is very much a case of
> - Doctor! It hurts when I put my hand on the hot stove!
> - Don't do that, then!"

Also, I don't see
> - My AES-GCM-128 code cannot handle inputs that are 1221 bytes long!

as a real world problem, and we shouldn't engineer around this theoretical.

I would very much be in favor of making this `max_udp_payload_size` and making it apply to all packet types.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589763507

---

From: MikkelFJ <[email-masked]>
Date: Sat, 22 Feb 2020 02:35:05 -0800

Could we possibly use the term segment as in max_segment_size to refer to the UDP payload size including aggregated packets in one payload? Using such a term is transport media neutral and would work over non-UDP media as well.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-589941997

---

From: Martin Thomson <[email-masked]>
Date: Sat, 22 Feb 2020 15:23:47 -0800

As others have noted, this can be a UDP-specific value.  If we start to push this over different substrates (ATM anyone?) then a different transport parameter might be better.  Making this UDP-specific means that we can apply clean constraints on the value.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-590008232

---

From: MikkelFJ <[email-masked]>
Date: Sat, 22 Feb 2020 19:32:40 -0800

I'm OK with that.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3471#issuecomment-590022526