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 Looks okay. At some point I think we should have ------------- Marked as reviewed by alanb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/33425#pullrequestreview-5585095593
