From: Eduard Zingerman <eddyz87@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Alexei Starovoitov <ast@kernel.org>, andrii <andrii@kernel.org>,
Nick Zavaritsky <mejedi@gmail.com>, bpf <bpf@vger.kernel.org>,
Kumar Kartikeya Dwivedi <memxor@gmail.com>
Subject: Re: Packet pointer invalidation and subprograms
Date: Fri, 06 Dec 2024 09:58:47 -0800 [thread overview]
Message-ID: <fca94f90badf43ee16e2773faf35e136d551ec28.camel@gmail.com> (raw)
In-Reply-To: <CAEf4BzZJOxnm7z6QaxRr9PsfD_DTV5nSPP9TjiEMQxNMxzLFRA@mail.gmail.com>
On Fri, 2024-12-06 at 09:46 -0800, Andrii Nakryiko wrote:
> On Fri, Dec 6, 2024 at 9:29 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > On Fri, 2024-12-06 at 08:08 -0800, Andrii Nakryiko wrote:
> >
> > [...]
> >
> > > The tags would be that generalizable side effect declaration approach,
> > > so seems worth it to set a uniform approach.
> > >
> > > > Please take a look at the patch, the change for check_cfg() is 32 lines.
> > >
> > > I did, actually. And I already explained what I don't like about it:
> > > eagerness. check_cfg() is not the right place for this, if we want to
> > > support dead code elimination and BPF CO-RE-based feature gating.
> > > Which your patches clearly violate, so I don't like them, sorry.
> > >
> > > We made this eagerness mistake with global subprogs verification
> > > previously, and had to switch it to lazy on-demand global subprog
> > > validation. I think we should preserve this lazy approach going
> > > forward.
> >
> > In this context tags have same detection power as current changes for check_cfg(),
>
> You keep ignoring the eagerness issue. I can't decide whether you
> think *it makes no difference* (I disagree, but whatever), or you *see
> no difference* (in which case let me know and I can explain with some
> simple example).
In the context of the packet pointer invalidation I see no difference.
Tags are as eager as check_cfg() traversal.
> > it is not possible to remove tag using dead code elimination.
>
> That's not the point of the tag to be dynamically adjustable. It's the
> opposite. It's something that the user declares upfront, and this is
> being enforced by the verifier (to prevent user errors, for example).
> If the user wants to have a "dynamic tag", they can have two global
> subprogs, one with and one without the tag, and pick which one should
> be called through, e.g., .rodata feature flag variable. I.e., make
> this decision outside of global subprog itself.
>
> > So I really don't see any advantages in the context of this particular issue.
>
> See also my reply to Alexei, and keep in mind freplace scenario, as
> one of the things your approach can't support.
Some freplace related mark will have to be present after program verification.
It might be in a form of a tag, or in a form of an additional bit in
an auxiliary structure. There would be code to check this with both approaches.
next prev parent reply other threads:[~2024-12-06 17:58 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-03 16:26 Packet pointer invalidation and subprograms Nick Zavaritsky
2024-12-03 20:19 ` Eduard Zingerman
2024-12-03 21:41 ` Eduard Zingerman
2024-12-06 0:03 ` Andrii Nakryiko
2024-12-06 0:12 ` Kumar Kartikeya Dwivedi
2024-12-06 0:29 ` Eduard Zingerman
2024-12-06 1:44 ` Alexei Starovoitov
2024-12-06 4:07 ` Eduard Zingerman
2024-12-06 6:22 ` Andrii Nakryiko
2024-12-06 10:44 ` Eduard Zingerman
2024-12-06 16:08 ` Andrii Nakryiko
2024-12-06 17:29 ` Eduard Zingerman
2024-12-06 17:46 ` Andrii Nakryiko
2024-12-06 17:58 ` Eduard Zingerman [this message]
2024-12-06 18:10 ` Andrii Nakryiko
2024-12-06 18:29 ` Eduard Zingerman
2024-12-06 16:07 ` Alexei Starovoitov
2024-12-06 16:12 ` Andrii Nakryiko
2024-12-06 16:20 ` Alexei Starovoitov
2024-12-06 17:42 ` Andrii Nakryiko
2024-12-06 18:23 ` Kumar Kartikeya Dwivedi
2024-12-06 18:30 ` Alexei Starovoitov
2024-12-06 19:31 ` Kumar Kartikeya Dwivedi
2024-12-06 18:26 ` Alexei Starovoitov
2024-12-06 18:30 ` Kumar Kartikeya Dwivedi
2024-12-06 18:32 ` Alexei Starovoitov
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=fca94f90badf43ee16e2773faf35e136d551ec28.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=mejedi@gmail.com \
--cc=memxor@gmail.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.