netdev - Re: [PATCH bpf-next 2/3] bpftool: don't append / to the progtype

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 26 Oct 2021 10:03:27 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Stanislav Fomichev <sdf@...gle.com>
Cc:     John Fastabend <john.fastabend@...il.com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>
Subject: Re: [PATCH bpf-next 2/3] bpftool: don't append / to the progtype

On Tue, Oct 26, 2021 at 8:46 AM Stanislav Fomichev <sdf@...gle.com> wrote:
>
> On Mon, Oct 25, 2021 at 9:27 PM Andrii Nakryiko
> <andrii.nakryiko@...il.com> wrote:
> >
> > On Mon, Oct 25, 2021 at 8:59 AM Stanislav Fomichev <sdf@...gle.com> wrote:
> > >
> > > On Fri, Oct 22, 2021 at 10:05 AM John Fastabend
> > > <john.fastabend@...il.com> wrote:
> > > >
> > > > Stanislav Fomichev wrote:
> > > > > Otherwise, attaching with bpftool doesn't work with strict section names.
> > > > >
> > > > > Also, switch to libbpf strict mode to use the latest conventions
> > > > > (note, I don't think we have any cli api guarantees?).
> > > > >
> > > > > Signed-off-by: Stanislav Fomichev <sdf@...gle.com>
> > > > > ---
> > > > >  tools/bpf/bpftool/main.c |  4 ++++
> > > > >  tools/bpf/bpftool/prog.c | 15 +--------------
> > > > >  2 files changed, 5 insertions(+), 14 deletions(-)
> > > > >
> > > > > diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> > > > > index 02eaaf065f65..8223bac1e401 100644
> > > > > --- a/tools/bpf/bpftool/main.c
> > > > > +++ b/tools/bpf/bpftool/main.c
> > > > > @@ -409,6 +409,10 @@ int main(int argc, char **argv)
> > > > >       block_mount = false;
> > > > >       bin_name = argv[0];
> > > > >
> > > > > +     ret = libbpf_set_strict_mode(LIBBPF_STRICT_ALL);
> > > > > +     if (ret)
> > > > > +             p_err("failed to enable libbpf strict mode: %d", ret);
> > > > > +
> > > >
> > > > Would it better to just warn? Seems like this shouldn't be fatal from
> > > > bpftool side?
> > > >
> > > > Also this is a potentially breaking change correct? Programs that _did_
> > > > work in the unstrict might suddently fail in the strict mode? If this
> > > > is the case whats the versioning plan? We don't want to leak these
> > > > type of changes across multiple versions, idealy we have a hard
> > > > break and bump the version.
> > > >
> > > > I didn't catch a cover letter on the series. A small
> > > > note about versioning and upgrading bpftool would be helpful.
> > >
> > > Yeah, it is a breaking change, every program that has non-strict
> > > section names will be rejected.
> > >
> > > I mentioned that in the bpftool's commit description:
> > > Also, switch to libbpf strict mode to use the latest conventions
> > > (note, I don't think we have any cli api guarantees?).
> > >
> > > So I'm actually not sure what's the best way to handle this migration
> > > and whether we really provide any cli guarantees to the users. I was
> > > always assuming that bpftool is mostly for debugging/introspection,
> > > but not sure.
> > >
> > > As Andrii suggested in another email, I can add a flag to disable this
> > > strict mode. Any better ideas?
> >
> > Maybe the other way around for the transition period. Add a --strict
> > flag to turn on libbpf strict mode? This follows libbpf's opt-in
> > approach to breaking change. We can also emit warnings when people are
> > trying to pin programs and mention that they should switch to --strict
> > as in some future version this will be the default. Would that be
> > better for users?
>
> Agreed, that sounds better for backwards compatibility. However, I'm
> not sure when we set that --strict to 'true' by default. The same
> moment libbpf loses non-strict behavior?

Yep, probably it will have to coincide with libbpf 1.0 release. And
it's not setting it to true, it's more like enforcing it to true (or
just dropping the --strict flag altogether).