BPF Archive on lore.kernel.org
 help / color / Atom feed
From: Daniel Borkmann <daniel@iogearbox.net>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Martynas Pumputis <m@lambda.lt>, Joe Stringer <joe@wand.net.nz>,
	bpf <bpf@vger.kernel.org>, Networking <netdev@vger.kernel.org>
Subject: Re: [PATCH bpf-next 6/7] bpf: enable retrival of pid/tgid/comm from bpf cgroup hooks
Date: Sat, 28 Mar 2020 02:40:17 +0100
Message-ID: <b1af640d-527c-8641-6c85-e31e94840f85@iogearbox.net> (raw)
In-Reply-To: <CAEf4BzYjh++aorwBzgjdcWmRiw7GV4p=2avWqZu8S2Jdv3A3tQ@mail.gmail.com>

On 3/28/20 1:49 AM, Andrii Nakryiko wrote:
> On Fri, Mar 27, 2020 at 8:59 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> We already have the bpf_get_current_uid_gid() helper enabled, and
>> given we now have perf event RB output available for connect(),
>> sendmsg(), recvmsg() and bind-related hooks, add a trivial change
>> to enable bpf_get_current_pid_tgid() and bpf_get_current_comm()
>> as well.
>>
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> ---
> 
> LGTM, there was probably never a good reason this wasn't available
> from the very beginning :)

Right. :-)

> Might as well add bpf_get_current_uid_gid() if it's not there yet.

It's already there. ;-)

> Acked-by: Andrii Nakryiko <andriin@fb.com>
> 
>>   net/core/filter.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 5cec3ac9e3dd..bb4a196c8809 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -6018,6 +6018,10 @@ sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>>                  return &bpf_get_netns_cookie_sock_proto;
>>          case BPF_FUNC_perf_event_output:
>>                  return &bpf_event_output_data_proto;
>> +       case BPF_FUNC_get_current_pid_tgid:
>> +               return &bpf_get_current_pid_tgid_proto;
>> +       case BPF_FUNC_get_current_comm:
>> +               return &bpf_get_current_comm_proto;
> 
> So you are not adding it to bpf_base_func_proto() instead, because
> that one can be used in BPF programs that don't have a valid current,
> is that right? If yes, would it make sense to have a common
> bpf_base_process_ctx_func_proto() function for cases where there is a
> valid current and add all the functions there (including uid_gid and
> whatever else makes sense?)

I didn't add it to bpf_base_func_proto() since we might not always have
a useful 'current' available. My focus in this series was on sock_addr
and sock helpers for our LB where 'current' is always the application
doing the syscall. A common base_func_proto() for both could be useful
though this only works with helpers that do not rely on the input context
since both are different. Tbh, the whole net/core/filter.c feels quite
convoluted these days where it's getting hard to follow which func_proto
and access checker belongs to which program type. I can check if we could
get some more order in there in general through some larger refactoring
to make it easier for people to extend, though not sure if more
bpf_base_func_proto()-like helpers would add much, I think it needs a
larger revamp.

>>   #ifdef CONFIG_CGROUPS
>>          case BPF_FUNC_get_current_cgroup_id:
>>                  return &bpf_get_current_cgroup_id_proto;
>> @@ -6058,6 +6062,10 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>>                  return &bpf_get_local_storage_proto;
>>          case BPF_FUNC_perf_event_output:
>>                  return &bpf_event_output_data_proto;
>> +       case BPF_FUNC_get_current_pid_tgid:
>> +               return &bpf_get_current_pid_tgid_proto;
>> +       case BPF_FUNC_get_current_comm:
>> +               return &bpf_get_current_comm_proto;
>>   #ifdef CONFIG_CGROUPS
>>          case BPF_FUNC_get_current_cgroup_id:
>>                  return &bpf_get_current_cgroup_id_proto;
>> --
>> 2.21.0
>>


  reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-27 15:58 [PATCH bpf-next 0/7] Various improvements to cgroup helpers Daniel Borkmann
2020-03-27 15:58 ` [PATCH bpf-next 1/7] bpf: enable retrieval of socket cookie for bind/post-bind hook Daniel Borkmann
2020-03-27 15:58 ` [PATCH bpf-next 2/7] bpf: enable perf event rb output for bpf cgroup progs Daniel Borkmann
2020-03-27 15:58 ` [PATCH bpf-next 3/7] bpf: add netns cookie and enable it for bpf cgroup hooks Daniel Borkmann
2020-03-28  0:32   ` Andrii Nakryiko
2020-03-28  1:30     ` Daniel Borkmann
2020-03-28  1:48   ` Alexei Starovoitov
2020-03-28  2:16     ` Daniel Borkmann
2020-03-28  2:56       ` Alexei Starovoitov
2020-03-27 15:58 ` [PATCH bpf-next 4/7] bpf: allow to retrieve cgroup v1 classid from v2 hooks Daniel Borkmann
2020-03-28  0:41   ` Andrii Nakryiko
2020-03-28  1:56     ` Daniel Borkmann
2020-03-28 20:27       ` Andrii Nakryiko
2020-03-27 15:58 ` [PATCH bpf-next 5/7] bpf: enable bpf cgroup hooks to retrieve cgroup v2 and ancestor id Daniel Borkmann
2020-03-28  0:43   ` Andrii Nakryiko
2020-03-27 15:58 ` [PATCH bpf-next 6/7] bpf: enable retrival of pid/tgid/comm from bpf cgroup hooks Daniel Borkmann
2020-03-28  0:49   ` Andrii Nakryiko
2020-03-28  1:40     ` Daniel Borkmann [this message]
2020-03-27 15:58 ` [PATCH bpf-next 7/7] bpf: add selftest cases for ctx_or_null argument type Daniel Borkmann
2020-03-28  0:51   ` Andrii Nakryiko

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=b1af640d-527c-8641-6c85-e31e94840f85@iogearbox.net \
    --to=daniel@iogearbox.net \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=joe@wand.net.nz \
    --cc=m@lambda.lt \
    --cc=netdev@vger.kernel.org \
    /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

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git