Page MenuHomeFreeBSD

tcp: remove TCP implied connect
Changes PlannedPublic

Authored by glebius on Mar 14 2024, 6:49 PM.
Tags
None
Referenced Files
F102177322: D44361.diff
Fri, Nov 8, 1:42 PM
Unknown Object (File)
Sep 15 2024, 5:41 PM
Unknown Object (File)
Sep 5 2024, 8:06 PM
Unknown Object (File)
Jul 8 2024, 12:20 AM
Unknown Object (File)
Jul 7 2024, 10:31 PM
Unknown Object (File)
Jun 28 2024, 6:23 PM
Unknown Object (File)
May 17 2024, 1:46 AM
Unknown Object (File)
May 4 2024, 1:22 PM

Details

Reviewers
None
Group Reviewers
transport
Summary

The TCP implied connect was a secret feature of FreeBSD. One could use
sendto(2) instead of connect(2) followed by send(2) on a newly created TCP
socket. It was added together with T/TCP in a0292f237572, however it is
not a part of T/TCP specification. When T/TCP was removed in c94c54e4df9a
the implied connect stayed.

It is not documented anywhere. The fact that no software had picked it up
during these 29 years justifies its removal. Other fun fact is that the
test case I added in 861274c9f8cf runs two orders of magnitude slowlier
than normal connect(2)+send(2). I didn't analyze why, though.

Andre Oppermann proposed implied connect for removal earlier in 2010:

https://lists.freebsd.org/pipermail/freebsd-net/2010-August/026311.html

The arguments brought against back then aren't valid today. Argument
brought by Robert Watson and Michael Tüxen is concern about other
protocols. In reality the only other protocol is unix/dgram, as
unix/stream does not support implied connect. PF_UNIX/SOCK_STREAM might
have supported implied connect a long time ago, but not on FreeBSD 5.3 or
newer. The unix/dgram though indeed supports sendto(2), which acts as an
implied connect for duration of a syscall, but this doesn't matter since
unix/dgram implementation is now isolated and has nothing to do with TCP.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 56610
Build 53498: arc lint + arc unit

Event Timeline

Question: Is TCP fast open still working? I think you just broke the client side...
Remark: SCTP supports this for 1-to-1 and 1-to-many style sockets.

Yep. The new fast open sits in the same clause. I'd probably start with unit tests for it.

I think having the ability to use sendto() also on a TCP socket without fast open is a nice feature. It allows user programs which want to enable/disable fast open via the command line to be written like

fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (enable_fastopen)
    setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &on, sizeof(on));
sendto(fd, req, req_len, addr, addr_len);

instead of

fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (enable_fastopen) {
    setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &on, sizeof(on));
    sendto(fd, req, req_len, 0, addr, addr_len);
} else {
    if (connect(fd, addr, addr_len) == 0)
        send(fd, req, req_len, 0);
}

Do we gain anything by removing the support?

I think having the ability to use sendto() also on a TCP socket without fast open is a nice feature. It allows user programs which want to enable/disable fast open via the command line to be written like

fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (enable_fastopen)
    setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &on, sizeof(on));
sendto(fd, req, req_len, addr, addr_len);

instead of

fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (enable_fastopen) {
    setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &on, sizeof(on));
    sendto(fd, req, req_len, 0, addr, addr_len);
} else {
    if (connect(fd, addr, addr_len) == 0)
        send(fd, req, req_len, 0);
}

Do we gain anything by removing the support?

The fact is that in the world there is no code written like the former. All code is written is like the latter. Through 29 years nobody did that. Correct me if I am wrong here. I don't think we should collect unused but otherwise nice code.

We are gaining removal of PR_IMPLOPCL that would remove several if clauses from sosend_generic(). Some code simplification of course. And a couple of instructions. But for almost 30 years thousands of computers have executed those these instructions billion times for what? For nothing.

I'd actually prefer to put question other way: do we gain anything keeping the support?

I think having the ability to use sendto() also on a TCP socket without fast open is a nice feature. It allows user programs which want to enable/disable fast open via the command line to be written like

fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (enable_fastopen)
    setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &on, sizeof(on));
sendto(fd, req, req_len, addr, addr_len);

instead of

fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (enable_fastopen) {
    setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &on, sizeof(on));
    sendto(fd, req, req_len, 0, addr, addr_len);
} else {
    if (connect(fd, addr, addr_len) == 0)
        send(fd, req, req_len, 0);
}

Do we gain anything by removing the support?

The fact is that in the world there is no code written like the former. All code is written is like the latter. Through 29 years nobody did that. Correct me if I am wrong here. I don't think we should collect unused but otherwise nice code.

I don't know if it is used or not. What you could gain is that you can use a blocking socket and get the data on the third segment of the handshake. So you can safe the sending of one packet.
To double check, I tested this and it doesn't work as expected:

  • With TCP fast open enabled, it works as expected. On the first handshake you get the request on the ACK.
  • With TCP fast open not enabled, the data gets on the SYN, is dropped by the peer, and retransmitted. This explains why it was slower in your testing. Will figure out why it is broken tomorrow.

We are gaining removal of PR_IMPLOPCL that would remove several if clauses from sosend_generic(). Some code simplification of course. And a couple of instructions. But for almost 30 years thousands of computers have executed those these instructions billion times for what? For nothing.

So you will remove TCP fast open support for the client side?

I'd actually prefer to put question other way: do we gain anything keeping the support?

I'm not requesting to keep it. I was asking what we would gain. I did not know that it needs additional cycles in the socket layer. SCTP supports this without using PR_IMPLOPCL...

Well, once I fully decompose sosend_generic() it would be isolated to TCP and won't need PR_IMPLOPCLOSE, too. So you may forget my argument about generic socket code.

Let me ask: what would we gain if we keep it? Extra time spent by us or somebody else in 2040 - 2050 to discover that it isn't used by anyone but is still there? Probably broken again in some weird fashion. btw, thanks a lot for investigation how it works on the TCP level and why it is so slow.