Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

handshake timer spec is inconsistent #1816

Closed
martinduke opened this issue Oct 1, 2018 · 1 comment
Closed

handshake timer spec is inconsistent #1816

martinduke opened this issue Oct 1, 2018 · 1 comment
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.

Comments

@martinduke
Copy link
Contributor

Section 4.3.1 of quic-recovery says:

The initial handshake timeout SHOULD be set to twice the initial RTT.

but the pseudocode says:

timeout = max(timeout + max_ack_delay, kMinTLPTimeout)

Do we include max_ack_delay or not?
@ianswett
Copy link
Contributor

ianswett commented Oct 1, 2018

My intent was to not include max_ack_delay in the handshake timeout, because "4.4.1. Crypto Handshake Data" advises using a very short or no ACK delay for handshake packets.

ianswett added a commit that referenced this issue Oct 1, 2018
@ianswett
No max_ack_delay in handshake alarm
6229a31 
Fixes #1816
@ianswett ianswett mentioned this issue Oct 1, 2018
Merged
ianswett added a commit that referenced this issue Oct 1, 2018
@ianswett
No max_ack_delay in handshake alarm (#1817)
007677b 
Fixes #1816
@mnot mnot added editorial An issue that does not affect the design of the protocol; does not require consensus. -recovery labels Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mnot @ianswett @martinduke