Skip TCP_NODELAY on sockets that NIO doesn't explicitly support

[simonjbeaumont]

Motivation:

NIO automatically calls setsockopt with TCP_NODELAY unless the protocol family is PF_UNIX. However, since NIO offers API that allows users to create sockets out of band (withConnectedSocket(_:) and withBoundSocket(_:)), it cannot know whether the socket supports this option or not.

Modifications:

This PR inverts the filter so that it is only added for PF_INET and PF_INET6. Any other socket, e.g. PF_UNIX, or any socket that NIO doesn't explicitly handle.

Result:

TCP_NODELAY is skipped for sockets that NIO doesn't know about.

[simonjbeaumont]

Hm, these tests pass on macOS. Was a bit of a fiddle working out how we can test this. I tried playing with the SAL-backed tests but didn't get very far.

Anyone got thoughts on if and how we should test this?

[Lukasa]

This kind of "narrative style" function name is discouraged in Swift APIs, and in this particular instance I think it's hard to read. I think it'd be better to rename this to _testTCP_NODELAYDefaultValue(value:

[simonjbeaumont]

Done.

[Lukasa]

Nice, Linux doesn't support getsockopt here either. Ok, we can make the SAL work, but we should sync up so I can help you drive it.

[simonjbeaumont]

Nice, Linux doesn't support getsockopt here either.

Yep, seems like it; this is the logs from CI:

14:29:15 /code/Tests/NIOPosixTests/ChannelTests.swift:2835: error: ChannelTests.testTCP_NODELAYisOffByDefaultForUnixSockets : XCTAssertEqual threw error "getsockopt(socket:level:optionName:optionValue:optionLen:): Operation not supported (errno: 95)" -

Ok, we can make the SAL work, but we should sync up so I can help you drive it.

I took a look and I don't think that SAL is going to be an option for testing this change, in its current form.

This is because HookedSocket: Socket overrides Socket.setOption but the filtering is (and was before this PR, too) happening in BaseSocket.setOption.

At this stage I'll remove the check for the skipping of this option for unix domain sockets. The feature of skipping for unix domain sockets isn't new in this PR and there wasn't a test for it already, and it's not clear how to test it.

I can keep the update to the test that checks that we do automatically add TCP_NODELAY on AF_INET6 sockets to give us confidence I didn't regress that it should be enabled for both AF_INET and AF_INET6 sockets, and the existing test was just for AF_INET. That being said, the test will be skipped on hosts without IPv6 support anyway.