On Mon, 19 Oct 2026 15:25:55 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 incrementally with one additional > commit since the last revision: > > simplify checks for no-op IPV6_TCLASS src/java.base/windows/native/libnio/ch/Net.c line 406: > 404: /* No op */ > 405: return; > 406: } Can you change this to use 4-space indent and move it down to below int arglen (same thing in Java_sun_nio_ch_Net_setIntOption0 so that changing of the level/opts are together). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33425#discussion_r4316641103
