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

When is the "current time" #3908

Closed
martinthomson opened this issue Jul 15, 2020 · 2 comments · Fixed by #3911
Closed

When is the "current time" #3908

martinthomson opened this issue Jul 15, 2020 · 2 comments · Fixed by #3911
Assignees
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.

Comments

@martinthomson
Copy link
Member

A client could have received and acknowledged a Handshake packet, causing it to discard state for the Initial packet number space, but not sent any ack-eliciting Handshake packets. In this case, the PTO is set from the current time.

This paragraph does not really provide any context that would allow you to infer what the "current time" is. Based on the pseudocode, this time is when SetLossDetectionTimer() is invoked, but that covers a range of cases:

  • When a packet is received and the endpoint is at its anti-amplification limit
  • When an in-flight packet is sent
  • When an ACK is received
  • When the loss detection timer pops
  • When a packet number space is discarded

Of these, the "current time" clause only really applies to the last two, and the last one is the real baseline (because the other only matters if the first was set). I think that this can be more precisely worded to say "In this case, the PTO is set from the time at which the Initial packet number space is discarded." That's far more direct, but I would like others to check my work to ensure that it catches all the cases properly.

@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. -transport labels Jul 15, 2020
@larseggert larseggert added this to Triage in Late Stage Processing via automation Jul 15, 2020
@ianswett
Copy link
Contributor

Setting the time from when the Initial PN space is discarded is a good clarification, and conveniently it lines up with the pseudocode. The pseudocode for OnPacketNumberSpaceDiscarded(pn_space) calls SetLossDetectionTimer().

ianswett added a commit that referenced this issue Jul 15, 2020
This is a rephrase of @martinthomson  suggestion.

Fixes #3908
@ianswett ianswett self-assigned this Jul 15, 2020
@LPardue LPardue moved this from Triage to Editorial Issues in Late Stage Processing Jul 17, 2020
Late Stage Processing automation moved this from Editorial Issues to Issue Handled Jul 27, 2020
@ianswett
Copy link
Contributor

There are actually two cases now() is used, so we can't solely rely on handshake keys being discarded.

See #3959 for a larger change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
Late Stage Processing
  
Issue Handled
Development

Successfully merging a pull request may close this issue.

3 participants