archive mirror
 help / color / mirror / Atom feed
From: Guillaume Nault <>
To: Tetsuo Handa <>
Cc: Jakub Sitnicki <>,, "David S. Miller" <>,
	Eric Dumazet <>,
	Jakub Kicinski <>, Paolo Abeni <>,
	Tom Parkin <>,,,
Subject: Re: [PATCH net] l2tp: Don't sleep and disable BH under writer-side sk_callback_lock
Date: Tue, 22 Nov 2022 15:10:11 +0100	[thread overview]
Message-ID: <20221122141011.GA3303@pc-4.home> (raw)
In-Reply-To: <>

On Tue, Nov 22, 2022 at 08:14:33PM +0900, Tetsuo Handa wrote:
> On 2022/11/22 19:46, Jakub Sitnicki wrote:
> >> is the one
> >> where changing lockdep class is concurrently done on pre-existing sockets.
> >>
> >> I think we need to always create a new socket inside l2tp_tunnel_register(),
> >> rather than trying to serialize setting of sk_user_data under sk_callback_lock.
> > 
> > While that would be easier to handle, I don't see how it can be done in
> > a backward-compatible way. User-space is allowed to pass a socket to
> > l2tp today [1].
> What is the expected usage of the socket which was passed to l2tp_tunnel_register() ?

It receives L2TP packets. Those can be either control or data ones.
Data packets are processed by the kernel. Control packets are queued to
user space.

> Is the userspace supposed to just close() that socket? Or, is the userspace allowed to
> continue using the socket?

User space uses this socket to send and receive L2TP control packets
(tunnel and session configuration, keep alive and tear down). Therefore
it absolutely needs to continue using this socket after the
registration phase.

> If the userspace might continue using the socket, we would
>   create a new socket, copy required attributes (the source and destination addresses?) from
>   the socket fetched via sockfd_lookup(), and call replace_fd() like e.g. umh_pipe_setup() does
> inside l2tp_tunnel_register(). i-node number of the socket would change, but I assume that
> the process which called l2tp_tunnel_register() is not using that i-node number.
> Since the socket is a datagram socket, I think we can copy required attributes. But since
> I'm not familiar with networking code, I don't know what attributes need to be copied. Thus,
> I leave implementing it to netdev people.

That looks fragile to me. If the problem is that setup_udp_tunnel_sock()
can sleep, we can just drop the udp_tunnel_encap_enable() call from
setup_udp_tunnel_sock(), rename it __udp_tunnel_encap_enable() and make
make udp_tunnel_encap_enable() a wrapper around it that'd also call

  reply	other threads:[~2022-11-22 14:11 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-19 13:03 [PATCH net] l2tp: Don't sleep and disable BH under writer-side sk_callback_lock Jakub Sitnicki
2022-11-19 13:52 ` Tetsuo Handa
2022-11-19 14:27   ` Tetsuo Handa
2022-11-21  9:00     ` Jakub Sitnicki
2022-11-21 10:03       ` Tetsuo Handa
2022-11-21 21:55         ` Jakub Sitnicki
2022-11-22  9:48           ` Tetsuo Handa
2022-11-22 10:46             ` Jakub Sitnicki
2022-11-22 11:14               ` Tetsuo Handa
2022-11-22 14:10                 ` Guillaume Nault [this message]
2022-11-22 14:28                   ` Tetsuo Handa
2022-11-23 15:24                     ` Guillaume Nault
2022-11-24 10:07                       ` Tom Parkin
2022-11-24 10:27                         ` Guillaume Nault
2022-11-21  9:00   ` Jakub Sitnicki

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221122141011.GA3303@pc-4.home \ \ \ \ \ \ \ \ \ \ \ \ \

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).