Add 'cancel on loss' send mode to MsQuicStream. #4037

sebastianpick · 2024-01-05T14:51:09Z

Description

This commit adds an implementation of the stream 'cancel of loss' feature as requested per #3984.

The feature is enabled on a stream when the 'QUIC_SEND_FLAG_CANCEL_ON_LOSS' send flag is supplied during a send. From that time on, the stream will never change back from this behavior again.

Testing

Yes, a test 'CancelOnLossSend' has been added for this.

Documentation

Yes, the documentation in Streams.md and StreamSend.md has been extended.

src/test/bin/quic_gtest.cpp

src/test/lib/DataTest.cpp

nibanks · 2024-01-05T15:05:32Z

src/inc/msquic.h

@@ -240,13 +240,14 @@ typedef enum QUIC_SEND_FLAGS {
    QUIC_SEND_FLAG_FIN                      = 0x0004,   // Indicates the request is the one last sent on the stream.
    QUIC_SEND_FLAG_DGRAM_PRIORITY           = 0x0008,   // Indicates the datagram is higher priority than others.
    QUIC_SEND_FLAG_DELAY_SEND               = 0x0010,   // Indicates the send should be delayed because more will be queued soon.
+    QUIC_SEND_FLAG_CANCEL_ON_LOSS           = 0x0020,   // Indicates that a stream is to be cancelled when packet loss is detected.


Going to need to update docs for this too.

src/core/stream.h

src/core/stream_send.c

src/test/MsQuicTests.h

src/test/bin/quic_gtest.cpp

src/test/bin/winkernel/control.cpp

src/test/lib/DataTest.cpp

codecov · 2024-01-05T15:19:47Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (81f0309) 87.35% compared to head (ffe5012) 86.97%.
Report is 34 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4037      +/-   ##
==========================================
- Coverage   87.35%   86.97%   -0.39%     
==========================================
  Files          56       56              
  Lines       16957    16938      -19     
==========================================
- Hits        14813    14731      -82     
- Misses       2144     2207      +63

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/test/lib/DataTest.cpp

nibanks · 2024-01-05T15:23:21Z

Looks like the .NET build failed because those headers weren't updated. Please run .\scripts\generate-dotnet.ps1.

nibanks · 2024-01-05T15:23:53Z

Kernel mode build failures:

  D:\a\msquic\msquic\src\test\bin\winkernel\control.cpp(515,1): error C2338: static_assert failed: 'QUIC_IOCTL_BUFFER_SIZES must be kept in sync with the IOCTLs' [D:\a\msquic\msquic\src\test\bin\winkernel\msquictest.kernel.vcxproj]
  D:\a\msquic\msquic\src\test\bin\winkernel\control.cpp(1424,9): error C2039: 'DropPackets': is not a member of 'QUIC_IOCTL_PARAMS' [D:\a\msquic\msquic\src\test\bin\winkernel\msquictest.kernel.vcxproj]

nibanks · 2024-01-08T12:41:34Z

Kernel build error:

"D:\a\msquic\msquic\msquic.kernel.sln" (default target) (1) ->
"D:\a\msquic\msquic\src\test\bin\winkernel\msquictest.kernel.vcxproj" (default target) (7) ->
(Link target) -> 
  testlib.lib(DataTest.obj) : error LNK2001: unresolved external symbol "void * __cdecl operator new(unsigned __int64)" (??2@YAPEAX_K@Z) [D:\a\msquic\msquic\src\test\bin\winkernel\msquictest.kernel.vcxproj]
  D:\a\msquic\msquic\artifacts\bin\winkernel\x64_Debug_schannel\msquictest.sys : fatal error LNK1120: 1 unresolved externals [D:\a\msquic\msquic\src\test\bin\winkernel\msquictest.kernel.vcxproj]

Looks like you're not using an std::nothrow new.

src/test/lib/DataTest.cpp

sebastianpick added 5 commits January 5, 2024 14:35

Implement stream 'cancel on loss' behavior.

446ae59

Started implementing 'cancel on loss' test.

9e883ea

Finished initial test implementation.

08ad497

Cleaned up test.

42c2089

Add Kernel test mode integration.

11b4cbb

sebastianpick requested a review from a team as a code owner January 5, 2024 14:51

sebastianpick commented Jan 5, 2024

View reviewed changes

src/test/bin/quic_gtest.cpp Show resolved Hide resolved

sebastianpick commented Jan 5, 2024

View reviewed changes

src/test/lib/DataTest.cpp Outdated Show resolved Hide resolved