From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
John Fastabend <john.fastabend@gmail.com>,
Jiri Olsa <jolsa@redhat.com>,
Eelco Chaudron <echaudro@redhat.com>,
KP Singh <kpsingh@chromium.org>,
Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next v7 03/10] bpf: verifier: refactor check_attach_btf_id()
Date: Tue, 22 Sep 2020 13:16:10 +0200 [thread overview]
Message-ID: <87a6xioydh.fsf@toke.dk> (raw)
In-Reply-To: <CAEf4BzY4UR+KjZ3bY6ykyW5CPNwAzwgKVhYHGdgDuMT2nntmTg@mail.gmail.com>
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> On Sat, Sep 19, 2020 at 4:50 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> The check_attach_btf_id() function really does three things:
>>
>> 1. It performs a bunch of checks on the program to ensure that the
>> attachment is valid.
>>
>> 2. It stores a bunch of state about the attachment being requested in
>> the verifier environment and struct bpf_prog objects.
>>
>> 3. It allocates a trampoline for the attachment.
>>
>> This patch splits out (1.) and (3.) into separate functions in preparation
>> for reusing them when the actual attachment is happening (in the
>> raw_tracepoint_open syscall operation), which will allow tracing programs
>> to have multiple (compatible) attachments.
>>
>> No functional change is intended with this patch.
>>
>> Acked-by: Andrii Nakryiko <andriin@fb.com>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> ---
>
> Ok, so bad news: you broke another selftest (test_overhead). Please,
> do run test_progs and make sure everything succeeds, every time before
> you post a new version.
Right, so I looked into this, and it seems the only reason it was
succeeding before were those skipped checks you pointed out that are now
fixed. I.e., __set_task_comm() is not actually supposed to be
fmod_ret'able according to check_attach_modify_return(). So I'm not sure
what the right way to fix this is?
The fmod_ret bit was added to test_overhead by:
4eaf0b5c5e04 ("selftest/bpf: Fmod_ret prog and implement test_overhead as part of bench")
so the obvious thing is to just do a (partial) revert of that? WDYT?
-Toke
next prev parent reply other threads:[~2020-09-22 11:16 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-19 11:49 [PATCH bpf-next v7 00/10] bpf: Support multi-attach for freplace programs Toke Høiland-Jørgensen
2020-09-19 11:49 ` [PATCH bpf-next v7 01/10] bpf: disallow attaching modify_return tracing functions to other BPF programs Toke Høiland-Jørgensen
2020-09-21 22:39 ` Andrii Nakryiko
2020-09-22 9:52 ` Toke Høiland-Jørgensen
2020-09-19 11:49 ` [PATCH bpf-next v7 02/10] bpf: change logging calls from verbose() to bpf_log() and use log pointer Toke Høiland-Jørgensen
2020-09-19 11:49 ` [PATCH bpf-next v7 03/10] bpf: verifier: refactor check_attach_btf_id() Toke Høiland-Jørgensen
2020-09-21 23:05 ` Andrii Nakryiko
2020-09-22 10:14 ` Toke Høiland-Jørgensen
2020-09-22 11:16 ` Toke Høiland-Jørgensen [this message]
2020-09-22 16:28 ` Andrii Nakryiko
2020-09-22 17:41 ` Toke Høiland-Jørgensen
2020-09-19 11:49 ` [PATCH bpf-next v7 04/10] bpf: move prog->aux->linked_prog and trampoline into bpf_link on attach Toke Høiland-Jørgensen
2020-09-21 23:05 ` Andrii Nakryiko
2020-09-22 10:17 ` Toke Høiland-Jørgensen
2020-09-22 16:45 ` Andrii Nakryiko
2020-09-22 17:48 ` Toke Høiland-Jørgensen
2020-09-19 11:49 ` [PATCH bpf-next v7 05/10] bpf: support attaching freplace programs to multiple attach points Toke Høiland-Jørgensen
2020-09-21 23:08 ` Andrii Nakryiko
2020-09-19 11:49 ` [PATCH bpf-next v7 06/10] bpf: Fix context type resolving for extension programs Toke Høiland-Jørgensen
2020-09-21 23:09 ` Andrii Nakryiko
2020-09-19 11:49 ` [PATCH bpf-next v7 07/10] libbpf: add support for freplace attachment in bpf_link_create Toke Høiland-Jørgensen
2020-09-21 23:18 ` Andrii Nakryiko
2020-09-19 11:49 ` [PATCH bpf-next v7 08/10] selftests: add test for multiple attachments of freplace program Toke Høiland-Jørgensen
2020-09-21 23:21 ` Andrii Nakryiko
2020-09-19 11:49 ` [PATCH bpf-next v7 09/10] selftests/bpf: Adding test for arg dereference in extension trace Toke Høiland-Jørgensen
2020-09-19 11:49 ` [PATCH bpf-next v7 10/10] selftests: Add selftest for disallowing modify_return attachment to freplace Toke Høiland-Jørgensen
2020-09-21 23:25 ` Andrii Nakryiko
2020-09-21 23:26 ` [PATCH bpf-next v7 00/10] bpf: Support multi-attach for freplace programs Andrii Nakryiko
2020-09-22 9:48 ` Toke Høiland-Jørgensen
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=87a6xioydh.fsf@toke.dk \
--to=toke@redhat.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=echaudro@redhat.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@redhat.com \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=yhs@fb.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.