-
Notifications
You must be signed in to change notification settings - Fork 537
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
Expose TTL (on handshake) to applications #4602
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4602 +/- ##
==========================================
- Coverage 87.06% 86.58% -0.49%
==========================================
Files 56 56
Lines 17354 17358 +4
==========================================
- Hits 15109 15029 -80
- Misses 2245 2329 +84 ☔ View full report in Codecov by Sentry. |
char Data[CMSG_SPACE(sizeof(struct in6_pktinfo)) + // IP_PKTINFO | ||
2 * CMSG_SPACE(sizeof(int)) // TOS | ||
+ CMSG_SPACE(sizeof(int))]; // IP_TTL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, this should have just been 3 * CMSG_SPACE(sizeof(int)
@@ -1833,6 +1895,10 @@ CxPlatSocketContextRecvComplete( | |||
|
|||
CXPLAT_FRE_ASSERT(FoundLocalAddr); | |||
CXPLAT_FRE_ASSERT(FoundTOS); | |||
// | |||
// TTL should always be available/enabled on Linux. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment not really needed
@@ -577,6 +577,9 @@ CxPlatDataPathGetSupportedFeatures( | |||
_In_ CXPLAT_DATAPATH* Datapath | |||
) | |||
{ | |||
// | |||
// Intentionally not enabling Feature_TTL on MacOS for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment not needed
@@ -150,7 +150,10 @@ RawDataPathGetSupportedFeatures( | |||
) | |||
{ | |||
UNREFERENCED_PARAMETER(Datapath); | |||
return CXPLAT_DATAPATH_FEATURE_RAW; | |||
// | |||
// TTL should always be available / enabled for XDP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment not really needed
@@ -765,6 +766,25 @@ CxPlatDataPathQuerySockoptSupport( | |||
|
|||
} while (FALSE); | |||
|
|||
do { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, a do/while loop here is overkill. You could have just had the if statement, and only added the feature flag if the status was success and build not equal to 20348.
// | ||
// TTL should always be available / enabled on Linux. | ||
// | ||
|
||
// | ||
// On Linux, IP_HOPLIMIT does not exist. So we will use IP_RECVTTL, IPV6_RECVHOPLIMIT instead. | ||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments unnecessary. We don't have it for all the others.
@@ -344,6 +345,10 @@ CxPlatDataPathCalculateFeatureSupport( | |||
} | |||
|
|||
Datapath->Features |= CXPLAT_DATAPATH_FEATURE_TCP; | |||
// | |||
// TTL should always be available / enabled on Linux. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary
// However, it is considered a "hack" and we should determine whether or not | ||
// the current release story fits this current workaround. | ||
// | ||
HMODULE NtDllHandle = LoadLibraryA("ntdll.dll"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't have this logic in two places. Perhaps, instead we update CX_PLATFORM
to store the BuildNumber
(on Windows) and put this logic in platform_winuser.c (and might as well update _winkernel.c).
Description
There was an external ask for security purposes to expose packet TTL values in QUIC.
This PR bubbles up the TTL value from the network layer up to the transport layer.
TODO: We will need to rinse and repeat these changes for ALL platforms.
Testing
Modified the handshake test to do a HandshakeHopLimitTTL > 0 check.
Modified the Datapath test to check packets have a HopLimit > 0.
Documentation
N/A