[PATCH net] tcp: initialize send sequence numbers before deferring TFO

[PATCH net] tcp: initialize send sequence numbers before deferring TFO

[Qingfang Deng]

TCP_FASTOPEN_CONNECT can return from connect() before tcp_connect_init()
when a cached cookie or cookieless Fast Open allows deferring the SYN.
At this point write_seq contains the initial sequence number, while
snd_una and snd_nxt have not been initialized to that sequence number.

Consequently, inet_diag and /proc/net/tcp{,6} report the initial
sequence number as the send queue length despite no SYN or payload being
queued.

Initialize snd_una and snd_nxt to write_seq before marking the
connection as deferred. Both sequence-based queue lengths then remain
zero until the first write starts the connection and runs
tcp_connect_init().

Fixes: 19f6d3f3c842 ("net/tcp-fastopen: Add new API support")
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
Reproducer:

#define _GNU_SOURCE
#include <netinet/in.h>
#include <netinet/tcp.h>
#include <stdio.h>
#include <sys/socket.h>
#include <unistd.h>

int main(void)
{
	struct sockaddr_in dst = {
		.sin_family = AF_INET,
		.sin_port = htons(9),
		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
	};
	struct tcp_info info = {0};
	socklen_t len = sizeof(info);
	int fd = socket(AF_INET, SOCK_STREAM, 0), one = 1;

	if (fd < 0 ||
	    setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN_CONNECT, &one, sizeof(one)) < 0 ||
	    setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN_NO_COOKIE, &one, sizeof(one)) < 0 ||
	    connect(fd, (struct sockaddr *)&dst, sizeof(dst)) < 0 ||
	    getsockopt(fd, IPPROTO_TCP, TCP_INFO, &info, &len) < 0) {
		perror("TFO setup");
		return 1;
	}
	if (info.tcpi_state != TCP_SYN_SENT) {
		fprintf(stderr, "Expected SYN_SENT, got state %u\n", info.tcpi_state);
		return 1;
	}
	printf("PID %ld: deferred SYN_SENT; no SYN or payload sent\n", (long)getpid());
	fflush(stdout);
	for (;;)
		pause();
}

`ss -ntp state syn-sent` will show a bogus Send-Q number, which is the
initial sequence number interpreted as signed 32-bit:

Recv-Q          Send-Q                        Local Address:Port                      Peer Address:Port          Process
0               -1405690727                       127.0.0.1:57290                        127.0.0.1:9              users:(("a.out",pid=63363,fd=3))

