Do not always enable DSCP on Recv#5467
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5467 +/- ##
==========================================
- Coverage 85.05% 84.67% -0.39%
==========================================
Files 59 59
Lines 18605 18620 +15
==========================================
- Hits 15825 15766 -59
- Misses 2780 2854 +74 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
guhetier
left a comment
There was a problem hiding this comment.
Two high level comments:
-
should we consider a "common" sub-struct to the CX_PLATFORM to avoid duplicating this code for all platforms?
-
I am not a fan of storing this setting in the
CX_PLATFORMin the first place. It isn't a property of the platform. It would be better as a parameter provided toDatapathInitialize, and stored with most other global settings in the library object.
yes I agree, storing this in the library object would be the way to go. I had an offline meeting with @anrossi and he wanted it in CX_PLATFORM but I forgot the reasoning. @anrossi what was the reason again? |
* work-around to add recv dscp * posix * wip * linux packages?c * posix * proper init * respond to feedback * more nit feedback, remove hard-coded TRUE * comments * fix build * fix winkernel build * wip * wip * address comments
* Do not always enable DSCP on Recv (#5467) * work-around to add recv dscp * posix * wip * linux packages?c * posix * proper init * respond to feedback * more nit feedback, remove hard-coded TRUE * comments * fix build * fix winkernel build * wip * wip * address comments * remove comment
* work-around to add recv dscp * posix * wip * linux packages?c * posix * proper init * respond to feedback * more nit feedback, remove hard-coded TRUE * comments * fix build * fix winkernel build * wip * wip * address comments
Description
DSCP on Recv, when enabled, seriously regresses performance.
At the same time, we cannot just delete the code.
So let's come to a compromise and only enable it via a global SetParam.
Most users of MsQuic SHOULD NOT set this on. But we set it on in our CI so our test code can exercise DSCP.
Testing
CI
Documentation
No