If there are no timestamp option in SYN and SYNACK packets, it seems FreeBSD now rejects the last ACK packet with timestamp as the following packetdrill script shows, but should we do that? ``` // Establish a connection. 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3 +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0 +0 fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK) = 0 +0 bind(3, ..., ...) = 0 +0 listen(3, 1) = 0 +0 < S 0:0(0) win 12336 +0 ~ +1 > S. 0:0(0) ack 1 <mss 1460> +.1 < . 1:1(0) ack 1 win 25710 <TS val 100 ecr 0,eol,eol> +0.2 accept(3, ..., ...) = 4 // f-stack/FreeBSD return -1 ```
A commit references this bug: Author: tuexen Date: Mon Nov 9 21:49:42 UTC 2020 New revision: 367530 URL: https://svnweb.freebsd.org/changeset/base/367530 Log: RFC 7323 specifies that: * TCP segments without timestamps should be dropped when support for the timestamp option has been negotiated. * TCP segments with timestamps should be processed normally if support for the timestamp option has not been negotiated. This patch enforces the above. PR: 250499 Reviewed by: gnn, rrs MFC after: 1 week Sponsored by: Netflix, Inc Differential Revision: https://reviews.freebsd.org/D27148 Changes: head/sys/netinet/tcp_input.c head/sys/netinet/tcp_stacks/bbr.c head/sys/netinet/tcp_stacks/rack.c head/sys/netinet/tcp_syncache.c head/sys/netinet/tcp_timewait.c
A commit references this bug: Author: tuexen Date: Fri Nov 20 13:00:29 UTC 2020 New revision: 367891 URL: https://svnweb.freebsd.org/changeset/base/367891 Log: Fix an issue I introuced in r367530: tcp_twcheck() can be called with to == NULL for SYN segments. So don't assume tp != NULL. Thanks to jhb@ for reporting and suggesting a fix. PR: 250499 MFC after: 1 week XMFC-with: r367530 Sponsored by: Netflix, Inc. Changes: head/sys/netinet/tcp_timewait.c
A commit references this bug: Author: tuexen Date: Mon Nov 30 09:45:45 UTC 2020 New revision: 368181 URL: https://svnweb.freebsd.org/changeset/base/368181 Log: MFC r367530: RFC 7323 specifies that: * TCP segments without timestamps should be dropped when support for the timestamp option has been negotiated. * TCP segments with timestamps should be processed normally if support for the timestamp option has not been negotiated. This patch enforces the above. Manually resolved merge conflicts. MFC 367891: Fix an issue I introuced in r367530: tcp_twcheck() can be called with to == NULL for SYN segments. So don't assume tp != NULL. Thanks to jhb@ for reporting and suggesting a fix. MFC r367946: Fix two occurences of a typo in a comment introduced in r367530. Thanks to lstewart@ for reporting them. PR: 250499 Reviewed by: gnn, rrs Sponsored by: Netflix, Inc. Differential Revision: https://reviews.freebsd.org/D27148 Changes: _U stable/12/ stable/12/sys/netinet/tcp_input.c stable/12/sys/netinet/tcp_stacks/rack.c stable/12/sys/netinet/tcp_syncache.c stable/12/sys/netinet/tcp_timewait.c
A commit references this bug: Author: tuexen Date: Mon Nov 30 10:58:07 UTC 2020 New revision: 368183 URL: https://svnweb.freebsd.org/changeset/base/368183 Log: MFC r367530: RFC 7323 specifies that: * TCP segments without timestamps should be dropped when support for the timestamp option has been negotiated. * TCP segments with timestamps should be processed normally if support for the timestamp option has not been negotiated. This patch enforces the above. Manually resolved merge conflicts. MFC 367891: Fix an issue I introuced in r367530: tcp_twcheck() can be called with to == NULL for SYN segments. So don't assume tp != NULL. Thanks to jhb@ for reporting and suggesting a fix. MFC r367946: Fix two occurences of a typo in a comment introduced in r367530. Thanks to lstewart@ for reporting them. PR: 250499 Reviewed by: gnn, rrs Sponsored by: Netflix, Inc. Differential Revision: https://reviews.freebsd.org/D27148 Changes: _U stable/11/ stable/11/sys/netinet/tcp_input.c stable/11/sys/netinet/tcp_syncache.c stable/11/sys/netinet/tcp_timewait.c
Hi, Hope this is a right place to report this. This commit appears to be causing issues with the multimedia/plexmediaserver port. One of the tasks of this software is to do video decoding, and after this patch that breaks. Sometimes its HTTP-based API fails to respond, and also there are a many connections in LAST_ACK state: $ netstat -4n|grep LAST_ACK|wc -l 415 $ netstat -4n|grep LAST_ACK|grep "127.0.0.1.32400"|wc -l 396 127.0.0.1.32400 corresponds to the Plex API endpoint. Reverting this patch fixes the issue. Original bug report is here: https://forums.plex.tv/t/transcoder-errors-on-freebsd/676180 (thanks to forum user Volts for pointing to this commit).
(In reply to Roman Bogorodskiy from comment #5) Hi Roman, I introduced a regression, which was fixed in https://cgit.FreeBSD.org/src/commit/?id=cc3c34859eab1b317d0f38731355b53f7d978c97 and MFCed to stable/12 in https://cgit.FreeBSD.org/src/commit/?id=d05d908d6d3c85479c84c707f931148439ae826b. This was affecting the handling of received TCP RST segments. Does this fix your issue? In bug #252449 it was reported that there are broken TCP stacks, which negotiate TCP timestamp support but then don't behave as required. To deal with such broken implementations I added a sysctl variable in https://cgit.FreeBSD.org/src/commit/?id=d2b3ceddccac60b563f642898e3a314647666a10 , which was MFCed to stable/12 in https://cgit.FreeBSD.org/src/commit/?id=e82353f84c58da9a5c38bd471a09936c16a5b6ea. In this case you can set the sysctl variable net.inet.tcp.tolerate_missing_ts to 1. I would be interested to now which stack is broken and it would be great if you could provide a .pcap file of the failing TCP connection. Does this fix your issue?
(In reply to Michael Tuexen from comment #6) Hi Michael, Latest HEAD appears to fix the issue for me (without any additional configurations like the sysctl you mention or anything else). I hope to test it more on a weekend. > I would be interested to now which stack is broken and it would be great if you could provide a .pcap file of the failing TCP connection. Do I need to try to catch the moment when the connection fails (that implies rolling back the fix) or any dump will suffice?
(In reply to Roman Bogorodskiy from comment #7) I everything works now without tweaking the net.inet.tcp.tolerate_missing_ts sysctl variable, I don't need anything more. Only if you need to tweak the net.inet.tcp.tolerate_missing_ts sysctl, I would be interested in a .pcap file and knowing what the involved parties are. But if you say, head is fine, I expect stable/12 and stable/13 also to be fine.