Diagnosis and implementation were assisted by GPT Codex.
---
 net/ipv4/tcp_fastopen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 6dfe40322fb5..ff81e663ac61 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -545,6 +545,8 @@ bool tcp_fastopen_defer_connect(struct sock *sk, int *err)
 
 	if (tp->fastopen_connect && !tp->fastopen_req) {
 		if (tcp_fastopen_cookie_check(sk, &mss, &cookie)) {
+			WRITE_ONCE(tp->snd_una, tp->write_seq);
+			WRITE_ONCE(tp->snd_nxt, tp->write_seq);
 			inet_set_bit(DEFER_CONNECT, sk);
 			return true;
 		}
-- 
2.43.0

Re: [PATCH net] tcp: initialize send sequence numbers before deferring TFO

[Jiayuan Chen]

On 11/2/26 11:59 AM, Qingfang Deng wrote:
> TCP_FASTOPEN_CONNECT can return from connect() before tcp_connect_init()
> when a cached cookie or cookieless Fast Open allows deferring the SYN.
> At this point write_seq contains the initial sequence number, while
> snd_una and snd_nxt have not been initialized to that sequence number.
>
> Consequently, inet_diag and /proc/net/tcp{,6} report the initial
> sequence number as the send queue length despite no SYN or payload being
> queued.
>
> Initialize snd_una and snd_nxt to write_seq before marking the
> connection as deferred. Both sequence-based queue lengths then remain
> zero until the first write starts the connection and runs
> tcp_connect_init().


This patch looks good to me. But idiag_wqueue can still go negative
on the normal connect path, since the socket is already hashed and
diag does not take the socket lock. We always set write_seq first,
then init snd_una later:

  tcp_v4_connect
    WRITE_ONCE(tp->write_seq, st.seq)
    tcp_connect
      tcp_connect_init
        WRITE_ONCE(tp->snd_una, tp->write_seq)

Can you try this instead?

  r->idiag_wqueue = READ_ONCE(sk->sk_wmem_queued) ?
                    READ_ONCE(tp->write_seq) - READ_ONCE(tp->snd_una) : 0;

Re: [PATCH net] tcp: initialize send sequence numbers before deferring TFO

[Qingfang Deng]

On 2026/11/2 14:11, Jiayuan Chen wrote:
> On 11/2/26 11:59 AM, Qingfang Deng wrote:
>> TCP_FASTOPEN_CONNECT can return from connect() before tcp_connect_init()
>> when a cached cookie or cookieless Fast Open allows deferring the SYN.
>> At this point write_seq contains the initial sequence number, while
>> snd_una and snd_nxt have not been initialized to that sequence number.
>>
>> Consequently, inet_diag and /proc/net/tcp{,6} report the initial
>> sequence number as the send queue length despite no SYN or payload being
>> queued.
>>
>> Initialize snd_una and snd_nxt to write_seq before marking the
>> connection as deferred. Both sequence-based queue lengths then remain
>> zero until the first write starts the connection and runs
>> tcp_connect_init().
>
> This patch looks good to me. But idiag_wqueue can still go negative
> on the normal connect path, since the socket is already hashed and
> diag does not take the socket lock. We always set write_seq first,
> then init snd_una later:
>
>    tcp_v4_connect
>      WRITE_ONCE(tp->write_seq, st.seq)
>      tcp_connect
>        tcp_connect_init
>          WRITE_ONCE(tp->snd_una, tp->write_seq)
>
> Can you try this instead?
>
>    r->idiag_wqueue = READ_ONCE(sk->sk_wmem_queued) ?
>                      READ_ONCE(tp->write_seq) - READ_ONCE(tp->snd_una) : 0;

This does not guarantee the load order on a weakly-ordered CPU. LLM 
suggests using this load-acquire/store-release sequence:

r->idiag_wqueue = smp_load_acquire(&sk->sk_wmem_queued) ? 
READ_ONCE(tp->write_seq) - READ_ONCE(tp->snd_una) : 0;

Then in tcp_connect_queue_skb(), replace sk_wmem_queued_add() 
with smp_store_release():

/* Paired with smp_load_acquire() in tcp_diag_get_info(). */
smp_store_release(&sk->sk_wmem_queued, sk->sk_wmem_queued + skb->truesize);

What do you think?

Re: [PATCH net] tcp: initialize send sequence numbers before deferring TFO

[Eric Dumazet]

Le lun. 2 nov. 2026 à 08:41, Qingfang Deng <qingfang.deng@linux.dev> a écrit :
>
> On 2026/11/2 14:11, Jiayuan Chen wrote:
> > On 11/2/26 11:59 AM, Qingfang Deng wrote:
> >> TCP_FASTOPEN_CONNECT can return from connect() before tcp_connect_init()
> >> when a cached cookie or cookieless Fast Open allows deferring the SYN.
> >> At this point write_seq contains the initial sequence number, while
> >> snd_una and snd_nxt have not been initialized to that sequence number.
> >>
> >> Consequently, inet_diag and /proc/net/tcp{,6} report the initial
> >> sequence number as the send queue length despite no SYN or payload being
> >> queued.
> >>
> >> Initialize snd_una and snd_nxt to write_seq before marking the
> >> connection as deferred. Both sequence-based queue lengths then remain
> >> zero until the first write starts the connection and runs
> >> tcp_connect_init().
> >
> > This patch looks good to me. But idiag_wqueue can still go negative
> > on the normal connect path, since the socket is already hashed and
> > diag does not take the socket lock. We always set write_seq first,
> > then init snd_una later:
> >
> >    tcp_v4_connect
> >      WRITE_ONCE(tp->write_seq, st.seq)
> >      tcp_connect
> >        tcp_connect_init
> >          WRITE_ONCE(tp->snd_una, tp->write_seq)
> >
> > Can you try this instead?
> >
> >    r->idiag_wqueue = READ_ONCE(sk->sk_wmem_queued) ?
> >                      READ_ONCE(tp->write_seq) - READ_ONCE(tp->snd_una) : 0;
>
> This does not guarantee the load order on a weakly-ordered CPU. LLM
> suggests using this load-acquire/store-release sequence:
>
> r->idiag_wqueue = smp_load_acquire(&sk->sk_wmem_queued) ?
> READ_ONCE(tp->write_seq) - READ_ONCE(tp->snd_una) : 0;
>
> Then in tcp_connect_queue_skb(), replace sk_wmem_queued_add()
> with smp_store_release():
>
> /* Paired with smp_load_acquire() in tcp_diag_get_info(). */
> smp_store_release(&sk->sk_wmem_queued, sk->sk_wmem_queued + skb->truesize);
>
> What do you think?

Please do not do this.

/proc/net/tcp and inet_diag dump sockets without holding the socket
lock. They provide a best effort snapshot, and transient inconsistent
values are expected (see the comment about rx_queue in get_tcp4_sock()).

The window in tcp_v4_connect()/tcp_v6_connect() is a few usec, with the
socket lock held by the caller. Adding memory barriers in the TCP stack,
on a field updated in many places, to cover this is not worth it.

The bug you found is different: with deferred TFO, the bogus values stay
forever, until the application calls sendmsg(). Your v1 fixes this.

Note that tcpi_notsent_bytes (tcp_get_info()) is also fixed by your
patch, you might mention this in the changelog. Not a big deal if you
do not want to send a v2.

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

Re: [PATCH net] tcp: initialize send sequence numbers before deferring TFO

[Qingfang Deng]

On 2026/11/2 16:00, Eric Dumazet wrote:
> Le lun. 2 nov. 2026 à 08:41, Qingfang Deng <qingfang.deng@linux.dev> a écrit :
>> On 2026/11/2 14:11, Jiayuan Chen wrote:
>>> On 11/2/26 11:59 AM, Qingfang Deng wrote:
>>>> TCP_FASTOPEN_CONNECT can return from connect() before tcp_connect_init()
>>>> when a cached cookie or cookieless Fast Open allows deferring the SYN.
>>>> At this point write_seq contains the initial sequence number, while
>>>> snd_una and snd_nxt have not been initialized to that sequence number.
>>>>
>>>> Consequently, inet_diag and /proc/net/tcp{,6} report the initial
>>>> sequence number as the send queue length despite no SYN or payload being
>>>> queued.
>>>>
>>>> Initialize snd_una and snd_nxt to write_seq before marking the
>>>> connection as deferred. Both sequence-based queue lengths then remain
>>>> zero until the first write starts the connection and runs
>>>> tcp_connect_init().
>>> This patch looks good to me. But idiag_wqueue can still go negative
>>> on the normal connect path, since the socket is already hashed and
>>> diag does not take the socket lock. We always set write_seq first,
>>> then init snd_una later:
>>>
>>>     tcp_v4_connect
>>>       WRITE_ONCE(tp->write_seq, st.seq)
>>>       tcp_connect
>>>         tcp_connect_init
>>>           WRITE_ONCE(tp->snd_una, tp->write_seq)
>>>
>>> Can you try this instead?
>>>
>>>     r->idiag_wqueue = READ_ONCE(sk->sk_wmem_queued) ?
>>>                       READ_ONCE(tp->write_seq) - READ_ONCE(tp->snd_una) : 0;
>> This does not guarantee the load order on a weakly-ordered CPU. LLM
>> suggests using this load-acquire/store-release sequence:
>>
>> r->idiag_wqueue = smp_load_acquire(&sk->sk_wmem_queued) ?
>> READ_ONCE(tp->write_seq) - READ_ONCE(tp->snd_una) : 0;
>>
>> Then in tcp_connect_queue_skb(), replace sk_wmem_queued_add()
>> with smp_store_release():
>>
>> /* Paired with smp_load_acquire() in tcp_diag_get_info(). */
>> smp_store_release(&sk->sk_wmem_queued, sk->sk_wmem_queued + skb->truesize);
>>
>> What do you think?
> Please do not do this.
>
> /proc/net/tcp and inet_diag dump sockets without holding the socket
> lock. They provide a best effort snapshot, and transient inconsistent
> values are expected (see the comment about rx_queue in get_tcp4_sock()).
>
> The window in tcp_v4_connect()/tcp_v6_connect() is a few usec, with the
> socket lock held by the caller. Adding memory barriers in the TCP stack,
> on a field updated in many places, to cover this is not worth it.

tcp_connect_queue_skb(), as the name suggests, only queues the 
connecting SYN. So this adds a small setup cost on weakly-ordered CPUs 
(on x86, smp_store_release degrades to a compiler barrier).

If strong memory ordering is not required, do you prefer Jiayuan's approach?

>
> The bug you found is different: with deferred TFO, the bogus values stay
> forever, until the application calls sendmsg(). Your v1 fixes this.
>
> Note that tcpi_notsent_bytes (tcp_get_info()) is also fixed by your
> patch, you might mention this in the changelog. Not a big deal if you
> do not want to send a v2.
>
> Reviewed-by: Eric Dumazet <edumazet@kernel.org>

Re: [PATCH net] tcp: initialize send sequence numbers before deferring TFO

[Eric Dumazet]

Le lun. 2 nov. 2026 à 09:13, Qingfang Deng <qingfang.deng@linux.dev> a écrit :
>
> On 2026/11/2 16:00, Eric Dumazet wrote:
> > Le lun. 2 nov. 2026 à 08:41, Qingfang Deng <qingfang.deng@linux.dev> a écrit :
> >> On 2026/11/2 14:11, Jiayuan Chen wrote:
> >>> On 11/2/26 11:59 AM, Qingfang Deng wrote:
> >>>> TCP_FASTOPEN_CONNECT can return from connect() before tcp_connect_init()
> >>>> when a cached cookie or cookieless Fast Open allows deferring the SYN.
> >>>> At this point write_seq contains the initial sequence number, while
> >>>> snd_una and snd_nxt have not been initialized to that sequence number.
> >>>>
> >>>> Consequently, inet_diag and /proc/net/tcp{,6} report the initial
> >>>> sequence number as the send queue length despite no SYN or payload being
> >>>> queued.
> >>>>
> >>>> Initialize snd_una and snd_nxt to write_seq before marking the
> >>>> connection as deferred. Both sequence-based queue lengths then remain
> >>>> zero until the first write starts the connection and runs
> >>>> tcp_connect_init().
> >>> This patch looks good to me. But idiag_wqueue can still go negative
> >>> on the normal connect path, since the socket is already hashed and
> >>> diag does not take the socket lock. We always set write_seq first,
> >>> then init snd_una later:
> >>>
> >>>     tcp_v4_connect
> >>>       WRITE_ONCE(tp->write_seq, st.seq)
> >>>       tcp_connect
> >>>         tcp_connect_init
> >>>           WRITE_ONCE(tp->snd_una, tp->write_seq)
> >>>
> >>> Can you try this instead?
> >>>
> >>>     r->idiag_wqueue = READ_ONCE(sk->sk_wmem_queued) ?
> >>>                       READ_ONCE(tp->write_seq) - READ_ONCE(tp->snd_una) : 0;
> >> This does not guarantee the load order on a weakly-ordered CPU. LLM
> >> suggests using this load-acquire/store-release sequence:
> >>
> >> r->idiag_wqueue = smp_load_acquire(&sk->sk_wmem_queued) ?
> >> READ_ONCE(tp->write_seq) - READ_ONCE(tp->snd_una) : 0;
> >>
> >> Then in tcp_connect_queue_skb(), replace sk_wmem_queued_add()
> >> with smp_store_release():
> >>
> >> /* Paired with smp_load_acquire() in tcp_diag_get_info(). */
> >> smp_store_release(&sk->sk_wmem_queued, sk->sk_wmem_queued + skb->truesize);
> >>
> >> What do you think?
> > Please do not do this.
> >
> > /proc/net/tcp and inet_diag dump sockets without holding the socket
> > lock. They provide a best effort snapshot, and transient inconsistent
> > values are expected (see the comment about rx_queue in get_tcp4_sock()).
> >
> > The window in tcp_v4_connect()/tcp_v6_connect() is a few usec, with the
> > socket lock held by the caller. Adding memory barriers in the TCP stack,
> > on a field updated in many places, to cover this is not worth it.
>
> tcp_connect_queue_skb(), as the name suggests, only queues the
> connecting SYN. So this adds a small setup cost on weakly-ordered CPUs
> (on x86, smp_store_release degrades to a compiler barrier).
>
> If strong memory ordering is not required, do you prefer Jiayuan's approach?

Neither, let's keep v1 as is.

I know tcp_connect_queue_skb() only queues the SYN. The cost is not the
problem; sk_wmem_queued has many other writers, and one release store
among them is a special ordering rule that nobody will remember the
next time this code is changed. This is too much complexity to hide a
transient value in a best-effort dump.

Without barriers, Jiayuan's version is only a heuristic: the loads are
not ordered, so it guarantees nothing. It also ties a sequence-space
value to memory accounting, and it would have to be copied into
get_tcp4_sock() and get_tcp6_sock() to be consistent.

Thanks.