Subject: Re: [quicwg/base-drafts] Rework packetization section (#1053)

From: martinduke <[email-masked]>
Date: Mon, 12 Feb 2018 23:45:29 +0000 (UTC)

martinduke commented on this pull request.

This is a massiv improvement on the current text.

-* The most recent MAX_STREAM_DATA frame for a stream MUST be retransmitted until
-  the receive stream enters a "Size Known" state. Any previous unacknowledged
-  MAX_STREAM_DATA frame for the same stream SHOULD NOT be retransmitted since a
-  newer MAX_STREAM_DATA frame for a stream obviates the need for delivering
-  older ones. Similarly, the most recent MAX_DATA frame MUST be retransmitted;
-  previous unacknowledged ones SHOULD NOT be retransmitted.
+* The current maximum data offset is sent in MAX_DATA frames until acknowledged.

There is no offset in MAX_DATA, so I'm not sure what you mean here.

-* BLOCKED, STREAM_BLOCKED, and STREAM_ID_BLOCKED frames SHOULD be retransmitted
-  if the sender is still blocked on the same limit.  If the limit has been
-  increased since the frame was originally sent, the frame SHOULD NOT be
-  retransmitted.
+* The current maximum stream data offset is sent in MAX_STREAM_DATA frames until
+  acknowledged or the receive stream enters a "Size Known" state.  Like
+  MAX_DATA, endpoints need to limit the rate at which this frame is sent.

In this paragraph, and others to follow, do you mean to say that *every packet* contains this frame from when it's first sent till acknowledgment, or only that it's retransmitted when the carrying packet is declared lost? Whichever it is, I think this could stand to be clearer.

-- 
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/1053#pullrequestreview-95983070

---

From: ianswett <[email-masked]>
Date: Tue, 13 Feb 2018 03:12:41 +0000 (UTC)

ianswett commented on this pull request.

Comments below

> +
+Once the packet has been fully processed, a receiver acknowledges receipt by
+sending one or more ACK frames containing the packet number of the received
+packet.  To avoid creating an indefinite feedback loop, an endpoint MUST NOT
+send an ACK frame in response to a packet containing only ACK or PADDING frames,
+even if there are packet gaps which precede the received packet.  The endpoint
+MUST acknowledge packets containing only ACK or PADDING frames in the next ACK
+frame that it sends.
+
+Strategies and implications of the frequency of generating acknowledgments are
+discussed in more detail in {{QUIC-RECOVERY}}.
+
+
+## Retransmission of Information
+
+QUIC packets that are determined to be lost are never retransmitted whole.  The

Technically, there's the one exception around connection close?

> +MUST acknowledge packets containing only ACK or PADDING frames in the next ACK
+frame that it sends.
+
+Strategies and implications of the frequency of generating acknowledgments are
+discussed in more detail in {{QUIC-RECOVERY}}.
+
+
+## Retransmission of Information
+
+QUIC packets that are determined to be lost are never retransmitted whole.  The
+same applies to the frames that are contained within lost packets.  Instead, the
+different types of information that might be carried in frames is sent again as
+needed.  New frames and packets are used to carry information that is determined
+to have been lost.
+
+* Application data sent in STREAM frames is retransmitted in new STREAM frames

I would expect this text to exist elsewhere.  Does it need to be here as well?

> +sending one or more ACK frames containing the packet number of the received
+packet.  To avoid creating an indefinite feedback loop, an endpoint MUST NOT
+send an ACK frame in response to a packet containing only ACK or PADDING frames,
+even if there are packet gaps which precede the received packet.  The endpoint
+MUST acknowledge packets containing only ACK or PADDING frames in the next ACK
+frame that it sends.
+
+Strategies and implications of the frequency of generating acknowledgments are
+discussed in more detail in {{QUIC-RECOVERY}}.
+
+
+## Retransmission of Information
+
+QUIC packets that are determined to be lost are never retransmitted whole.  The
+same applies to the frames that are contained within lost packets.  Instead, the
+different types of information that might be carried in frames is sent again as

is => are?

> +discussed in more detail in {{QUIC-RECOVERY}}.
+
+
+## Retransmission of Information
+
+QUIC packets that are determined to be lost are never retransmitted whole.  The
+same applies to the frames that are contained within lost packets.  Instead, the
+different types of information that might be carried in frames is sent again as
+needed.  New frames and packets are used to carry information that is determined
+to have been lost.
+
+* Application data sent in STREAM frames is retransmitted in new STREAM frames
+  unless the endpoint has sent a RST_STREAM for that stream.  Once an endpoint
+  sends a RST_STREAM frame, no further STREAM frames are needed.
+
+* The most recent set of acknowledgments is sent in ACK frames.  An ACK frame

is => are

-* All other frames MUST be retransmitted.
+* The maximum stream ID is sent in MAX_STREAM_ID frames until acknowledged.
+  Like MAX_DATA and MAX_STREAM_DATA, endpoints might need to limit the rate at
+  which this frame is sent.
+
+* Blocked signals are carried in BLOCKED, STREAM_BLOCKED, and STREAM_ID_BLOCKED
+  frames until acknowledged, but only while the endpoint is blocked on the
+  corresponding limit.  These frames always include the limit that is causing
+  blocking at the time that they are transmitted.
+
+* An liveness or path validation check using PING frames is sent until

An => A or Any

> -* All other frames MUST be retransmitted.
+* The maximum stream ID is sent in MAX_STREAM_ID frames until acknowledged.
+  Like MAX_DATA and MAX_STREAM_DATA, endpoints might need to limit the rate at
+  which this frame is sent.
+
+* Blocked signals are carried in BLOCKED, STREAM_BLOCKED, and STREAM_ID_BLOCKED
+  frames until acknowledged, but only while the endpoint is blocked on the
+  corresponding limit.  These frames always include the limit that is causing
+  blocking at the time that they are transmitted.
+
+* An liveness or path validation check using PING frames is sent until
+  acknowledged or until there is no remaining need for liveness or path
+  validation checking.  PING frames with a payload SHOULD include a different
+  payload each time they are sent.
+
+* Responses to path validation using PONG frames are sent just once.  A new PING

PONG doesn't exist anymore.

-- 
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/1053#pullrequestreview-96014401

---

From: Mike Bishop <[email-masked]>
Date: Tue, 13 Feb 2018 05:17:04 +0000 (UTC)

MikeBishop approved this pull request.

Some subject-verb disagreement, but otherwise fine.  I think this is a net improvement.

> @@ -1894,10 +1894,10 @@ Note that Stateless Reset messages do not have any cryptographic protection.
 
 # Frame Types and Formats
 
-As described in {{frames}}, Regular packets contain one or more frames.
-We now describe the various QUIC frame types that can be present in a Regular
-packet. The use of these frames and various frame header bits are described in
-subsequent sections.
+As described in {{frames}}, packets contain one or more frames.  We now

Not new text, but basically none of the recent text is written in the first-person plural.  Can we rephrase this, while you're touching the paragraph?

"types are" or "information is", but not "types is"

+
+

Nope, he's right here.  "The ... set ... is sent"

+same applies to the frames that are contained within lost packets.  Instead, the
+different types of information that might be carried in frames is sent again as
+needed.  New frames and packets are used to carry information that is determined
+to have been lost.
+
+* Application data sent in STREAM frames is retransmitted in new STREAM frames
+  unless the endpoint has sent a RST_STREAM for that stream.  Once an endpoint
+  sends a RST_STREAM frame, no further STREAM frames are needed.
+
+* The most recent set of acknowledgments is sent in ACK frames.  An ACK frame
+  SHOULD contain all unacknowledged acknowledgments, as described in
+  {{sending-ack-frames}}.
+
+* Cancellation of stream transmission, as carried in a RST_STREAM frame, is sent
+  until acknowledged or until acknowledgments for all stream data is received

"until acknowledgements ... are received" or "until all stream data is acknowledged by the peer"

-* ACK and PADDING frames MUST NOT be retransmitted.  ACK frames
-  containing updated information will be sent as described in {{frame-ack}}.
+* Similarly, requests to cancel stream transmission, as encoded in a
+  STOP_SENDING frame, is sent until the receive stream enters either a "Data

"requests ... are sent"

PING frames don't have a payload any more.

-- 
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/1053#pullrequestreview-96026582

---

From: Mike Bishop <[email-masked]>
Date: Tue, 13 Feb 2018 05:18:23 +0000 (UTC)

MikeBishop commented on this pull request.

And related, the merge conflict is probably that this section doesn't mention PATH_CHALLENGE / PATH_RESPONSE frames.

-- 
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/1053#discussion_r167762943

---

From: Martin Thomson <[email-masked]>
Date: Mon, 12 Feb 2018 21:56:27 -0800

martinthomson commented on this pull request.

Thanks for picking up on my sloppy copy-edits.  Please take another look at the MAX_DATA and friends changes.  I think that they are much better, but you might disagree.

I don't like explaining exceptions every place, but that would leave "never", which is a bit absolute.

+frame that it sends.
+

I think that there is value in having a complete list here.

You will note that date at which this PR was created relative to when it is being reviewed.  I have rebased.

I have rephrased this completely.  I've changed this so that retransmission only occurs when the *most recently sent* frame is declared lost.  I think that is much neater.

As you say, it was unclear before, and a valid interpretation of the proposed text was "send a frame in every packet", which I agree is a little silly.

-- 
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/1053#pullrequestreview-96029086

---

From: MikkelFJ <[email-masked]>
Date: Tue, 13 Feb 2018 00:20:06 -0800

mikkelfj commented on this pull request.

> +* The current connection maximum data is sent in MAX_DATA frames. An updated
+  value is sent in a MAX_DATA frame if the packet containing the most recently
+  sent MAX_DATA frame is declared lost, or when the endpoint decides to update
+  the limit.  Care is necessary to avoid sending this frame too often as the
+  limit can increase frequently and cause an unnecessarily large number of
+  MAX_DATA frames to be sent.
+
+* The current maximum stream data offset is sent in MAX_STREAM_DATA frames.
+  Like MAX_DATA, an updated value is sent when the packet containing
+  the most recent MAX_STREAM_DATA frame for a stream is lost or when the limit
+  is updated, with care taken to prevent the frame from being sent too often. An
+  endpoint SHOULD stop sending MAX_STREAM_DATA frames when the receive stream
+  enters a "Size Known" state.
+
+* The maximum stream ID is sent in MAX_STREAM_ID frames.  Like MAX_DATA, an
+  updated value is sent when a packet containing the most recent MAX_STREAM_ID

The maximum stream ID for a stream type

-- 
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/1053#pullrequestreview-96053969

---

From: Mike Bishop <[email-masked]>
Date: Tue, 13 Feb 2018 07:17:46 -0800

Yep, looks good.  Two nits.

-* All other frames MUST be retransmitted.
+* The maximum stream ID is sent in MAX_STREAM_ID frames.  Like MAX_DATA, an
+  updated value is sent when a packet containing the most recent MAX_STREAM_ID
+  frame is declared lost or when the limit is updated, with care taken to
+  prevent the frame from being sent too often.

@mikkelfj raised a good point, but I don't see it associated with this paragraph:  The most recently sent from *for each stream type* needs this behavior.

-* All other frames MUST be retransmitted.
+* The maximum stream ID is sent in MAX_STREAM_ID frames.  Like MAX_DATA, an
+  updated value is sent when a packet containing the most recent MAX_STREAM_ID
+  frame is declared lost or when the limit is updated, with care taken to
+  prevent the frame from being sent too often.
+
+* Blocked signals are carried in BLOCKED, STREAM_BLOCKED, and STREAM_ID_BLOCKED
+  frames until acknowledged, but only while the endpoint is blocked on the
+  corresponding limit.  These frames always include the limit that is causing
+  blocking at the time that they are transmitted.
+
+* A liveness or path validation check using PATH_CHALLENGE frames is sent until
+  acknowledged or until there is no remaining need for liveness or path
+  validation checking.  PATH_CHALLENGE frames with a payload SHOULD include a

Are there PATH_CHALLENGE frames without a payload?

-- 
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/1053#pullrequestreview-96177438

---

From: ianswett <[email-masked]>
Date: Tue, 13 Feb 2018 15:24:36 +0000 (UTC)

ianswett approved this pull request.

Looks great, minus Mike's point about PATH_CHALLENGE and payload.

-- 
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/1053#pullrequestreview-96180774

---

From: martinduke <[email-masked]>
Date: Tue, 13 Feb 2018 19:44:24 +0000 (UTC)

Thanks for fixing MAX_STREAM_DATA, but I think we have the same ambiguity with RST_STREAM, STOP_SENDING, BLOCKED, STREAM_BLOCKED, STREAM_ID_BLOCKED, and NEW_CONNECTION_ID. Send in every packet until acked, or once per loss event?

Are ACKs the only information we send even though the  information is in flight? Perhaps some introductory text in "Retransmission of Information" can make the default behavior more explicit.

> @@ -2730,75 +2728,112 @@ transmission efficiency to underfilled packets.
 
 # Packetization and Reliability {#packetization}
 
-A sender bundles one or more frames in a Regular QUIC packet (see {{frames}}).
+A sender bundles one or more frames in a QUIC packet (see {{frames}}).

Sorry to be pedantic, but Version Negotiation and Stateless Reset packets don't have any frames.

-- 
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/1053#pullrequestreview-96268342

---

From: Martin Thomson <[email-masked]>
Date: Tue, 13 Feb 2018 14:44:43 -0800

@martinthomson pushed 1 commit.

e6beac4  Make *BLOCKED retransmission rules clearer, add a little more general context

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 13 Feb 2018 22:45:36 +0000 (UTC)

I've further clarified the scope of *BLOCKED frames and the rules for retransmitting those.  I plan to merge this pretty soon.

-- 
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/1053#issuecomment-365431250

---

From: Martin Thomson <[email-masked]>
Date: Wed, 14 Feb 2018 04:56:46 +0000 (UTC)

Merged #1053.

-- 
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/1053#event-1473073430

---

From: MikkelFJ <[email-masked]>
Date: Wed, 14 Feb 2018 08:51:38 +0000 (UTC)

> +  enters a "Size Known" state.
+
+* The maximum stream ID for a stream of a given type is sent in MAX_STREAM_ID
+  frames.  Like MAX_DATA, an updated value is sent when a packet containing the
+  most recent MAX_STREAM_ID for a stream type frame is declared lost or when
+  the limit is updated, with care taken to prevent the frame from being sent
+  too often.
+
+* Blocked signals are carried in BLOCKED, STREAM_BLOCKED, and STREAM_ID_BLOCKED
+  frames. BLOCKED streams have connection scope, STREAM_BLOCKED frames have
+  stream scope, and STREAM_ID_BLOCKED frames are scoped to a specific stream
+  type. New frames are sent if packets containing the most recent frame for a
+  scope is lost, but only while the endpoint is blocked on the corresponding
+  limit. These frames always include the limit that is causing blocking at the
+  time that they are transmitted.
+

It is no longer clear a BLOCKED frame should be sent again if the blocking limitation is removed or updated.

-- 
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/1053#pullrequestreview-96419634

---

From: MikkelFJ <[email-masked]>
Date: Wed, 14 Feb 2018 08:53:15 +0000 (UTC)

> +## Retransmission of Information
+
+QUIC packets that are determined to be lost are not retransmitted whole. The
+same applies to the frames that are contained within lost packets. Instead, the
+information that might be carried in frames is sent again in new frames as
+needed.
+
+New frames and packets are used to carry information that is determined to have
+been lost.  In general, information is sent again when a packet containing that
+information is determined to be lost and sending ceases when a packet
+containing that information is acknowledged.
+
+* Application data sent in STREAM frames is retransmitted in new STREAM frames
+  unless the endpoint has sent a RST_STREAM for that stream.  Once an endpoint
+  sends a RST_STREAM frame, no further STREAM frames are needed.
+

It is not obvious that the `endpoint` is the local endpoint, or `sender` unless you are quite familiar with stream state already.

-- 
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/1053#pullrequestreview-96420032