Subject: Re: [quicwg/base-drafts] Prevent an unlikely deadlock (#1965)

From: ianswett <[email-masked]>
Date: Tue, 18 Dec 2018 20:02:12 -0800

Given if there is something to be sent, I don't think this is a deadlock, I'm inclined to close this PR, both because it's old and I'm not convinced it's a practical issue.

-- 
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/1965#issuecomment-448462126

---

From: martinduke <[email-masked]>
Date: Tue, 18 Dec 2018 22:45:49 -0800

@nibanks The simplest way for this to happen is that the first TLP timeout resends some data, and then the original is acked. Thus we have an in-flight packet (the retransmission) but the actual application data is already gone from the send buffer.

The TLP timer will fire again, and there's nothing to send.

@ianswett There are still some problems with the new text. The main practical impact is that we will delay declaring the last packet lost, meaning we won't free up the flightsize or execute a congestion response as quickly. I'm having a little trouble reasoning about the full implications of this for new 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/1965#issuecomment-448487557

---

From: ianswett <[email-masked]>
Date: Wed, 19 Dec 2018 06:53:04 -0800

@ianswett pushed 1 commit.

ae6aedc35602c2da09ca7ef264f98b217856c04b  Merge branch 'master' into ianswett-tlp-deadlock

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1965/files/9905361d72c7b94468ccc8400dcc6220beddac7d..ae6aedc35602c2da09ca7ef264f98b217856c04b

---

From: ianswett <[email-masked]>
Date: Wed, 19 Dec 2018 06:53:19 -0800

fe11f580127517a9c7b0cc9fc585d570622c4564  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/1965/files/ae6aedc35602c2da09ca7ef264f98b217856c04b..fe11f580127517a9c7b0cc9fc585d570622c4564

---

From: ianswett <[email-masked]>
Date: Wed, 19 Dec 2018 06:56:49 -0800

699fb76fa02e648c2bce41fc0adeadd79893e0bd  Not a deadlock

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1965/files/fe11f580127517a9c7b0cc9fc585d570622c4564..699fb76fa02e648c2bce41fc0adeadd79893e0bd

---

From: ianswett <[email-masked]>
Date: Wed, 19 Dec 2018 14:58:59 +0000 (UTC)

I updated this to clarify it's not really a deadlock, but suggest what should be done on PTO in this case: either send an ack-eliciting packet or declare the packets 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/1965#issuecomment-448624703

---

From: martinduke <[email-masked]>
Date: Wed, 19 Dec 2018 08:16:46 -0800

For the record, I believe this was a potential deadlock:
Pkt 5 outstanding, flightsize = cwnd = 1 MSS
TLP timeout, resend in pkt 6 (flightsize = 2 MSS)
Pkt 5 acked, no data in send queue (flightsize = cwnd = 1 MSS)
TLP timeout, nothing to send, no new timers.
If new data arrives, there is no way to progress.

-- 
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/1965#issuecomment-448653175

---

From: ianswett <[email-masked]>
Date: Wed, 19 Dec 2018 08:25:42 -0800

Previously, Jana said you should store that the PTO had fired and there was nothing to send, so when data does arrive to send, you can immediately send it.  But that wasn't explicitly documented anywhere to my knowledge.

-- 
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/1965#issuecomment-448656469

---

From: janaiyengar <[email-masked]>
Date: Wed, 19 Dec 2018 14:05:07 -0800

janaiyengar commented on this pull request.

I think @martinduke is right that we should handle this particular case, and I'm fine with the resolution in this PR. A few comments (and the text is in teh wrong place)

> @@ -421,6 +421,15 @@ control and loss recovery state, including resetting any pending timers.  Either
 packet indicates that the Initial was received but not processed.  Neither
 packet can be treated as an acknowledgment for the Initial.
 
+If no new data or unacknowledged data is available to send, an ack-eliciting
+packet SHOULD be sent.  Sending a retransmittable frame ensures that any in
+flight packets are acknowledged or declared lost in a timely manner.  If no
+ack-eliciting packet is sent, the in flight packets should be declared lost
+to avoid repeatedly arming and firing the PTO timer.
+
+A sender may not know that a packet being sent is a tail packet.  Consequently,
+a sender may have to arm or adjust the TLP timer on every sent retransmittable
+packet.

I suspect this para is leftover and should be removed.

> @@ -421,6 +421,15 @@ control and loss recovery state, including resetting any pending timers.  Either
 packet indicates that the Initial was received but not processed.  Neither
 packet can be treated as an acknowledgment for the Initial.
 
+If no new data or unacknowledged data is available to send, an ack-eliciting
+packet SHOULD be sent.  Sending a retransmittable frame ensures that any in
+flight packets are acknowledged or declared lost in a timely manner.  If no
+ack-eliciting packet is sent, the in flight packets should be declared lost

```suggestion
ack-eliciting packet is sent, any packets currently in flight should be declared lost
```

> @@ -421,6 +421,15 @@ control and loss recovery state, including resetting any pending timers.  Either
 packet indicates that the Initial was received but not processed.  Neither
 packet can be treated as an acknowledgment for the Initial.
 
+If no new data or unacknowledged data is available to send, an ack-eliciting

This new text should be further below in the #pto section.

-- 
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/1965#pullrequestreview-186767719

---

From: ianswett <[email-masked]>
Date: Wed, 19 Dec 2018 16:06:14 -0800

b1807240c6ff92930cb59c1e890bcc1005fa3546  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/1965/files/699fb76fa02e648c2bce41fc0adeadd79893e0bd..b1807240c6ff92930cb59c1e890bcc1005fa3546

---

From: ianswett <[email-masked]>
Date: Wed, 19 Dec 2018 16:11:01 -0800

59377bf109b86f729e3ee2951dd1774d14e5a383  Jana's suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1965/files/b1807240c6ff92930cb59c1e890bcc1005fa3546..59377bf109b86f729e3ee2951dd1774d14e5a383

---

From: ianswett <[email-masked]>
Date: Wed, 19 Dec 2018 16:11:24 -0800

6b719028f97025406f371136f632ea5776571577  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/1965/files/59377bf109b86f729e3ee2951dd1774d14e5a383..6b719028f97025406f371136f632ea5776571577

---

From: janaiyengar <[email-masked]>
Date: Wed, 19 Dec 2018 17:26:43 -0800

Hm. I started to think about how to rephrase a bit, which ended up leading to #2212. Take a look. 

-- 
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/1965#issuecomment-448823566

---

From: janaiyengar <[email-masked]>
Date: Thu, 20 Dec 2018 13:48:14 -0800

@janaiyengar pushed 7 commits.

df582552db8b1f791861367046ff6e50d66c4bc4  Proposed change: MUST mark inflight data as lost
cf7894acb8e8a557b857f4baf281a10756e2e469  reflow
a7b0f3699c6a341a2aa2a6e9a281dba06cdd6f80  add MAY re-arm PTO
66c435b4bae996e7cae2d74bd59ed7b1d68acfff  more rephrasing
302f3df6b6d2f41ff8224a5c092c6e9243502208  more simple
8e1c599d517cee2dbe86f74b61194b2ab02217e1  comma
f09333babf3067be1694b568afb54994dc6f5f5e  Merge pull request #2212 from quicwg/janaiyengar-patch-2

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1965/files/6b719028f97025406f371136f632ea5776571577..f09333babf3067be1694b568afb54994dc6f5f5e

---

From: janaiyengar <[email-masked]>
Date: Thu, 20 Dec 2018 13:49:20 -0800

janaiyengar approved this pull request.

@martinduke : See if this resolves your concerns.

-- 
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/1965#pullrequestreview-187198599

---

From: ianswett <[email-masked]>
Date: Thu, 20 Dec 2018 13:50:18 -0800

@martinduke Jana and I replaced my text with a version of his PR.  We believe this solves the issue you point out 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/1965#issuecomment-449146643

---

From: martinduke <[email-masked]>
Date: Wed, 26 Dec 2018 08:48:14 -0800

I like this language a lot. Ship 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/1965#issuecomment-449992351

---

From: ianswett <[email-masked]>
Date: Wed, 26 Dec 2018 20:22:01 -0800

Merged #1965 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/1965#event-2045311539