Subject: [quicwg/base-drafts] Update pseudocode to better match text (#3461)

From: ianswett <[email-masked]>
Date: Sat, 15 Feb 2020 11:28:00 -0800

If there are bytes_in_flight, then always do a standard PTO.
If not, it must be a client which is unsure if the peer has completed address validation.

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

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

-- Commit Summary --

  * Update pseudocode to better match text

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Sun, 16 Feb 2020 15:44:15 -0800

martinthomson commented on this pull request.

> @@ -1179,20 +1179,20 @@ OnLossDetectionTimeout():
     SetLossDetectionTimer()
     return
 
-  if (endpoint is client without 1-RTT keys):
+  if (bytes_in_flight > 0):
+    // PTO. Send new data if available, else retransmit old data.
+    // If neither is available, send a single PING frame.
+    _, pn_space = GetEarliestTimeAndSpace(
+      time_of_last_sent_ack_eliciting_packet)
+    SendOneOrTwoAckElicitingPackets(pn_space)
+  else:

```suggestion
  else if client:
```

Otherwise, the server might hit this path if it has sent HelloRetryRequest and had it acknowledged.  In that case, the server has to sit tight.  It's a perverse scenario, but PQ is coming...

-- 
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/3461#pullrequestreview-359439734

---

From: Martin Thomson <[email-masked]>
Date: Sun, 16 Feb 2020 15:47:06 -0800

SetLossDetectionTimer()
     return

Oh, this might need all the original constraints as well, otherwise this path will be hit by a quiescent client all the time.

-- 
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/3461#discussion_r379944712

---

From: ianswett <[email-masked]>
Date: Sun, 16 Feb 2020 16:55:32 -0800

@ianswett pushed 1 commit.

7918097aca5334aaabf4c8298070063adc27cc5d  Add assert

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3461/files/74fd16b602fd7b8913f883d17d23d0a3ddd7aa3f..7918097aca5334aaabf4c8298070063adc27cc5d

---

From: ianswett <[email-masked]>
Date: Sun, 16 Feb 2020 17:01:57 -0800

ianswett commented on this pull request.

     SetLossDetectionTimer()
     return

The alarm shouldn't be armed in those case, as stated above in the normative text
"If no data can be sent, then the PTO alarm MUST NOT be armed until datagrams have been received from the client."

Would you like some pseudocode added to the "Setting the Loss Detection Timer" pseudocode?

-- 
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/3461#discussion_r379951368

---

From: ianswett <[email-masked]>
Date: Sun, 16 Feb 2020 17:13:06 -0800

4de119f1d207ada8de6689112f876d9d2fc29527  Add check for address validation limits

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3461/files/7918097aca5334aaabf4c8298070063adc27cc5d..4de119f1d207ada8de6689112f876d9d2fc29527

---

From: Martin Thomson <[email-masked]>
Date: Sun, 16 Feb 2020 18:41:50 -0800

martinthomson approved this pull request.

Oh, I see how this all fits together now. I think that it's entirely too clever.

I suspect that some commentary is going to be needed to explain that the PTO timer isn't armed when

a: the server hits address validation limits (the new text), and
b: the endpoint has nothing to send (except when the client isn't sure that the server has completed validation)

> +  if (IsServerWhichReachedAddressValidationLimit()):
+    loss_detection_timer.cancel()
+    return

The odd mix of pseudo-prose and undefined functions is starting to get noticeable.

```
    if (server is at anti-amplification limit):
```

And later:

```
    if (no ack-eliciting packets in flight &&
        peer not awaiting address validation):
```

-- 
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/3461#pullrequestreview-359463182