Reliable reset: plumb FinalSize and enforce reset-equivalent flow-control accounting#6024
Reliable reset: plumb FinalSize and enforce reset-equivalent flow-control accounting#6024gaurav2699 wants to merge 6 commits into
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (40.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #6024 +/- ##
==========================================
+ Coverage 85.03% 85.37% +0.34%
==========================================
Files 60 60
Lines 18792 18816 +24
==========================================
+ Hits 15980 16065 +85
+ Misses 2812 2751 -61 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
guhetier
left a comment
guhetier
left a comment
There was a problem hiding this comment.
Except for the FinalSize < ReliableOffset check, I don't understand what this change tries to achieve.
| BOOLEAN CloseNow = Stream->RecvBuffer.BaseOffset >= ReliableOffset; | ||
|
|
||
| if (!Stream->Flags.RemoteCloseResetReliable && CloseNow) { | ||
| uint64_t TotalRecvLength = QuicRecvBufferGetTotalLength(&Stream->RecvBuffer); |
There was a problem hiding this comment.
Are you looking for Stream->RecvMaxLength?
| // as a result. | ||
| // | ||
| uint64_t FlowControlIncrease = FinalSize - TotalReadLength; | ||
| Stream->Connection->Send.MaxData += FlowControlIncrease; |
There was a problem hiding this comment.
I don't think we have a reason to give flow control here. More flow control will be granted as we receive bytes.
|
What issue is this PR resolving? |
3 participants
Description
Adds FinalSize to QuicStreamProcessReliableResetFrame and updates RELIABLE_RESET dispatch to pass Frame.FinalSize.
Mirrors QuicStreamProcessResetFrame accounting in reliable reset handling:
Testing
Do any existing tests cover this change? Are new tests needed?
Documentation
Is there any documentation impact for this change?