Subject: [quicwg/base-drafts] Don't retransmit lost packets that are acked later (#3957)

From: Jana Iyengar <[email-masked]>
Date: Thu, 23 Jul 2020 19:58:20 -0700

Closes #3956.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * retain info on lost packets

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Thu, 23 Jul 2020 22:35:49 -0700

@martinthomson approved this pull request.

This is useful information and in the right place.  Thanks.

I have a small suggestion, but I'm OK with the original phrasing.

> +When a sender declares a packet as lost and either retransmits the contained
+frames or marks them for retransmission, the sender SHOULD avoid subsequent
+retransmission of this information if it later receives an acknowledgement for
+that packet. Doing so requires senders to retain information about packets after
+they are declared lost. A sender can discard this information after a period of
+time elapses, such as three times the PTO (Section 6.2 of {{QUIC-RECOVERY}}), or
+on other events, such as reaching a memory limit.

```suggestion
Acknowledgments can be received after a sender declares a packet as lost.  A
sender SHOULD avoid sending information again after receiving a late
acknowledgment. Doing so requires senders to retain information about packets
for some time after they are declared lost. A sender can discard this
information after a period of time elapses, such as three times the PTO (Section
6.2 of {{QUIC-RECOVERY}}), or on other events, such as reaching a memory 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/3957#pullrequestreview-454642705

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 24 Jul 2020 00:06:29 -0700

@kazuho commented on this pull request.

I think I like @martinthomson's idea of splitting the sentence into two. The only thing I might prefer is changing the last part of the second sentence "after receiving a late acknowledgement" to something like "after receiving such an acknowledgement" to make it clearer, but anyways it's good.

-- 
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/3957#discussion_r459884709

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 24 Jul 2020 00:08:31 -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/3957#pullrequestreview-454673071

---

From: ianswett <[email-masked]>
Date: Fri, 24 Jul 2020 11:45:23 -0700

@ianswett commented on this pull request.

Thanks for adding this, some more comments.

> @@ -3804,6 +3804,14 @@ number length, connection ID length, and path MTU.  A receiver MUST accept
 packets containing an outdated frame, such as a MAX_DATA frame carrying a
 smaller maximum data than one found in an older packet.

3*PTO is really long in this case, and it can consume a fair bit of memory depending upon implementation details.  I'd suggest an ~RTT, since by that point you're expecting the acknowledgement of the retransmitted data.

How about "..after receiving an acknowledgement for a previously lost packet."  I think late acknowledgement is potentially confusing.

And how about starting with "A packet can be acknowledged after being declared lost." instead of "Acknowledgements can be received after a sender declares a packet as lost."

-- 
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/3957#pullrequestreview-455106331

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 24 Jul 2020 15:05:05 -0700

Generally speaking, I tend to think that 3*PTO is fine, and that is because the size of a sentmap entry (i.e., the amount of memory required to remember what was sent in a packet) is only a tiny fraction of the MTU.

If your send buffer is capable of retaining X packets amount of data, then it does not matter if you use *one* tiny fraction, *two* tiny fractions,  or *three*, for sentmap.

All that said, I agree that it might sound a bit frightening, it does not need to be as long as 3*PTO, and therefore my view is that this is a trade-off issue between simplicity (i.e. reusing 3*PTO timer) vs. less frightening.

-- 
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/3957#discussion_r460307539

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 24 Jul 2020 15:10:05 -0700

@janaiyengar pushed 1 commit.

98c9d0486998cf804b6b85010958e9480af69577  rewrite based on feedback

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3957/files/d2658fd51765ed7dbe3ecd49c546940c07ea862d..98c9d0486998cf804b6b85010958e9480af69577

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 24 Jul 2020 15:10:43 -0700

@janaiyengar commented on this pull request.

I've rewritten the long sentence based on feedback

The PTO is the period of time that you expect the ack back by, not RTT. 

It's true that 3 PTO is a conservative value, but, it's only on lost packets. When that's the uncommon case, it won't consume a lot of memory. When it's the common case, you're not sending a lot of packets anymore because the sender is in congestion, so it shouldn't also cause memory exhaustion.

Either way, this is a suggestion, not a recommendation.

-- 
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/3957#pullrequestreview-455206964

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 24 Jul 2020 15:13:36 -0700

b0ed70ae8ca5a93ccf92d7b23f981181158dbcdf  smithing

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3957/files/98c9d0486998cf804b6b85010958e9480af69577..b0ed70ae8ca5a93ccf92d7b23f981181158dbcdf

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 24 Jul 2020 15:14:33 -0700

e8db499a77adc0e4d137ef0cb2fd856b21cf2411  split up sentence

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

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 24 Jul 2020 15:49:03 -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/3957#pullrequestreview-455222446

---

From: ianswett <[email-masked]>
Date: Mon, 27 Jul 2020 07:45:07 -0700

I still think 3*PTO is far too long.  This time period exists to ensure there's sufficient tolerance of reordering, not to wait for multiple PTOs to fire.

> +are declared lost. A sender can discard this information after a period of time
+elapses, such as three times the PTO (Section 6.2 of {{QUIC-RECOVERY}}), or on

```suggestion
are declared lost. A sender can discard this information a period of time after the
packet is lost, such as after the Smoothed RTT (Section 5 of {{QUIC-RECOVERY}}), or on
```

-- 
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/3957#pullrequestreview-455849653

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 27 Jul 2020 08:29:24 -0700

I'm happy to change it to 1 x PTO. I'd like the ack delay and the rttdev to be included in this 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/3957#discussion_r460976107

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 27 Jul 2020 08:31:19 -0700

7abdc4b8128585bbaaa07b4f5b30991640d3cdea  wait for one PTO, not 3

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3957/files/e8db499a77adc0e4d137ef0cb2fd856b21cf2411..7abdc4b8128585bbaaa07b4f5b30991640d3cdea

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 27 Jul 2020 08:38:30 -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/3957#pullrequestreview-455900837

---

From: ianswett <[email-masked]>
Date: Mon, 27 Jul 2020 09:29:03 -0700

ack delay shouldn't need to be included, because the packet would be acknowledged immediately.

Including RTTVar implies that the network's reordering is sometimes as much 3*SRTT, but assuming the data was retransmitted, you'd expect the retransmission to be acknowledged in about 1 RTT, so suggesting you might need to wait longer seems odd to me.

-- 
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/3957#discussion_r461015398

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 27 Jul 2020 16:01:46 -0700

e3999ba854c5641351cc3630060c1c28e2418abe  sync recovery

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3957/files/7abdc4b8128585bbaaa07b4f5b30991640d3cdea..e3999ba854c5641351cc3630060c1c28e2418abe

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 27 Jul 2020 16:09:21 -0700

Added changes to recovery as well.

-- 
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/3957#issuecomment-664683484

---

From: ianswett <[email-masked]>
Date: Mon, 27 Jul 2020 18:04:19 -0700

Some suggestions, but I think we're close.

> @@ -1007,9 +1007,9 @@ It is expected that implementations will be able to access this information by
 packet number and crypto context and store the per-packet fields
 ({{sent-packets-fields}}) for loss recovery and congestion control.
 
-After a packet is declared lost, the endpoint can track it for an amount of
-time comparable to the maximum expected packet reordering, such as 1 RTT.
-This allows for detection of spurious retransmissions.
+After a packet is declared lost, the endpoint can still maintain state for it
+for an amount of time, such as a PTO, to allow for packet reordering. This

```suggestion
for an amount of time to allow for packet reordering. This
```

> +acknowledged. This includes packets that are acknowledged after being declared
+lost. Doing so requires senders to retain information about packets after they
+are declared lost. A sender can discard this information after a period of time
+elapses, such as a PTO (Section 6.2 of {{QUIC-RECOVERY}}), or on other events,
+such as reaching a memory limit.

```suggestion
acknowledged. Doing this for packets acknowledged after they are declared
lost requires retaining state for lost packets. A sender can discard this information
after the expected maximum packet reordering elapses, such as after PTO has elapsed
(Section 6.2 of {{QUIC-RECOVERY}}), or on other events, such as reaching a packet 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/3957#pullrequestreview-456231326

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 27 Jul 2020 18:25:08 -0700

I will take the compression, but we don't need to say "expected maximum packet reordering", since that's not a well-defined term. I'll spell in out in other words.

-- 
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/3957#discussion_r461262443

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 27 Jul 2020 18:33:39 -0700

I'm trying to wordsmith this compression, but it's a bit difficult. The text is a bit verbose, but I think it is clearer. Unless you think there's a substantive difference, I'd prefer the existing text for clarity.

-- 
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/3957#discussion_r461264796

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 27 Jul 2020 18:34:08 -0700

89ff7fad0f72429751c3e3e2868845abc90fcc5a  add ref to transport

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3957/files/e3999ba854c5641351cc3630060c1c28e2418abe..89ff7fad0f72429751c3e3e2868845abc90fcc5a

---

From: Martin Thomson <[email-masked]>
Date: Mon, 27 Jul 2020 22:51:44 -0700

@martinthomson commented on this pull request.

The expanded form 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/3957#discussion_r461334248

---

From: ianswett <[email-masked]>
Date: Tue, 28 Jul 2020 07:42:03 -0700

The intent was to both make it clearer and talk about the goal of waiting for a reordering period.  I find the existing text a bit unclear, which is why I put the first two sentences together.

-- 
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/3957#discussion_r461636266

---

From: ianswett <[email-masked]>
Date: Tue, 28 Jul 2020 07:43:15 -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/3957#pullrequestreview-456706770

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 16:52:30 -0700

Ok, added some rationale.

-- 
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/3957#discussion_r461958218

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 16:52:45 -0700

2e534ad9a0be9f6d41340f7ceabca3dfad610eda  Ian's comment

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3957/files/89ff7fad0f72429751c3e3e2868845abc90fcc5a..2e534ad9a0be9f6d41340f7ceabca3dfad610eda

---

From: ianswett <[email-masked]>
Date: Tue, 28 Jul 2020 17:33:15 -0700

I think what you're saying is fine, but another attempt at an editorial suggstion.

> +acknowledged. This includes packets that are acknowledged after being declared
+lost, which can happen in the presence of network reordering. Doing so requires
+senders to retain information about packets after they are declared lost. A

```suggestion
acknowledged. This includes packets acknowledged after being
declared lost due to network reordering, which requires senders to
retain information about packets after they are declared lost. A
```

-- 
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/3957#pullrequestreview-457117735

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 19:11:44 -0700

That's a run-on sentence.

-- 
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/3957#discussion_r461996220

---

From: Martin Thomson <[email-masked]>
Date: Tue, 28 Jul 2020 21:39:39 -0700

Let's not thrash too much on this one.

-- 
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/3957#pullrequestreview-457188857

---

From: ianswett <[email-masked]>
Date: Wed, 29 Jul 2020 04:45:21 -0700

Fair, I just find the current structure awkward.  Either way, this proposal is basically done.

-- 
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/3957#discussion_r462236836