Subject: [quicwg/base-drafts] MAY send 1 packet entering recovery (#3443)

From: ianswett <[email-masked]>
Date: Sun, 09 Feb 2020 05:10:01 -0800

Similar to TCP as described in Section 5 of RFC 6675

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

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

-- Commit Summary --

  * MAY send 1 packet entering recovery

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Sun, 09 Feb 2020 11:01:45 -0800

@ianswett pushed 1 commit.

998a43f5aebbd68123e968cd6c9f88e887825a31  Update pseudocode

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3443/files/ecb7a76c16022536bc420fa08e74af11d2602ed6..998a43f5aebbd68123e968cd6c9f88e887825a31

---

From: ianswett <[email-masked]>
Date: Sun, 09 Feb 2020 11:06:32 -0800

02c21901dcfbf55abb1a801ac5118cab06f0ad2a  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/3443/files/998a43f5aebbd68123e968cd6c9f88e887825a31..02c21901dcfbf55abb1a801ac5118cab06f0ad2a

---

From: martinduke <[email-masked]>
Date: Tue, 11 Feb 2020 10:30:23 -0800

Should we restrict this to retransmittable frames that were lost in the packet? If it was a small packet, can I pack to the MTU? What about pure ACKs that I've declared 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/3443#issuecomment-584783128

---

From: ianswett <[email-masked]>
Date: Tue, 11 Feb 2020 13:16:00 -0800

Those are great questions.  There are two goals in my mind.  The first is to ensure loss recovery is not unnecessarily slower than TCP.  The second is to ensure this behavior isn't significantly more aggressive than TCP.  But within those bounds, I'm inclined to not be too restrictive, if that makes sense?  Suggestions welcome if you think they'd be an improvement.

-- 
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/3443#issuecomment-584856154

---

From: martinduke <[email-masked]>
Date: Tue, 11 Feb 2020 14:56:15 -0800

Well the pedantic answer is that TCP would only detect a loss if there was something in the bytestream. the QUIC equivalents would then be any packet that contained STREAM, CRYPTO, MAX_STREAM_DATA, etc, plus any packet that flipped the key update bit. But not ACK, MAX_DATA, and other things that are in the TCP header. That is obviously a dumb way to approach things.

I don't actually have a strong opinion on this one way of the other, but if I read the draft correctly, the loss of an ACK-only packet will affect cwnd in QUIC but not TCP (granted, a cwnd-constrained endpoint sending pure ACKs is a corner case). So I guess I'd err on the side of permitting the packet in response to loss of an ACK, as we're already quite a bit less aggressive than TCP in that case.

Anyway, do we want any normative language against, say, using this permission to send high-priority new data instead of the lost data?

-- 
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/3443#issuecomment-584898157

---

From: ianswett <[email-masked]>
Date: Tue, 11 Feb 2020 15:11:34 -0800

Adding language against sending new data or data besides data that's been lost couples the "can I send" with "what should I send" questions more than I would like unless it's absolutely critical.

FYI, to be declared lost, it must be in-flight: https://github.com/quicwg/base-drafts/blob/master/draft-ietf-quic-recovery.md#acknowledgement-based-detection-ack-loss-detection
"A packet is declared lost if it meets all the following conditions:
  * The packet is unacknowledged, in-flight, and was sent prior to an acknowledged 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/3443#issuecomment-584904542

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 11 Feb 2020 15:29:05 -0800

janaiyengar approved this pull request.

Just a nit, but LGTM.

> @@ -687,6 +687,12 @@ The recovery period limits congestion window reduction to once per round trip.
 During recovery, the congestion window remains unchanged irrespective of new
 losses or increases in the ECN-CE counter.
 
+When entering recovery, a single packet MAY be sent even if bytes in flight
+now exceeds the recently reduced congestion window.  This speeds up loss
+recovery if the data in the lost packet is retransmitted and is similar to TCP
+as described in Section 5 of {{?RFC6675}}.  If further packets are lost after
+already in recovery, the congestion window MUST be observed as normal.

nit: "If further packets are lost while the sender is in recovery, sending any packets in response MUST obey the congestion window 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/3443#pullrequestreview-357071802

---

From: martinduke <[email-masked]>
Date: Tue, 11 Feb 2020 15:29:49 -0800

Ah, I think we're fine then.

-- 
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/3443#issuecomment-584912935

---

From: martinduke <[email-masked]>
Date: Tue, 11 Feb 2020 15:29:58 -0800

martinduke 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/3443#pullrequestreview-357072337

---

From: ianswett <[email-masked]>
Date: Wed, 12 Feb 2020 08:08:55 -0800

d7fabb01733c3ccfb21c2eedb25d17e4de82cbb6  Jana's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3443/files/02c21901dcfbf55abb1a801ac5118cab06f0ad2a..d7fabb01733c3ccfb21c2eedb25d17e4de82cbb6

---

From: ianswett <[email-masked]>
Date: Wed, 12 Feb 2020 08:09:08 -0800

ianswett commented on this pull request.

Thanks, 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/3443#discussion_r378351929

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 27 Feb 2020 17:19:41 -0800

-- 
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/3443#pullrequestreview-366108391

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 27 Feb 2020 17:21:05 -0800

@janaiyengar pushed 1 commit.

40ed46141174740f8f9bb9aec5fef2b03594bab3  Merge branch 'master' into ianswett-send-one-packet

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3443/files/d7fabb01733c3ccfb21c2eedb25d17e4de82cbb6..40ed46141174740f8f9bb9aec5fef2b03594bab3