Subject: [quicwg/base-drafts] Include handshake anti-deadlock logic in pseudocode (#2281)

From: Benjamin Saunders <[email-masked]>
Date: Mon, 31 Dec 2018 17:08:44 -0800

Recovery §6.2.1 instructs:
> Because the server could be blocked until more packets are received, the client MUST start the crypto retransmission timer even if there is no unacknowledged CRYPTO data. If the timer expires and the client has no CRYPTO data to retransmit and does not have Handshake keys, it SHOULD send an Initial packet in a UDP datagram of at least 1200 bytes. If the client has Handshake keys, it SHOULD send a Handshake packet.

but this was not reflected in the pseudocode. I think these changes reflect the intended behavior.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Include handshake anti-deadlock logic in pseudocode

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 31 Dec 2018 17:55:25 -0800

martinthomson commented on this pull request.

I think that this requirement only applies when Initial packets are in flight.  The text allows for Handshake, but I think that we don't need these special rules for Handshake packets.  Though we might want to send more aggressively for Handshake, the standard rules are sufficient there.  I wonder if we need to change the text.

> @@ -823,6 +824,9 @@ Pseudocode for OnLossDetectionTimeout follows:
        // Crypto retransmission timeout.
        RetransmitUnackedCryptoData()
        crypto_count++
+     else if (handshaking client)
+       SendOnePacket()

Is this right?
```suggestion
        SendPaddedInitialPacket()
```

Or is it this?
```suggestion
        SendPaddedPacket()
```

-- 
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/2281#pullrequestreview-188545445

---

From: Benjamin Saunders <[email-masked]>
Date: Mon, 31 Dec 2018 20:22:03 -0800

@Ralith pushed 1 commit.

329d238c245a559be4c075ddaa53cf1a2961767d  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/2281/files/3e8b05d80bcd18c6709f929e9faff15c1a5b5bab..329d238c245a559be4c075ddaa53cf1a2961767d

---

From: Benjamin Saunders <[email-masked]>
Date: Mon, 31 Dec 2018 20:26:28 -0800

> I think that this requirement only applies when Initial packets are in flight. The text allows for Handshake, but I think that we don't need these special rules for Handshake packets.

I interpreted "MUST NOT send more than three times as many bytes as the number of bytes they have received" as referring to real bytes transmitted, rather than congestion control bytes in flight. Given that, repeated complete loss of the server's handshake flights can cause a server to overrun its budget before the client's address is validated by handshake completion, requiring a padding packet to be transmitted even if the client received acknowledgement for its Initial.

-- 
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/2281#issuecomment-450709115