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

Include note about loss detection update #306

Open
michael-eriksson opened this issue Sep 26, 2024 · 8 comments
Open

Include note about loss detection update #306

michael-eriksson opened this issue Sep 26, 2024 · 8 comments
Labels

Comments

@michael-eriksson
Copy link

The example loss detection algorithm in Appendix A of RFC 9002 assumes that packets that arrives out of order are always acknowledged immediately. Because of this, it does not include max_ack_delay when the loss delay threshold is computed.

Setting the Ack-Eliciting Threshold field in the ACK_FREQUENCY frame to something else than 1 implies that some packets that arrive out of order will have delayed acknowledgments. Hence, the max_ack_delay should be added to the loss_delay variable in the algorithm above in those cases.

It would probably be valuable for implementers if this information was included in the document.

See also the quicdev Slack thread: https://quicdev.slack.com/archives/CTDAH4H71/p1725960809852329

@mirjak
Copy link
Contributor

mirjak commented Jan 27, 2025

Not really sure this is needed. Unfortunately I can't see anymore if there was a discuss on slack. However, loss_delay is defined like this:

loss_delay = kTimeThreshold * max(latest_rtt, smoothed_rtt)

where kTimeThreshold > 1. This should be large enough even if max_ack_delay is not added.

@ianswett
Copy link
Collaborator

ianswett commented Feb 6, 2025

I don't think there was ever a consideration to include max_ack_delay, partially because max_ack_delay can be quite larger compared to RTT.

I'm also having a very difficult time coming up with a case we'd want to delay loss detection on the sender by adding max_ack_delay. In cases when the ACK that indicates the missing packet is delayed by max_ack_delay, the use of max(latest_rtt, smoothed_rtt) would ensure that it isn't immediately declared lost.

@michael-eriksson
Copy link
Author

OK, a scenario.

The sender has configured the Requested Max Ack Delay to 1/4 of the RTT. Packets 100 and 101 are sent directly after each other but reordered, so 101 is received before 100. When 101 arrives, an ack is sent. 100 arrives shortly after, but no ack is sent because of the Ack Frequency configuration (Reordering Threshold is larger than 1). After 1/4 of the RTT, the ack delay timer expires and the ack for packet 100 is finally sent.

With the default calculation of loss_delay, where max_ack_delay is not included, packet 100 will be (falsely) declared as lost at the sender after 9/8 = 1.125 (kTimeThreshold) of the RTT, which is before the ack will arrive at around 5/4 = 1.25 of the RTT.

@mirjak
Copy link
Contributor

mirjak commented Feb 10, 2025

packet 100 will trigger an immediate ACK because it is smaller than 101 as specified in sec 13.2.1 in RFC9000.

@michael-eriksson
Copy link
Author

Huh? What's the point with Reordering Threshold then?

@mirjak
Copy link
Contributor

mirjak commented Feb 10, 2025

you delay the ack if you have a gap but if you receive a packet that fills a gap it still makes sense to tell the other end immediately to avoid spurious retransmissions. I guess we could optimise this further and e.g. only send a ACK if the packet fills a gap and a larger PN has already been ack'ed otherwise you wait for the usually ACK delay. Not sure if that makes a big difference but it would probably correct to do it that way.

@michael-eriksson
Copy link
Author

Quoting the draft:

After an ACK_FREQUENCY frame with a Reordering Threshold value other than 1 has been received, this extension delays immediate acknowledgements to reordered ack-eliciting packets and the gaps they can create.

@mirjak
Copy link
Contributor

mirjak commented Feb 10, 2025

Yes, I saw this but I would interpreted this as delaying the ACK but still send an immediate ACK if the re-ordered packet is received after the ack delay or re-ordering threshold, or, to say it in other words, if a higher PN packet already triggered an ACK. However, this part seems actually not very clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants