From: Eric Dumazet <eric.dumazet@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>, Eric Dumazet <eric.dumazet@gmail.com>
Cc: willemb@google.com, netdev@vger.kernel.org
Subject: Re: [RFC net] net: stream: don't purge sk_error_queue without holding its lock
Date: Tue, 14 Sep 2021 10:55:49 -0700 [thread overview]
Message-ID: <8ce5b709-17bb-ea01-48b4-b80447fb5d3f@gmail.com> (raw)
In-Reply-To: <20210914095621.5fa08637@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On 9/14/21 9:56 AM, Jakub Kicinski wrote:
> Right, but then inet_sock_destruct() also purges the err queue, again.
> I was afraid of regressions but we could just remove the purging
> from sk_stream_kill_queues(), and target net-next?
>
Yes, this would be the safest thing.
>> If you think there is a bug, it must be fixed in another way.
>>
>> IMO, preventing err packets from a prior session being queued after a tcp_disconnect()
>> is rather hard. We should not even try (packets could be stuck for hours in a qdisc)
>
> Indeed, we could rearrange the SOCK_DEAD check in sock_queue_err_skb()
> to skip queuing and put it under the err queue lock (provided we make
> sk_stream_kill_queues() take that lock as well). But seems like an
> overkill. I'd lean towards the existing patch or removing the purge from
> sk_stream_kill_queues(). LMK what you prefer, this is not urgent.
>
The issue would really about the tcp_disconnect() case,
followed by a reuse of the socket to establish another session.
In order to prevent polluting sk_error_queue with notifications
triggered by old packets (from prior flow), this would require
to record the socket cookie in skb, or something like that :/
next prev parent reply other threads:[~2021-09-14 17:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-13 22:38 [RFC net] net: stream: don't purge sk_error_queue without holding its lock Jakub Kicinski
2021-09-14 5:14 ` Eric Dumazet
2021-09-14 14:18 ` Jakub Kicinski
2021-09-14 16:32 ` Eric Dumazet
2021-09-14 16:56 ` Jakub Kicinski
2021-09-14 17:55 ` Eric Dumazet [this message]
2021-09-14 18:03 ` Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8ce5b709-17bb-ea01-48b4-b80447fb5d3f@gmail.com \
--to=eric.dumazet@gmail.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=willemb@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.