Re: [PATCH net] tcp: reset late connection after listening socket close
Asbjørn Sloth Tønnesen <[email protected]>
| Newsgroups | org.kernel.vger.stable,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> 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).