Rework packetization section #1053

martinthomson · 2018-01-16T01:54:51Z

In line with our established principles for retransmission, I've reworked the
description of packetization. The description now concentrates on the
information that is being repaired in response to perceived loss. This should
help avoid the confusion about retransmission.

Two new subsections are added to the packetization section. One covers the
processing of packets and includes the existing text on processing requirements
before acknowledgment. The other includes the retransmission logic.

The retransmission section still mentions frame types, but I've tried to make
that secondary to the description of the information that is being repaired.

I also removed mention of "Regular QUIC packets", which only occurred in 3
places.

Closes #463, #765.

martinduke

This is a massive improvement on the current text.

martinduke · 2018-02-12T23:45:00Z

draft-ietf-quic-transport.md

-  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.

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.

ianswett

Comments below

ianswett · 2018-02-13T03:06:48Z

draft-ietf-quic-transport.md

+
+## 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?

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

ianswett · 2018-02-13T03:08:13Z

draft-ietf-quic-transport.md

+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?

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

ianswett · 2018-02-13T03:09:14Z

draft-ietf-quic-transport.md

+
+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


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

ianswett · 2018-02-13T03:09:32Z

draft-ietf-quic-transport.md

+  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


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

ianswett · 2018-02-13T03:10:29Z

draft-ietf-quic-transport.md

+  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

ianswett · 2018-02-13T03:11:50Z

draft-ietf-quic-transport.md

+  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.

MikeBishop

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

MikeBishop · 2018-02-13T05:07:29Z

draft-ietf-quic-transport.md

-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?

MikeBishop · 2018-02-13T05:09:54Z

draft-ietf-quic-transport.md

+
+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


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

MikeBishop · 2018-02-13T05:10:57Z

draft-ietf-quic-transport.md

+  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


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

MikeBishop · 2018-02-13T05:11:49Z

draft-ietf-quic-transport.md

+  {{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"

MikeBishop · 2018-02-13T05:14:23Z

draft-ietf-quic-transport.md

-* 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"

MikeBishop · 2018-02-13T05:15:53Z

draft-ietf-quic-transport.md

+* 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.


PING frames don't have a payload any more.

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

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

In line with our established principles for retransmission, I've reworked the description of packetization. The description now concentrates on the information that is being repaired in response to perceived loss. This should help avoid the confusion about retransmission. Two new subsections are added to the packetization section. One covers the processing of packets and includes the existing text on processing requirements before acknowledgment. The other includes the retransmission logic. The retransmission section still mentions frame types, but I've tried to make that secondary to the description of the information that is being repaired. I also removed mention of "Regular QUIC packets", which only occurred in 3 places. Closes #463, #765.

martinthomson

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.

martinthomson · 2018-02-13T05:28:59Z

draft-ietf-quic-transport.md

+
+## Retransmission of Information
+
+QUIC packets that are determined to be lost are never retransmitted whole.  The


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

martinthomson · 2018-02-13T05:30:12Z

draft-ietf-quic-transport.md

+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 think that there is value in having a complete list here.

martinthomson · 2018-02-13T05:47:13Z

draft-ietf-quic-transport.md

+* 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.


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

martinthomson · 2018-02-13T05:54:49Z

draft-ietf-quic-transport.md

-  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.


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.

mikkelfj · 2018-02-13T08:18:49Z

draft-ietf-quic-transport.md

+  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

MikeBishop

Yep, looks good. Two nits.

MikeBishop · 2018-02-13T15:16:15Z

draft-ietf-quic-transport.md

+* 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 frame for each stream type needs this behavior.

MikeBishop · 2018-02-13T15:17:07Z

draft-ietf-quic-transport.md

+
+* 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?

ianswett

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

martinduke

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.

martinduke · 2018-02-13T19:25:24Z

draft-ietf-quic-transport.md

@@ -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.

… context

martinthomson · 2018-02-13T22:45:34Z

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

mikkelfj · 2018-02-14T08:51:34Z

draft-ietf-quic-transport.md

+  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.
EDIT: disregard, I misread this.

mikkelfj · 2018-02-14T08:53:08Z

draft-ietf-quic-transport.md

+* 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.

martinthomson added the -transport label Jan 16, 2018

martinduke reviewed Feb 12, 2018

View reviewed changes

ianswett reviewed Feb 13, 2018

View reviewed changes

MikeBishop approved these changes Feb 13, 2018

View reviewed changes

martinthomson added 2 commits February 13, 2018 16:51

Tighten language around MAX_XXX

6ac00a4

martinthomson force-pushed the packets-and-frames-v2 branch from 327dd47 to 6ac00a4 Compare February 13, 2018 05:52

martinthomson commented Feb 13, 2018

View reviewed changes

mikkelfj reviewed Feb 13, 2018

View reviewed changes

MikeBishop reviewed Feb 13, 2018

View reviewed changes

ianswett approved these changes Feb 13, 2018

View reviewed changes

martinduke reviewed Feb 13, 2018

View reviewed changes

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

e6beac4

… context

MikeBishop added the merge-imminent label Feb 14, 2018

martinthomson merged commit f229404 into master Feb 14, 2018

martinthomson removed the merge-imminent label Feb 14, 2018

mikkelfj reviewed Feb 14, 2018

View reviewed changes

mikkelfj mentioned this pull request Feb 18, 2018

Clarify how frames are retransmitted #1095

Closed

martinthomson deleted the packets-and-frames-v2 branch May 29, 2018 02:52


		## Retransmission of Information

		QUIC packets that are determined to be lost are never retransmitted whole. The

Rework packetization section #1053

Rework packetization section #1053

Conversation

martinthomson commented Jan 16, 2018

martinduke left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianswett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeBishop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinthomson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeBishop left a comment

Choose a reason for hiding this comment

MikeBishop Feb 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianswett left a comment

Choose a reason for hiding this comment

martinduke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinthomson commented Feb 13, 2018

mikkelfj Feb 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinduke left a comment •

edited

Loading

MikeBishop Feb 13, 2018 •

edited

Loading

mikkelfj Feb 14, 2018 •

edited

Loading