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.

Sign up for GitHub

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

Jump to bottom

A gap range is missing #4304

Open
PremRL opened this issue May 16, 2024 · 2 comments
Open

A gap range is missing #4304

PremRL opened this issue May 16, 2024 · 2 comments
Labels
external Proposed by non-MSFT feature request A request for new functionality
Milestone
Future

Comments

@PremRL
Copy link

PremRL commented May 16, 2024 •
edited

Describe the feature you'd like supported

An msquic peer may response an ACK without a certain gap in the situation where it experiences some packet loss.

Proposed solution

The example below shows what I think is happening with the current situation.

PKN Client PKN msquic 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

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. To tackle this, I have modified a source file in the core in this directory msquic/blob/main/src/core/ack_tracker.c.

//
// Drop all packet numbers less than or equal to the largest acknowledged
// packet number.
//
QuicRangeSetMin(
    &Tracker->PacketNumbersToAck,
    LargestAckedPacketNumber + 1);

In the code, the "LargestAckedPacketNumber + 1" removes an ACK range entirely, and because of this, it cannot represent a gap range for the subsequent numbers if they are missing. So, I have changed it to dropping all packet numbers that are less than the largest acknowledged packet number, which is from "LargestAckedPacketNumber + 1" to "LargestAckedPacketNumber".

Additional context

No response
@PremRL PremRL added the feature request A request for new functionality label May 16, 2024
@nibanks nibanks added this to the Future milestone May 16, 2024
@nibanks nibanks added the external Proposed by non-MSFT label May 16, 2024
@nibanks
Copy link
Member

nibanks commented May 16, 2024

The code you reference is when we get an acknowledgment for an ACK frame we sent. Th expectation is the peer knows our up to date state for all packets up to the largest packet number we sent in that ACK frame. So, then we can 'forget' all packets ranges before this number. This is not perfect, as this comment right below states, because it's theoretically possible if some of the packets less than this were significantly delayed, but this should be super rare, and we might already have sent an ACK for them anyways.

But I don't think that's the scenario you're describing anyways. So, I'm confused about what the problem really is. In your table above, you seem to be indicating the MsQuic server is only acknowledging packet 3, and not packets 0 & 1 too? Is that a correct understanding? Why wouldn't it acknowledge everything it had received? So, I'd appreciate additional information so I may better understand your problem. Thanks!

@PremRL
Copy link
Author

PremRL commented May 16, 2024

You're right that's not what I was describing.

Sorry for the confusion. I'll try to be clearer.
So, the packet number 10 sent from the MsQuic server is acknowledging packet3 from the largest acknowledgement and packet0-1 from the ack range, which is correct. The point that I want you to focus is the packet#9.

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

As the MsQuic server is receiving an ACK#9 that is also acknowledging the ACK packet number 10, I believe it deletes packet numbers up to 4 so in the ACK table, the packet numbers#0,1,3 are removed. This is because the ACK packet number 10 contains the largest acknowledgement number of 3, and in the code, it drops up to "LargestAckedPacketNumber + 1" so that's 4.

At this point, the ACK packet number 12 sent by the MsQuic server only includes the largest acknowledgement of 9, a gap at 8, an ACK range of 5-7. Therefore, the packer number 4 sent by the other endpoint is not indicated as acknowledged or lost by the MsQuic server.

What I suggest is that we should only drop the packet numbers in the ACK table up to "LargestAckedPacketNumber" where it would be 3 in my example. So, when the MsQuic server receives the ACK packet number 9, the number#0 and 1 are removed as usual but the packet number#3 remains in the ACK table. By doing this, the ACK packet number 12 can include the largest acknowledgement of 9, a gap at 8, an ACK range of 5-7, a gap of 4, and an ACK range of 3.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
external Proposed by non-MSFT feature request A request for new functionality
Projects
None yet
Milestone
Future
Development

No branches or pull requests
2 participants
@nibanks @PremRL