Subject: [quicwg/base-drafts] Better duplicate detection (#3696)

From: Martin Thomson <[email-masked]>
Date: Mon, 25 May 2020 21:44:23 -0700

Including mention of a minimum packet number and the risks in setting
that value.

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

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

-- Commit Summary --

  * Better duplicate detection

-- File Changes --

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

-- Patch Links --

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

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 25 May 2020 22:16:16 -0700

@kazuho commented on this pull request.

Thank you for the PR. Looks mostly good, though I have some comments.

> @@ -3204,9 +3204,15 @@ response to further packets that it receives.
 A receiver MUST discard a newly unprotected packet unless it is certain that it
 has not processed another packet with the same packet number from the same
 packet number space. Duplicate suppression MUST happen after removing packet
-protection for the reasons described in Section 9.3 of {{QUIC-TLS}}. An
-efficient algorithm for duplicate suppression can be found in Section 3.4.3 of
-{{?RFC4303}}.
+protection for the reasons described in Section 9.3 of {{QUIC-TLS}}.
+
+Endpoints that track individual packets for the purposes of detecting duplicates
+might accumulate excessive state.  The data required for detecting duplicates

Would it be possible for us to recommend tracking ranges? For example, it might be a good idea to state like: _For the purposes of detecting duplicates, endpoints might track individual packets or ranges of consecutive packet numbers, though tracking individual packets might accumulate excessive state._

> @@ -3204,9 +3204,15 @@ response to further packets that it receives.
 A receiver MUST discard a newly unprotected packet unless it is certain that it
 has not processed another packet with the same packet number from the same
 packet number space. Duplicate suppression MUST happen after removing packet
-protection for the reasons described in Section 9.3 of {{QUIC-TLS}}. An
-efficient algorithm for duplicate suppression can be found in Section 3.4.3 of
-{{?RFC4303}}.
+protection for the reasons described in Section 9.3 of {{QUIC-TLS}}.
+
+Endpoints that track individual packets for the purposes of detecting duplicates
+might accumulate excessive state.  The data required for detecting duplicates
+can be limited by maintaining a minimum packet number below which all packets
+are immediately dropped.  In setting a minimum packet number endpoints might
+need to account for large variations in round trip time that could significantly
+delay packets, especially if a peer migrates to or probes a different network
+path; see {{migration}}.

I think it might be a good idea to change this sentence to something like: _In setting a minimum packet number endpoints **have** to account for large variations in round trip time that could significantly delay packets, especially if a peer probes a different network path; see {migration}}._

The changes are use of "have to" rather than "might," and omitting the mention of migration.

Migration is not an issue, because when the peer migrates to a path with higher latency there would be no loss, and because when the peer migrates to a path with lower latency, the loss would be a one-time effect that can be recovered.

In contrast to that, for path migration to work, endpoints have to be capable of deduplicating packets with significant reordering. Without that, the peer cannot reliably probe a path while sending data.

I think this is essentially a requirement regardless of how we phrase it.

-- 
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/3696#pullrequestreview-417972533

---

From: Martin Thomson <[email-masked]>
Date: Mon, 25 May 2020 22:23:30 -0700

@martinthomson commented on this pull request.

I considered that, but I don't feel like a recommendation is necessary.  It's the natural implementation strategy because of how we structured ACK frames, so there is enough bias there.  But it crosses somewhat over from "here are the pitfalls" into "here is how you build a stack" too much for me.

I see how you might pick on probing exclusively.  I'll remove migration and just identify probing.

s/have/need means the same to me.  It's 6919 language of a different form :)

-- 
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/3696#discussion_r430161554

---

From: Martin Thomson <[email-masked]>
Date: Mon, 25 May 2020 22:24:46 -0700

@martinthomson pushed 1 commit.

9f8038def838e3d42c1595d35c5ba70bce19ea41  Remove migration from the example

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3696/files/927fe3957b1930c38fbdf68dd950eec7bdff7db8..9f8038def838e3d42c1595d35c5ba70bce19ea41

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 26 May 2020 05:25:30 +0000 (UTC)

Thank you for elaborating. This works for 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/3696#discussion_r430162408

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 25 May 2020 22:26:05 -0700

I think I'm advocating for removing "might."

-- 
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/3696#discussion_r430162587

---

From: Martin Thomson <[email-masked]>
Date: Mon, 25 May 2020 22:27:15 -0700

Oh, I see.  I was thinking that you don't need to do this if you disable_active_migration.  Which is why the probing mention is important.

-- 
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/3696#discussion_r430162861

---

From: Martin Thomson <[email-masked]>
Date: Mon, 25 May 2020 22:39:23 -0700

0f6ca28b653cc6c0fdbe7450d027fa6c619e21a7  Rewording to avoid potential confusion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3696/files/9f8038def838e3d42c1595d35c5ba70bce19ea41..0f6ca28b653cc6c0fdbe7450d027fa6c619e21a7

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 25 May 2020 22:41:56 -0700

@kazuho approved this pull request.

Thank you! Looks good 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/3696#pullrequestreview-417983259

---

From: Marten Seemann <[email-masked]>
Date: Tue, 26 May 2020 03:20:35 -0700

@marten-seemann commented on this pull request.

> @@ -3204,9 +3204,16 @@ response to further packets that it receives.
+are immediately dropped.  Endpoints that use a minimum packet number need to
+ensure that any value does not cause delayed packets to be dropped.  Any minimum

This doesn't apply to arbitrarily delayed packets, otherwise every minimum value will be too high.

-- 
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/3696#pullrequestreview-418162483

---

From: ianswett <[email-masked]>
Date: Tue, 26 May 2020 16:01:31 -0700

@ianswett commented on this pull request.

```suggestion
Endpoints that track all individual packets for the purposes of detecting duplicates
```

```suggestion
are at risk of accumulating excessive state.  The data required for detecting duplicates
```

I think I'd drop this 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/3696#pullrequestreview-418734141

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 May 2020 16:03:05 -0700

This was the entire point of the issue though.

-- 
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/3696#discussion_r430753959

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 May 2020 16:03:36 -0700

d90c544322dc27c22539bb87fa09a6a110af4ab7  Tweaking

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3696/files/0f6ca28b653cc6c0fdbe7450d027fa6c619e21a7..d90c544322dc27c22539bb87fa09a6a110af4ab7

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 May 2020 16:07:42 -0700

@martinthomson pushed 2 commits.

081f95f0de6f0b3c0736c36e0062a8cee6c540e6  Rewrap
cbb0b2bc2b10816f686896779361cc42f4be8c35  remove unnecessary sentence

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

---

From: ianswett <[email-masked]>
Date: Tue, 26 May 2020 16:08:07 -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/3696#pullrequestreview-418738077

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 26 May 2020 16:54:18 -0700

@janaiyengar approved this pull request.

 A receiver MUST discard a newly unprotected packet unless it is certain that it
 has not processed another packet with the same packet number from the same
 packet number space. Duplicate suppression MUST happen after removing packet
-protection for the reasons described in Section 9.3 of {{QUIC-TLS}}. An
-efficient algorithm for duplicate suppression can be found in Section 3.4.3 of
-{{?RFC4303}}.
+protection for the reasons described in Section 9.3 of {{QUIC-TLS}}.
+
+Endpoints that track all individual packets for the purposes of detecting
+duplicates are at risk of accumulating excessive state.  The data required for
+detecting duplicates can be limited by maintaining a minimum packet number below
+which all packets are immediately dropped.  Any minimum needs to account for
+large variations in round trip time, which includes the possibility that a peer
+might probe a network path with a much longer round trip time; see

```suggestion
might probe network paths with a much larger round trip times; see
```

-- 
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/3696#pullrequestreview-418754341

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 May 2020 18:27:06 -0700

4d1bee363f0289ea7388a8283fe131c6841f6f23  plurals

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 May 2020 18:27:21 -0700

Merged #3696 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/3696#event-3376003151