Subject: [quicwg/base-drafts] If PTO is armed for anti-deadlock, use now() (#3556)

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 07:37:02 -0700

Use now() instead of setting the PTO based on an invalid time(0) when the PTO is armed for anti-deadlock and there are no bytes in flight.

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

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

-- Commit Summary --

  * If PTO is armed for anti-deadlock, use now()

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 07:39:12 -0700

@ianswett pushed 1 commit.

c08afeea0ecfa510b10f3924224500e5c667835d  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/3556/files/49526ef56c9225ee5ac2fce7a70403ead0b6df74..c08afeea0ecfa510b10f3924224500e5c667835d

---

From: Martin Thomson <[email-masked]>
Date: Mon, 30 Mar 2020 15:32:04 -0700

@martinthomson commented on this pull request.

Consistency FTW :)

> +  if (sent_time == 0)
+    assert(!PeerNotAwaitingAddressValidation())
+    sent_time = now()
   loss_detection_timer.update(sent_time + timeout)

```suggestion
  if (sent_time != 0)
    loss_detection_timer.update(sent_time + timeout)
```

-- 
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/3556#pullrequestreview-384278968

---

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 15:40:14 -0700

@ianswett commented on this pull request.

We need to arm the timer for anti-deadlock reasons even if sent_time is 0, so I think this needs to stay as is?

-- 
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/3556#discussion_r400537204

---

From: Martin Thomson <[email-masked]>
Date: Mon, 30 Mar 2020 17:19:54 -0700

My bad.  I didn't think that through.  Would it instead be better to change this line:

```
  time_of_last_sent_ack_eliciting_packet[kPacketNumberSpace] = 0
```
to 
```
  time_of_last_sent_ack_eliciting_packet[kPacketNumberSpace] = now()
```

-- 
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/3556#discussion_r400570511

---

From: ianswett <[email-masked]>
Date: Tue, 31 Mar 2020 11:28:15 -0700

I'm worried that's too clever.  This is a special case, but so are many things about the anti-deadlock functionality, unfortunately.

-- 
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/3556#discussion_r401124460

---

From: ianswett <[email-masked]>
Date: Tue, 31 Mar 2020 11:30:07 -0700

551c9763470354175b5e5c2075ca90fb941b337f  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/3556/files/c08afeea0ecfa510b10f3924224500e5c667835d..551c9763470354175b5e5c2075ca90fb941b337f