Subject: Re: [quicwg/base-drafts] What packet information to track and send (#1939)

From: martinduke <[email-masked]>
Date: Thu, 01 Nov 2018 19:58:02 -0700

martinduke approved this pull request.

I believe this PR addresses  #765, though maybe not #1724 

I have  a few nits, but otherwise I think this ready to merge.

> +
+Sent packets are tracked for each packet number space, and ACK
+processing only applies to a single space.
+
+### Sent Packet Fields {#sent-packets-fields}
+
+packet_number
+: The packet number of the sent packet.
+
+retransmittable:
+: A boolean that indicates whether a packet is retransmittable.
+  If true, it is expected that an acknowledgement will be received,
+  though the peer could delay sending the ACK frame containing it
+  by up to the MaxAckDelay.
+
+in_flight:

Not a big deal, but I found it easier to just store "flightsize" which is set to zero if it doesn't count, instead of having in_flight and send_bytes. If you find that more compact and understandable, feel free to change it.

> +
+QUIC stores information about every packet sent. It's expected implementations
+will index this per-packet information by packet number and store the
+per-packet fields detailed below for loss recovery and congestion control.
+Additionally, implementations MUST ensure that any retransmittable frames
+being transmitted are tracked in case of loss.
+
+If a packet containing retransmittable frames is lost, the QUIC transport
+needs to recover from that loss, such as by retransmitting the data,
+sending an updated frame, or abandoning the frame.  For more information,
+see Section 13.2 of {{QUIC-TRANSPORT}}.
+
+Packets MUST be tracked until acknowledged or lost.  After a packet is lost,
+it SHOULD be tracked for an amount of time comparable to the maximum
+expected packet reordering, such as 1 RTT.  This allows detection of
+spurious retransmissions and MAY allow avoiding an extra retransmissions if

s/allow avoiding an/avoid

> @@ -509,6 +509,57 @@ frames containing them could be lost. In this case, the loss recovery algorithm
 may cause spurious retransmits, but the sender will continue making forward
 progress.
 
+## Tracking Sent Packets {#tracking-sent-packets}
+
+QUIC stores information about every packet sent. It's expected implementations
+will index this per-packet information by packet number and store the
+per-packet fields detailed below for loss recovery and congestion control.
+Additionally, implementations MUST ensure that any retransmittable frames
+being transmitted are tracked in case of loss.
+

"Implementations MUST track the ack blocks in a packet in order to stop sending those ack blocks if the packet is 
acknowledged."

Perhaps this is not strictly loss-recovery-related, but this is the section where I'm going to go to implement struct sent_pkt.

> @@ -509,6 +509,57 @@ frames containing them could be lost. In this case, the loss recovery algorithm
 may cause spurious retransmits, but the sender will continue making forward
 progress.
 
+## Tracking Sent Packets {#tracking-sent-packets}
+
+QUIC stores information about every packet sent. It's expected implementations
+will index this per-packet information by packet number and store the

"by packet number and crypto context"?

-- 
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/1939#pullrequestreview-170953385

---

From: ianswett <[email-masked]>
Date: Fri, 02 Nov 2018 19:22:09 -0700

ianswett commented on this pull request.

I'll keep it as is for now, this PR is 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/pull/1939#discussion_r230542522

---

From: ianswett <[email-masked]>
Date: Fri, 02 Nov 2018 19:25:04 -0700

Hmm, I am actually trying to say something different here, which is that you need to track frames like Stream frames, so if they're lost, you can decide whether to retransmit the data/etc.

-- 
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/1939#discussion_r230542585

---

From: ianswett <[email-masked]>
Date: Fri, 02 Nov 2018 19:26:41 -0700

@ianswett pushed 1 commit.

f6de336  Update draft-ietf-quic-recovery.md

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

---

From: ianswett <[email-masked]>
Date: Fri, 02 Nov 2018 19:29:13 -0700

3506fa6  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1939/files/f6de3364ab31ac593e2ada6ca1432ce9c70930bd..3506fa6f0a8f27c3778955011508b0cc647fdf94

---

From: ianswett <[email-masked]>
Date: Fri, 02 Nov 2018 19:31:43 -0700

Merged #1939 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/1939#event-1944202187

---

From: Martin Thomson <[email-masked]>
Date: Sat, 03 Nov 2018 04:03:47 -0700

@ianswett for some reason this got merged with some other changes.  I'm going to open you a new 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/1939#issuecomment-435579090

---

From: Martin Thomson <[email-masked]>
Date: Sat, 03 Nov 2018 04:07:29 -0700

See #1956.

-- 
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/1939#issuecomment-435579353