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

[Qingfang Deng <qingfang.deng@linux.dev>]

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>