All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin KaFai Lau <kafai@fb.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Stanislav Fomichev <sdf@google.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org,
	Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	David Miller <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	kernel-team@fb.com, Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH bpf-next 02/14] bpf: net: Avoid sock_setsockopt() taking sk lock when called from bpf
Date: Thu, 28 Jul 2022 10:20:04 -0700	[thread overview]
Message-ID: <20220728172004.6mpkycl52sszuudc@kafai-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <20220728095629.6109f78c@kernel.org>

On Thu, Jul 28, 2022 at 09:56:29AM -0700, Jakub Kicinski wrote:
> On Thu, 28 Jul 2022 09:31:04 -0700 Martin KaFai Lau wrote:
> > If I understand the concern correctly, it may not be straight forward to
> > grip the reason behind the testings at in_bpf() [ the in_task() and
> > the current->bpf_ctx test ] ?  Yes, it is a valid point.
> > 
> > The optval.is_bpf bit can be directly traced back to the bpf_setsockopt
> > helper and should be easier to reason about.
> 
> I think we're saying the opposite thing. in_bpf() the context checking
> function is fine. There is a clear parallel to in_task() and combined
> with the capability check it should be pretty obvious what the code
> is intending to achieve.
> 
> sockptr_t::in_bpf which randomly implies that the lock is already held
> will be hard to understand for anyone not intimately familiar with the
> BPF code. Naming that bit is_locked seems much clearer.
> 
> Which is what I believe Stan was proposing.
Yeah, I think I read the 'vote against @in_bpf' in the other way. :)

Sure. I will do s/is_bpf/is_locked/ and do the in_bpf() context
checking before ns_capable().

  reply	other threads:[~2022-07-28 17:20 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-27  6:08 [PATCH bpf-next 00/14] bpf: net: Remove duplicated codes from bpf_setsockopt() Martin KaFai Lau
2022-07-27  6:09 ` [PATCH bpf-next 01/14] net: Change sock_setsockopt from taking sock ptr to sk ptr Martin KaFai Lau
2022-07-27  8:11   ` David Laight
2022-07-27 20:42     ` Martin KaFai Lau
2022-07-27  8:16   ` Eric Dumazet
2022-07-27 18:50     ` Martin KaFai Lau
2022-07-27  6:09 ` [PATCH bpf-next 02/14] bpf: net: Avoid sock_setsockopt() taking sk lock when called from bpf Martin KaFai Lau
2022-07-27  8:36   ` David Laight
2022-07-27 20:05     ` Martin KaFai Lau
2022-07-27 16:47   ` sdf
2022-07-27 18:37     ` Martin KaFai Lau
2022-07-27 20:39       ` Stanislav Fomichev
2022-07-27 21:21         ` Martin KaFai Lau
2022-07-27 21:38           ` Stanislav Fomichev
2022-07-28  0:45             ` Martin KaFai Lau
2022-07-28  1:49               ` Jakub Kicinski
2022-07-28 16:31                 ` Martin KaFai Lau
2022-07-28 16:56                   ` Jakub Kicinski
2022-07-28 17:20                     ` Martin KaFai Lau [this message]
2022-07-28 17:40                       ` Jakub Kicinski
2022-07-29 10:04                     ` David Laight
2022-07-29 19:06                       ` Martin KaFai Lau
2022-07-27  6:09 ` [PATCH bpf-next 03/14] bpf: net: Consider optval.is_bpf before capable check in sock_setsockopt() Martin KaFai Lau
2022-07-27 16:54   ` sdf
2022-07-27 18:47     ` Martin KaFai Lau
2022-07-27  6:09 ` [PATCH bpf-next 04/14] bpf: net: Avoid do_tcp_setsockopt() taking sk lock when called from bpf Martin KaFai Lau
2022-07-27  6:09 ` [PATCH bpf-next 05/14] bpf: net: Avoid do_ip_setsockopt() " Martin KaFai Lau
2022-07-27  6:09 ` [PATCH bpf-next 06/14] bpf: net: Avoid do_ipv6_setsockopt() " Martin KaFai Lau
2022-07-27  6:09 ` [PATCH bpf-next 07/14] bpf: Embed kernel CONFIG check into the if statement in bpf_setsockopt Martin KaFai Lau
2022-07-27  6:09 ` [PATCH bpf-next 08/14] bpf: Change bpf_setsockopt(SOL_SOCKET) to reuse sock_setsockopt() Martin KaFai Lau
2022-07-27  6:09 ` [PATCH bpf-next 09/14] bpf: Refactor bpf specific tcp optnames to a new function Martin KaFai Lau
2022-07-27  6:09 ` [PATCH bpf-next 10/14] bpf: Change bpf_setsockopt(SOL_TCP) to reuse do_tcp_setsockopt() Martin KaFai Lau
2022-07-27  6:10 ` [PATCH bpf-next 11/14] bpf: Change bpf_setsockopt(SOL_IP) to reuse do_ip_setsockopt() Martin KaFai Lau
2022-07-27  6:10 ` [PATCH bpf-next 12/14] bpf: Change bpf_setsockopt(SOL_IPV6) to reuse do_ipv6_setsockopt() Martin KaFai Lau
2022-07-27  6:10 ` [PATCH bpf-next 13/14] bpf: Add a few optnames to bpf_setsockopt Martin KaFai Lau
2022-07-27  6:10 ` [PATCH bpf-next 14/14] selftests/bpf: bpf_setsockopt tests Martin KaFai Lau
2022-07-27 17:14 ` [PATCH bpf-next 00/14] bpf: net: Remove duplicated codes from bpf_setsockopt() Jakub Kicinski
2022-07-27 20:42   ` Martin KaFai Lau

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=20220728172004.6mpkycl52sszuudc@kafai-mbp.dhcp.thefacebook.com \
    --to=kafai@fb.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kernel-team@fb.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@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.