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
Change PTO to be per packet number space #3066
Changes from 1 commit
f7f22f9
c551041
3abb835
33c994e
6c3ae35
3f0da81
62e394f
8af3d28
49a2833
054fae1
2140c28
69afe42
1c9886e
a87a04c
545e788
a00c475
5e81481
ed8750a
6322d36
45bf06c
34e3d61
54e26e7
d0e6996
50f6701
355d1e5
cff00e0
7b29c66
6e902d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -524,10 +524,10 @@ removed from bytes in flight when the Initial and Handshake keys are discarded. | |
### Sending Probe Packets | ||
|
||
When a PTO timer expires, a sender MUST send at least one ack-eliciting packet | ||
in the packet number space as a probe, unless there is no data available to send. | ||
An endpoint MAY send up to two full-sized datagrams containing ack-eliciting | ||
packets, to avoid an expensive consecutive PTO expiration due to a single lost | ||
datagram. | ||
in the packet number space as a probe, unless there is no data available to | ||
send. An endpoint MAY send up to two full-sized datagrams containing | ||
ack-eliciting packets, to avoid an expensive consecutive PTO expiration due | ||
to a single lost datagram. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmation question: does this mean that we'd be permitting more datagrams to be sent in effectively one PTO, when multiple packet number spaces are involved? Consider the case where the PTO is armed for both the Handshake packet number space and the 1-RTT packet number space. Now that we have a PTO timer for each of the packet number space, we'd see both of them firing, and the text reads that an endpoint is allowed to send two datagrams for each. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I updated the PR to try to limit the number of packets sent when implementations did not coalesce. |
||
|
||
In addition to sending data in the packet number space for which the timer | ||
expired, the sender SHOULD coalesce ack-eliciting packets from all other packet | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this necessarily "all" spaces? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'all' removed, since coalescing 2 is sufficient. |
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we can drop the "containing ack-eliciting packets" here, it's redundant with the first sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's mostly redundant, but if there are two packets, this makes it clear they have to be ack-eliciting. This didn't change as part of this PR, FWIW.