Subject: [quicwg/base-drafts] renamed max_packet_size to max_udp_size (#3473)

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

Fixes #3471 with the aim of being editorial. Assuming one used the only sensible interpretation of the parameter definition, there shouldn&#39;t be an code change except renaming the parameter in logs and stats.

Also took @DavidSchinazi&#39;s recommendation to avoid the overloaded word &quot;datagram.&quot;
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/3473

-- Commit Summary --

  * renamed max_packet_size to max_udp_size

-- File Changes --

    M draft-ietf-quic-transport.md (17)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/3473.patch
https://github.com/quicwg/base-drafts/pull/3473.diff

-- 
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/pull/3473

---

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

@martinduke pushed 1 commit.

b50ed262f07380cdb9ccc6d479fdd473031b0389  fixed typo

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3473/files/5989bb0affa6d676c0376895979f8f0b78567676..b50ed262f07380cdb9ccc6d479fdd473031b0389

---

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

ianswett approved this pull request.

-- 
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/pull/3473#pullrequestreview-362280502

---

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

6b9a39e1d28db32f2cb5393cde08a1df28311b15  synced to latest, resolved conflict

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3473/files/b50ed262f07380cdb9ccc6d479fdd473031b0389..6b9a39e1d28db32f2cb5393cde08a1df28311b15

---

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

985a57a2a8c761ba5cb20158df8975971a8717cd  erase merge artifact

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3473/files/6b9a39e1d28db32f2cb5393cde08a1df28311b15..985a57a2a8c761ba5cb20158df8975971a8717cd

---

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

ef507076480a2b94374d7f6f7b4b5b910944c012  delete whitespace

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3473/files/985a57a2a8c761ba5cb20158df8975971a8717cd..ef507076480a2b94374d7f6f7b4b5b910944c012

---

From: martinduke <[email-masked]>
Date: Thu, 20 Feb 2020 15:01:11 -0800

Well that was a very annoying merge conflict. Sorry for the noise.

-- 
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/pull/3473#issuecomment-589396913

---

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

nibanks approved this pull request.

-- 
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/pull/3473#pullrequestreview-362299551

---

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

DavidSchinazi commented on this pull request.

-: 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 ({{packet-protected}}).
+: The maximum udp size parameter is an integer value that limits the size of
+  UDP payloads that the endpoint is willing to receive.  This indicates that
+  packets larger than this limit are likely not to be processed by the receiver.

I would change this sentence to something like `This indicates that UDP packets whose payload is larger than this limit are likely not to be processed by the receiver.`

-: 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 ({{packet-protected}}).
+: The maximum udp size parameter is an integer value that limits the size of
+  UDP payloads that the endpoint is willing to receive.  This indicates that
+  packets larger than this limit are likely not to be processed by the receiver.
+  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

What was the rationale for this text: `This limit only applies to protected packets ({{packet-protected}}).` ? Given the new clarification, I think we should just remove this sentence.

> @@ -4584,13 +4584,14 @@ stateless_reset_token (0x0002):
   reset ({{stateless-reset}}) for the connection ID negotiated during the
   handshake.
 
-max_packet_size (0x0003):
+max_udp_size (0x0003):

I think `max_udp_payload_size` is clearer than `max_udp_size` but this is still better than `max_packet_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/pull/3473#pullrequestreview-362299331

---

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

martinduke commented on this pull request.

agreed.

-- 
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/pull/3473#discussion_r382322415

---

From: martinduke <[email-masked]>
Date: Thu, 20 Feb 2020 15:45:01 -0800

I like short names but if this in the consensus I'll change it.

-- 
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/pull/3473#discussion_r382322665

---

From: martinduke <[email-masked]>
Date: Thu, 20 Feb 2020 15:45:45 -0800

I have no idea. Maybe @martinthomson or @janaiyengar knows?

I'm happy to remove it, if it's a mistake.

-- 
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/pull/3473#discussion_r382322877

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 20 Feb 2020 15:49:52 -0800

kazuho commented on this pull request.

I think we need to talk about both UDP payload size and QUIC packet size, because they are different things.

I would assume that we'd all agree that an endpoint is allowed to process UDP datagrams that are larger than the advertised maximum. The endpoint cannot consider that as a protocol violation, because UDP datagrams are not authenticated.

Contrary to that, certain QUIC packets are "protected," and an endpoint _can_ report a protocol violation if it receives a protected QUIC packet that can be decrypted, but is larger than the advertised maximum. If such check needs to be done (or should not be done) is under debate in #3447.

The new text says that "packets larger than this limit are likely not to be processed."

I'd interpret this as "a receiver MAY process packets larger than this limit", and therefore would consider that this PR at the moment is trying to also close #3447, by implying that "a receiver MUST NOT close the connection when it receives a QUIC packet that is greater than the advertised limit."

-- 
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/pull/3473#discussion_r382324193

---

From: David Schinazi <[email-masked]>
Date: Thu, 20 Feb 2020 16:16:41 -0800

Are we really concerned about nodes performing this check? My understanding is that we added this transport parameter in the first place to allow folks to read UDP into a fixed-size stack buffer, and this allows an endpoint to communicate the size of that buffer to its peer. If I'm able to decrypt this packet it means I'm able to read and process it, so I shouldn't be sending a lower transport parameter. I think the sentence I propose above makes it clear that this is advisory and should not be enforced.

-- 
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/pull/3473#discussion_r382332227

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 20 Feb 2020 16:25:17 -0800

@DavidSchinazi I totally agree with your rationale. However, #3447 discusses about enforcing the maximum at the QUIC packet level.

Anyways, regarding enforcement, I think what we need to build consensus, before discussing how it should be baked into the specification. And that might be out-of-scope for a PR that aims to be editorial. Based on that view, I think it is fine to just talk about UDP payload size here, and that having only the text you proposed is fine.

-- 
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/pull/3473#discussion_r382334497

---

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

martinthomson requested changes on this pull request.

See the issue for a discussion of how this limit might lead to the need for additional text regarding Retry packets.

```suggestion
: The maximum UDP size parameter is an integer value that limits the size of
```

See the issue for rationale.  However, with this change it can be removed.  It does create the issue I mentioned about Retry though.

-- 
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/pull/3473#pullrequestreview-362392179

---

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

OK, I'm going to take 3 thumbs up as "consensus" for this cosmetic issue, and will fix it in the next commit.

-- 
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/pull/3473#discussion_r382701891

---

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

I've applied @DavidSchinazi's original rewording suggestion but am leaving this conversation open as the subject has drifted off.

-- 
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/pull/3473#discussion_r382734956

---

From: martinduke <[email-masked]>
Date: Fri, 21 Feb 2020 10:26:17 -0800

bbf7a7298a63b48c78e30fad7cdf2bd2bb4d8db0  Resolved multiple review comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3473/files/ef507076480a2b94374d7f6f7b4b5b910944c012..bbf7a7298a63b48c78e30fad7cdf2bd2bb4d8db0

---

From: David Schinazi <[email-masked]>
Date: Fri, 21 Feb 2020 10:39:54 -0800

DavidSchinazi approved this pull request.

Thanks for making these edits

-- 
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/pull/3473#pullrequestreview-362835225

---

From: Marten Seemann <[email-masked]>
Date: Fri, 21 Feb 2020 20:40:07 -0800

marten-seemann approved this pull request.

-- 
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/pull/3473#pullrequestreview-363009505

---

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

I realize QUIC is fairly tied to UDP, notably in all transport text. But if you want to modify QUIC to run over a separate channel like over a bluetooth bus, would you then invent a new transport parameter for that, or would packet size be the right choice? If so, renaming to UDP might have some consequences.

-- 
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/pull/3473#issuecomment-589941553

---

From: Marten Seemann <[email-masked]>
Date: Sat, 22 Feb 2020 02:38:54 -0800

@mikkelfj Note that the name of the document is "QUIC: A UDP-Based Multiplexed and Secure Transport".

-- 
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/pull/3473#issuecomment-589942307

---

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

martinthomson approved this pull request.

Yep, this is better.

> @@ -4549,7 +4549,6 @@ values of N are reserved to exercise the requirement that unknown transport
 parameters be ignored.  These transport parameters have no semantics, and may
 carry arbitrary values.

```suggestion

```

-- 
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/pull/3473#pullrequestreview-363056173

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 22 Feb 2020 18:50:44 -0800

kazuho approved this pull request.

-- 
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/pull/3473#pullrequestreview-363062381

---

From: martinduke <[email-masked]>
Date: Sat, 22 Feb 2020 21:10:16 -0800

2fa2b972b25fa08cff394ac32ec07aaf0bc057e0  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3473/files/bbf7a7298a63b48c78e30fad7cdf2bd2bb4d8db0..2fa2b972b25fa08cff394ac32ec07aaf0bc057e0

---

From: martinduke <[email-masked]>
Date: Mon, 24 Feb 2020 10:32:49 -0800

@martinduke pushed 2 commits.

64710472cca748614f36ece3ad206416120e8e27  MT's suggestion and resolve conflict
de3edd54868f36ce17fb2a2700573c72deb9ba5a  Merge branch 'max_datagram' of https://github.com/martinduke/base-drafts into max_datagram

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3473/files/2fa2b972b25fa08cff394ac32ec07aaf0bc057e0..de3edd54868f36ce17fb2a2700573c72deb9ba5a

---

From: martinduke <[email-masked]>
Date: Mon, 24 Feb 2020 11:00:26 -0800

Editors, I believe the latest commit covers all open concerns about the PR.

-- 
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/pull/3473#issuecomment-590494267

---

From: David Schinazi <[email-masked]>
Date: Mon, 24 Feb 2020 11:22:33 -0800

-- 
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/pull/3473#pullrequestreview-363638110

---

From: ianswett <[email-masked]>
Date: Mon, 24 Feb 2020 11:24:57 -0800

-- 
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/pull/3473#pullrequestreview-363639814

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 27 Feb 2020 19:47:45 -0800

janaiyengar approved this pull request.

Looks good, thanks @martinduke !

-- 
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/pull/3473#pullrequestreview-366146382