Subject: [quicwg/base-drafts] nonsensical MUST on frames fitting in packets (#2603)

From: Martin Thomson <[email-masked]>
Date: Wed, 10 Apr 2019 02:14:44 +0000 (UTC)

These were phrased as requirements, but they are really just statements
of fact.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * nonsensical MUST on frames fitting in packets

-- File Changes --

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

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2603.patch
https://github.com/quicwg/base-drafts/pull/2603.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/2603

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 09 Apr 2019 21:57:15 -0700

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/2603#pullrequestreview-224759329

---

From: MikkelFJ <[email-masked]>
Date: Tue, 09 Apr 2019 22:36:43 -0700

I'm not sure that I agree. You could easily have a long stream frame spilling over into the next packet. There is nothing but implied complications that really prevents that from happing, except from this text. That said, but old and new version will work.

-- 
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/2603#issuecomment-481540543

---

From: ianswett <[email-masked]>
Date: Wed, 10 Apr 2019 06:28:01 -0700

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/2603#pullrequestreview-224968862

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 10 Apr 2019 13:01:17 -0700

I've answered this very question, including recently, to people who were trying to understand QUIC framing. The text is definitely not nonsensical, and unless you want people experimenting with making frames go across packet boundaries (yes, you _can_ do this and use packet numbers as a way to reassemble pieces of a frame), I'd keep the MUST. I don't see a reason to drop the MUST.

This isn't editorial. The MUST is not implied anywhere.

-- 
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/2603#issuecomment-481840579

---

From: Lucas Pardue <[email-masked]>
Date: Wed, 10 Apr 2019 13:06:14 -0700

I'm in the other camp. The spec doesn't describe means for frames to span packets, so how does a receiving peer detect that this has occurred, and how would it react if it did?

-- 
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/2603#issuecomment-481842166

---

From: ianswett <[email-masked]>
Date: Wed, 10 Apr 2019 13:06:48 -0700

I'm still not understanding how you get a frame to span packet boundaries.  I mean, you could do that, but the first one would cause the connection to close and if a subsequent one arrived first who knows what it would be interpreted as.

-- 
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/2603#issuecomment-481842341

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 10 Apr 2019 20:31:09 +0000 (UTC)

@ianswett : The first one would cause the connection to close because the receiver is enforcing a MUST.  It's not clear to me why requiring the same thing of the sender is a problem.

-- 
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/2603#issuecomment-481850258

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 10 Apr 2019 20:35:50 +0000 (UTC)

@LPardue: We require the receiver to close the connection when it doesn't understand how to handle a frame that is split across packets, why not require the sender to not do this thing that will cause the receiver to close the connection.

I could be in the rough here, but I don't understand why we need to drop this requirement. I can live with either text, but I'd prefer the requirement.

-- 
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/2603#issuecomment-481851793

---

From: MikkelFJ <[email-masked]>
Date: Wed, 10 Apr 2019 14:15:18 -0700

> I'm in the other camp. The spec doesn't describe means for frames to span packets, so how does a receiving peer detect that this has occurred, and how would it react if it did?

The fact that this is difficult does not prevent the sender from doing it. The receiver could detect that a stream frame was shorter that advertised and assume the next higher packet number must have the rest, or some of the rest. Of course, this is an incorrect assumption, but if the receivers implementation is the same as the senders, this could work for a while, until it breaks.

-- 
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/2603#issuecomment-481868159

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 10 Apr 2019 21:27:10 +0000 (UTC)

I am favor of adopting the PR. Regarding @janaiyengar's concern, maybe we can rephrase the paragragh right before from:

_The payload of QUIC packets, after removing packet protection, commonly consists of a sequence of frames, as shown in Figure 7. Version Negotiation, Stateless Reset, and Retry packets do not contain frames._

to :

_The Payload of QUIC packets, after removing packet protection, consists of a sequence of complete frames, as shown in Figure 7. Version Negotiation, Stateless Reset, and Retry packets do carry the Payload._

Note that the three exceptional packets (VN, Stateless Reset, Retry) do not carry "Payload" by their definition.

-- 
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/2603#issuecomment-481871735

---

From: Martin Thomson <[email-masked]>
Date: Wed, 10 Apr 2019 18:18:04 -0700

@martinthomson pushed 1 commit.

4a2b70059b48cc6cca75b7c5fef096c8a17b1e06  Taking kazuho's suggestion, with tweaks

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2603/files/b604fa33db34417f087c6a8d0b53deb5e818e8a6..4a2b70059b48cc6cca75b7c5fef096c8a17b1e06

---

From: Martin Thomson <[email-masked]>
Date: Wed, 10 Apr 2019 18:20:50 -0700

@janaiyengar 
> The MUST is not implied anywhere.

It's implied everywhere.  It's implied by the fact that UDP is lossy.  It is implied in that there are rules for handling of malformed frames and mandates to generate a connection error in that case.

You would prefer that it be explicit, I get that, but this is better described as an axiom.

-- 
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/2603#issuecomment-481929224

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 10 Apr 2019 18:34:36 -0700

janaiyengar approved this pull request.

@kazuho: This helps -- it changes the definition of a QUIC payload to only have partial frames, which means the question of partial frames doesn't arise.

> @@ -2678,12 +2678,9 @@ Packet number encoding at a sender and decoding at a receiver are described in
 
 ## Frames and Frame Types {#frames}
 
-The payload of QUIC packets, after removing packet protection, commonly consists
-of a sequence of frames, as shown in {{packet-frames}}.  Version Negotiation,
-Stateless Reset, and Retry packets do not contain frames.
-
-<!-- TODO: Editorial work needed in this section. Not all packets contain
-frames. -->
+The payload of QUIC packets, after removing packet protection, commonly consist

```suggestion
The payload of a QUIC packet, after removing packet protection, commonly consists
```

or "payload of QUIC packets, ... consists"

> @@ -2678,12 +2678,9 @@ Packet number encoding at a sender and decoding at a receiver are described in
 
 ## Frames and Frame Types {#frames}
 
-The payload of QUIC packets, after removing packet protection, commonly consists
-of a sequence of frames, as shown in {{packet-frames}}.  Version Negotiation,
-Stateless Reset, and Retry packets do not contain frames.
-
-<!-- TODO: Editorial work needed in this section. Not all packets contain
-frames. -->
+The payload of QUIC packets, after removing packet protection, commonly consist

... maybe remove "commonly" as well, since the exceptions are listed next anyway?

-- 
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/2603#pullrequestreview-225291221

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 10 Apr 2019 18:34:39 -0700

-- 
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/2603#pullrequestreview-225293637

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 10 Apr 2019 18:40:52 -0700

> You would prefer that it be explicit, I get that, but this is better described as an axiom.

I think that's where the disconnect was. @kazuho's definitional change made it clear to me that you were all seeing it as axiomatic, where I (and perhaps @mikkelfj as well) was seeing it as a choice that needed enforcement.  And making the definitional change makes the axiom explicit.

-- 
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/2603#issuecomment-481932555

---

From: Marten Seemann <[email-masked]>
Date: Wed, 10 Apr 2019 18:44:13 -0700

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/2603#pullrequestreview-225295329

---

From: Martin Thomson <[email-masked]>
Date: Wed, 10 Apr 2019 18:46:42 -0700

01a87bcd67393a5bfb282cff4cbca0e39e30e5e2  Tweak

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2603/files/4a2b70059b48cc6cca75b7c5fef096c8a17b1e06..01a87bcd67393a5bfb282cff4cbca0e39e30e5e2

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 10 Apr 2019 19:05:16 -0700

Merged #2603 into master.

-- 
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/2603#event-2267867446