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 RESEND bpf-next v3 2/9] bpf: verifier: refactor check_attach_btf_id()
Date: Fri, 11 Sep 2020 21:50:55 +0200 [thread overview]
Message-ID: <878sdg9jog.fsf@toke.dk> (raw)
In-Reply-To: <CAEf4BzYNpOjt=Ua1hg3jAEe07a9mEd1UF2CZPys05O+ReaLo+Q@mail.gmail.com>
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> On Fri, Sep 11, 2020 at 3:00 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.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> ---
>
> I can't tell if there are any functional changes or not, tbh. The
> logic is quite complicated and full of intricate details. I did leave
> some suggestions on hopefully simplifying code flow in some places
> (and ensuring it's harder to break it on future changes), but I hope
> Alexei will give it a very thorough review and check that none of the
> subtle details broke.
Yeah, totally agree this is gnarly... :/
Which is also why I chickened out of doing any further changes in an
attempt to simplify the flow, but rather kept as much as the existing
structure as possible (with somewhat mixed results, I suppose).
Let's see what Alexei thinks. I guess I can take another crack at it, in
which case, thank you for the suggestions for simplifying things!
-Toke
next prev parent reply other threads:[~2020-09-11 19:51 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-11 9:59 [PATCH RESEND bpf-next v3 0/9] bpf: Support multi-attach for freplace programs Toke Høiland-Jørgensen
2020-09-11 9:59 ` [PATCH RESEND bpf-next v3 1/9] bpf: change logging calls from verbose() to bpf_log() and use log pointer Toke Høiland-Jørgensen
2020-09-11 9:59 ` [PATCH RESEND bpf-next v3 2/9] bpf: verifier: refactor check_attach_btf_id() Toke Høiland-Jørgensen
2020-09-11 19:13 ` Andrii Nakryiko
2020-09-11 19:50 ` Toke Høiland-Jørgensen [this message]
2020-09-11 9:59 ` [PATCH RESEND bpf-next v3 3/9] bpf: wrap prog->aux->linked_prog in a bpf_tracing_link Toke Høiland-Jørgensen
2020-09-11 19:57 ` Andrii Nakryiko
2020-09-11 20:56 ` Toke Høiland-Jørgensen
2020-09-11 9:59 ` [PATCH RESEND bpf-next v3 4/9] bpf: support attaching freplace programs to multiple attach points Toke Høiland-Jørgensen
2020-09-11 21:10 ` Andrii Nakryiko
2020-09-14 16:08 ` Toke Høiland-Jørgensen
2020-09-14 23:10 ` Andrii Nakryiko
2020-09-15 11:35 ` Toke Høiland-Jørgensen
2020-09-11 9:59 ` [PATCH RESEND bpf-next v3 5/9] bpf: Fix context type resolving for extension programs Toke Høiland-Jørgensen
2020-09-11 9:59 ` [PATCH RESEND bpf-next v3 6/9] tools: add new members to bpf_attr.raw_tracepoint in bpf.h Toke Høiland-Jørgensen
2020-09-11 21:16 ` Andrii Nakryiko
2020-09-11 9:59 ` [PATCH RESEND bpf-next v3 7/9] libbpf: add support for supplying target to bpf_raw_tracepoint_open() Toke Høiland-Jørgensen
2020-09-11 21:21 ` Andrii Nakryiko
2020-09-11 9:59 ` [PATCH RESEND bpf-next v3 8/9] selftests: add test for multiple attachments of freplace program Toke Høiland-Jørgensen
2020-09-11 9:59 ` [PATCH RESEND bpf-next v3 9/9] selftests/bpf: Adding test for arg dereference in extension trace 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=878sdg9jog.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.