Split PTO calculation into a dedicated method #3681
Conversation
As @martinthomson pointed out in #3666, it was far too complex and error-prone as-is. Als fixes #3564 and #3674 by calculating the PTO timeout for each PN space and picking the earliest, as well as checking that there are inflight packets before calculating PTO for a PN space.
|
FWIW, the problem with exponentially increasing max_ack_delay thing is pretty obvious here. I think that I might choose not to implement that bit :) |
Co-authored-by: Martin Thomson <mt@lowentropy.net>
MikeBishop
added
-recovery
editorial
draft-ietf-quic-recovery.md
Outdated
| max(4 * rttvar, kGranularity) * (2 ^ pto_count) | ||
| # Arm PTO from now when there are no inflight packets | ||
| if (no in-flight packets): | ||
| return (now() + duration), Initial |
There was a problem hiding this comment.
This breaks the following case: As a client, I need to send a Handshake packet to unblock the server's amplification limit, if the Initial packet number space was already dropped.
There was a problem hiding this comment.
Thanks. The amplification factor is the gift that keeps on giving.
| duration = (smoothed_rtt + max(4 * rttvar, kGranularity)) | ||
| * (2 ^ pto_count) | ||
| # Arm PTO from now when there are no inflight packets | ||
| if (no in-flight packets): |
There was a problem hiding this comment.
Assuming this is for the amplification protection, something's missing here. Perhaps this should be
if (is_client && no packets in flight && !PeerCompletedAddressValidation()).
Otherwise, you never get into the case below on line 1293.
There was a problem hiding this comment.
I added an assert that for !PeerCompletedAddressValidation().
I think the condition you're mentioning is:
"if (no in-flight packets in space):
continue;"
That's per PN-space, not for the connection as a whole.
There was a problem hiding this comment.
never mind, I thought the code below to do the early return when no packets are in flight after address validation was removed. It's still there, so all good.
Co-authored-by: Jana Iyengar <jri.ietf@gmail.com>
As @martinthomson pointed out in #3666, it was far too complex and error-prone as-is.
Fixes #3564 and #3674 by calculating the PTO timeout for each PN space and picking the earliest, as well as checking that there are inflight packets before calculating PTO for a PN space.