On Tue, 20 Oct 2026 10:51:38 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get a review of this change which fixes an issue when setting 
>> the `IP_TOS` socket option on Windows? This addresses 
>> https://bugs.openjdk.org/browse/JDK-8394746.
>> 
>> `libnet` in the JDK has `NET_SetSockOpt` and `NET_GetSockOpt` functions 
>> which get called when the socket option being dealt with 
>> `mayNeedConversion`. The `NET_SetSockOpt` and `NET_GetSockOpt` have 
>> different types of conversions (and fall backs) for different socket options.
>> 
>> On Windows, for `IP_TOS`, the `NET_SetSockOpt` clears the lowest bit 
>> (representing the "unused" bit) of the value, before calling the 
>> `setsockopt`. Furthermore, on some Windows editions, setting the `IP_TOS` 
>> may fail altogether irrespective of the value. This function has the ability 
>> to handle such error and return back normally. `NET_GetSockOpt` has similar 
>> abilities.
>> 
>> When an application sets a socket option through `java.net.Socket` (or 
>> similar classes like `java.net.ServerSocket`), the call is handled by 
>> `libnio` (not `libnet`) in the `Java_sun_nio_ch_Net_setIntOption0` and 
>> `Java_sun_nio_ch_Net_getIntOption0` functions. When IPv6 isn't available on 
>> a host or when the `java.net.preferIPv4Stack` is set to `true`, these 
>> functions get called with `mayNeedConversion` as false. As a result, the 
>> `Java_sun_nio_ch_Net_setIntOption0` function will not invoke the `libnet`'s 
>> `NET_SetSockOpt` function and the special handling for `IP_TOS` is skipped. 
>> As a result, since the last bit of the value no longer gets cleared in such 
>> cases, (only) on Windows 11, the `setsockopt` syscall raises an error which 
>> propagates all the way to the application.
>> 
>> The commit in this PR addresses the issue by checking if `IP_TOS` is being 
>> set and if so then it ensures that `NET_SetSockOpt` gets called to do the 
>> special handling. This matches with what's already there on the get side of 
>> this option in `Java_sun_nio_ch_Net_getIntOption0`.
>> 
>> With this change, the issue no longer reproduces on Windows 11 and the tests 
>> continue to pass even on other Windows versions.
>> 
>> Note that a similar change might be needed for *nix side too in the 
>> `Java_sun_nio_ch_Net_setIntOption0` (and get) functions. A brief look 
>> suggests that they aren't calling the `NET_SetSockOpt`/`NET_GetSockOpt` 
>> functions when IPv6 is unavailable or `java.net.preferIPv4Stack` is set to 
>> `true`. That change however will need additional experiments and I will take 
>> it up separately.
>> 
>> tier1, tier2, tier3 testing with this change completed without any related 
>> issues...
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains eight additional 
> commits since the last revision:
> 
>  - better comment
>  - merge latest from master branch
>  - minor adjustments
>  - simplify checks for no-op IPV6_TCLASS
>  - adjust line length in comments
>  - fix comment
>  - adjust comment
>  - 8394746: java/net/SocketOption/OptionsTest.java fails for IP_TOS on 
> Windows - Invalid argument: setsocketopt

Thank you Alan for the review. Tests have completed normally on all platforms 
as well as on Windows 11 where the original issue was observed. I'll go ahead 
and integrate this now.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/33425#issuecomment-6291654093

Reply via email to