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

[MANAGING ACK] ACK ranges of sent ACK frames may exclude a gap #495

Closed
PremRL opened this issue Apr 23, 2024 · 5 comments
Closed

[MANAGING ACK] ACK ranges of sent ACK frames may exclude a gap #495

PremRL opened this issue Apr 23, 2024 · 5 comments
Labels
invalid This doesn't seem right

Comments

@PremRL
Copy link

PremRL commented Apr 23, 2024

I have been using this application for quite some time now, and I want to thank you for developing and maintaining this.

In the test environment, which experiences some packet loss, I encountered a situation where an "aioquic" receiver does not identify certain gaps, as demonstrated below.

PKN Client PKN aioquic server
0 stm#0-1000 -> RECEIVED
1 stm#1000-2000 -> RECEIVED
2 stm#2000-3000 -> LOST
3 stm#3000-4000 -> RECEIVED
10 ACK (largest=3, 1st_range=0, gap#0=0, range#0=1) -> RECEIVED
4 stm#4000-5000 -> LOST
6 stm#2000-3000 -> RECEIVED
7 stm#6000-7000 -> RECEIVED
8 stm#7000-8000 -> LOST
11 MAXDATA -> RECEIVED
9 ACK (largest=11, ...) -> RECEIVED
12 ACK (largest=9, 1st_range=0, gap#0=0, range#0=1) -> RECEIVED

In this example, ACK frame (number#12), sent to the client, does not identify packet number#4 as a gap in its ACK ranges after receiving ACK#9. Upon reviewing the code in aioquic/src/aioquic/quic/connection.py, I discovered that the "highest_acked + 1" removes an ACK range entirely, and because of this, it cannot represent a gap range for the subsequent numbers if they are missing.

I tried "space.ack_queue.subtract(0, highest_acked)", and it worked quite well. Therefore, I think it would be an improvement to implement this change in aioquic.

Thank you.

@rthalley
Copy link
Contributor

rthalley commented May 5, 2024

We'll take a look in more detail; I agree there is a problem but I'm not sure about your proposed fix.

@rthalley rthalley added the bug Something isn't working label May 5, 2024
jlaine added a commit to jlaine/aioquic that referenced this issue Jun 19, 2024
When a packet containing an ACK is acknowledged, we are allowed to
discard ACK ranges up to and including the highest acknowledged packet
in the sent ACK:

https://datatracker.ietf.org/doc/html/rfc9000#ack-tracking

However, doing so prevents us from reporting a gap if the immediately
following packet(s) are lost. We therefore retain at least the highest
acknowledged packet in our ACK ranges.

Fixes: aiortc#495.
@jlaine
Copy link
Contributor

jlaine commented Jun 19, 2024

I had a look into this, and AFAICT, the current aioquic behaviour is compliant with:

https://datatracker.ietf.org/doc/html/rfc9000#ack-tracking

However I agree with that you have a valid scenario here: when our ACK is acknowledged, we end up clearing the ACK ranges, so that if the immediately following packets are lost, we are unable to report a gap. I have put together PR #514 with what I think is a minimal reproduction of your scenario.

@jlaine jlaine added invalid This doesn't seem right and removed bug Something isn't working labels Jun 20, 2024
@jlaine
Copy link
Contributor

jlaine commented Jun 20, 2024

I have thought about this some more and I am now quite convinced there is no issue here.

ACK gaps do not convey any actual meaning: they are a detail of how ACKs are encoded, making it possible to acknowledge non-contiguous ranges of packets. When a QUIC implementation receives an ACK, it's only going to look at what packets are acknowledged by the ACK, not the gaps in the ACK.

In the example you give, no information is lost by not sending a "gap" for packet 4:

  • The client knows from the ACK#10 that packets 0, 1, 3 have been received.
  • The client also knows from ACK#11 that packets 6, 7, 9 have been received.
  • It therefore has the full picture, packets 2, 4, 8 (nor 5, not sure why it's not in your example) were not ACK'd.

A final thought: the case where all ACK ranges are dropped is not so "unique". This is identical to the initial situation before any packets have been received. For instance, if the first packets for a packet space are lost, there will be no "gap" in the ACK.

@PremRL
Copy link
Author

PremRL commented Jun 20, 2024

Thank you for your explanation. I now agree that it is possible to recover the lost packets.

I appreciate your effort in this.

@jlaine
Copy link
Contributor

jlaine commented Jun 20, 2024

With pleasure, it was a fair question :)

@jlaine jlaine closed this as completed Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants