Subject: [quicwg/base-drafts] Handshake packets aren't special (#4853)

From: ianswett <[email-masked]>
Date: Fri, 26 Mar 2021 11:57:31 -0700

Fixes #4847
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Handshake packets aren&#39;t special

-- File Changes --

    M draft-ietf-quic-recovery.md (7)

-- Patch Links --

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

---

From: Praveen Balasubramanian <[email-masked]>
Date: Fri, 26 Mar 2021 12:02:57 -0700

@pravb commented on this pull request.

> @@ -268,6 +268,13 @@ packets. While this increases network load, it is considered safe, since the
 sender will still reduce its sending rate exponentially under persistent
 congestion ({{pto}}).
 
+## Handshake packets are not special
+
+TCP reduces the congestion window to one packet when the SYN or SYN/ACK
+are lost ({{?RFC5682}}).  QUIC treats loss of a packet containing handshake

Typo: RFC5681. 5682 is F-RTO.

-- 
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/4853#pullrequestreview-622456756

---

From: Praveen Balasubramanian <[email-masked]>
Date: Fri, 26 Mar 2021 12:06:48 -0700

+## Handshake packets are not special
+
+TCP reduces the congestion window to one packet when the SYN or SYN/ACK
+are lost ({{?RFC5682}}).  QUIC treats loss of a packet containing handshake
+data the same as other losses and reduces the congestion window.  When the
+PTO timer expires during the handshake, one or two packets are sent and
+the timer is doubled.

Another suggestion for the first sentence:
TCP treats the loss of SYN or SYN-ACK packet as persistent congestion and reduces the congestion window to one packet.  

-- 
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/4853#pullrequestreview-622459411

---

From: Praveen Balasubramanian <[email-masked]>
Date: Fri, 26 Mar 2021 12:06:56 -0700

@pravb 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/4853#pullrequestreview-622459502

---

From: ianswett <[email-masked]>
Date: Sat, 27 Mar 2021 11:27:57 -0700

@ianswett commented on this pull request.

```suggestion
are lost ({{?RFC5681}}).  QUIC treats loss of a packet containing handshake
```

-- 
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/4853#pullrequestreview-622692140

---

From: ianswett <[email-masked]>
Date: Sat, 27 Mar 2021 11:28:02 -0700

@ianswett pushed 1 commit.

29d4353616d0369670491e9d48f78dae7bebf4a7  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/4853/files/74acc7072e928c3cca1f2cd29338c4ce6292559b..29d4353616d0369670491e9d48f78dae7bebf4a7

---

From: ianswett <[email-masked]>
Date: Sat, 27 Mar 2021 11:33:24 -0700

a3aa07b295b15cd97b1e548be231ca966ded9b43  Praveen's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4853/files/29d4353616d0369670491e9d48f78dae7bebf4a7..a3aa07b295b15cd97b1e548be231ca966ded9b43

---

From: ianswett <[email-masked]>
Date: Sat, 27 Mar 2021 11:33:42 -0700

Thanks Praveen, suggestion taken.

Thanks!

-- 
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/4853#pullrequestreview-622692163

---

From: Martin Thomson <[email-masked]>
Date: Sun, 28 Mar 2021 16:09:09 -0700

@martinthomson commented on this pull request.

> +reduces the congestion window.  When the PTO timer expires during the
+handshake, one or two packets are sent and the timer is doubled.

I don't think that you need this last sentence. 

> +reduces the congestion window to one packet ({{?RFC5681}}). QUIC treats
+loss of a packet containing handshake data the same as other losses and
+reduces the congestion window.  When the PTO timer expires during the

This second sentence could be shortened.

```suggestion
reduces the congestion window to one packet ({{?RFC5681}}). QUIC treats
loss of a packet containing handshake data the same as other losses.
```

After all, you don't mention the recovery period or the potential persistent congestion piece here, but both apply.

-- 
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/4853#pullrequestreview-622794207

---

From: Marten Seemann <[email-masked]>
Date: Mon, 29 Mar 2021 01:29:05 -0700

@marten-seemann 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/4853#pullrequestreview-622978688

---

From: ianswett <[email-masked]>
Date: Tue, 30 Mar 2021 11:17:08 -0700

I'm a bit hesitant to take your suggestions @martinthomson because I'm worried it won't be quite complete enough, but maybe @janaiyengar could take a look as well?

The intent is to clarify that QUIC has exponential backoff during the handshake, but possibly it's not clear or necessary?

On the issue, the concern was about QUIC responding to loss during the handshake, so I thought the note about reducing CWND was relevant, but I agree that this is not the place to explain the full algorithms.

-- 
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/4853#pullrequestreview-624566110

---

From: mjoras <[email-masked]>
Date: Tue, 30 Mar 2021 12:23:58 -0700

@mjoras commented on this pull request.

I think it's not totally clear from this text that you're essentially summarizing the broad behavior of loss recovery and probing. So maybe, if you want to keep these sentences, you need something like an "i.e."

-- 
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/4853#discussion_r604371438

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 30 Mar 2021 15:12:52 -0700

@janaiyengar commented on this pull request.

+1 to remove the last sentence... there are rules around PTO and handshake in 6.2.1 and 6.2.2, that should be enough I think. This would be repetitive, so if you think it bears repeating, then it's worth adding it, but I don't think we need 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/4853#discussion_r604465592