Re: [PATCH net] tcp: reset late connection after listening socket close

["Asbjørn Sloth Tønnesen" <ast@fiberby.net>]

Thank you for your review.

On 8/7/26 9:05 PM, Kuniyuki Iwashima wrote:
> On Fri, Aug 7, 2026 at 12:47 PM Asbjørn Sloth Tønnesen <ast@fiberby.net> wrote:
>> In commit c82199061009 ("task_work: remove fifo ordering guarantee")
>> Eric removed the ordering guarantee, thereby changing it from a
>> guaranteed FIFO to currently LIFO ordering, in an effort to reduce
>> jitter.
> 
> Could you elaborate a bit more on how c82199061009 introduced the issue?
> 
> I didn't quite see the connection between the first and second paragraphs.

Sorry, I didn't elaborate more, it was already getting quite long.  I had also
hoped that the bisection had yielded a stronger and clearer connection, or that
this commit had been blamed before.

My understanding is that due to the LIFO (Last In First Out) ordering, the order
and thereby timing sequence is reversed. I haven't looked too deeply into the
specifics here, like which items have been scheduled together etc., as I don't
suggest reverting the blamed commit.  I have rather focused on fixing this TCP
quirk, and make it work with LIFO ordering, and not restoring the FIFO ordering.
I have mainly used netstat counters to debug the path taken by the affected
connections, and identify where and why the connections died.

I was also skeptical of the bisection result.  Did my reproducer have a false
negative?  However by reverting the commit, and thereby re-adding the order
reversal logic I have reliably proved that the blamed commit is deeply connected
to the experienced symptom, on everything I have tested post-v4.3 (the revert has
a trivial conflict on v5.10, but can then cleanly be cherry-picked on the current
net tree).  I have tested with my reproducer on v4.3, v4.4, v5.10.y and net, both
that my reproducer detects the issue in the tagged release, and that the issue is
gone with the revert.  I tested v5.10.y as the original report was against v5.10.


> The listener's state could change at any time on SMP.

True, but it won't schedule work items, and execute them in reverse order, which
IMHO makes this race condition a lot easier to hit with LIFO ordering.


>> This allows for a TCP handshake to complete, after the listening socket
>> has been closed, and after inet_csk_listen_stop() has been run.
>>
>> In that case the client sees the connection as ESTABLISHED, however in
>> tcp_v{4,6}_syn_recv_sock() the call to __inet_inherit_port() returns
>> -ENOENT, and the new connection is dropped silently by put_and_exit.
> 
> Note that if __inet_inherit_port() passes, inet_csk_reqsk_queue_add()
> calls inet_child_forget().

__inet_inherit_port() consistently exits early with -ENOENT, because
inet_csk(sk)->icsk_bind_hash is NULL for these ill timed connections.

inet_csk(sk)->icsk_bind_hash is NULL, because inet_put_port() has been
called.

inet_put_port() has been called by tcp_set_state(TCP_CLOSE).