mbox series

[bpf-next,0/4] libbpf: Move CO-RE logic into separate file.

Message ID 20210721000822.40958-1-alexei.starovoitov@gmail.com
Headers show
Series libbpf: Move CO-RE logic into separate file. | expand

Message

Alexei Starovoitov July 21, 2021, 12:08 a.m. UTC
From: Alexei Starovoitov <ast@kernel.org>

Split CO-RE processing logic from libbpf into separate file
with an interface that doesn't dependend on libbpf internal details.
As the next step relo_core.c will be compiled with libbpf and with the kernel.
The _internal_ interface between libbpf/CO-RE and kernel/CO-RE will be:
int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,
			     int insn_idx,
			     const struct bpf_core_relo *relo,
			     int relo_idx,
			     const struct btf *local_btf,
			     struct bpf_core_cand_list *cands);
where bpf_core_relo and bpf_core_cand_list are simple types
prepared by kernel and libbpf.

Though diff stat shows a lot of lines inserted/deleted they are moved lines.
Pls review with diff.colorMoved.

Alexei Starovoitov (4):
  libbpf: Cleanup the layering between CORE and bpf_program.
  libbpf: Split bpf_core_apply_relo() into bpf_program indepdent helper.
  libbpf: Move CO-RE types into relo_core.h.
  libbpf: Split CO-RE logic into relo_core.c.

 tools/lib/bpf/Build             |    2 +-
 tools/lib/bpf/libbpf.c          | 1344 +------------------------------
 tools/lib/bpf/libbpf_internal.h |   81 +-
 tools/lib/bpf/relo_core.c       | 1326 ++++++++++++++++++++++++++++++
 tools/lib/bpf/relo_core.h       |  102 +++
 5 files changed, 1473 insertions(+), 1382 deletions(-)
 create mode 100644 tools/lib/bpf/relo_core.c
 create mode 100644 tools/lib/bpf/relo_core.h

Comments

Toke Høiland-Jørgensen July 22, 2021, 4:02 p.m. UTC | #1
Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> From: Alexei Starovoitov <ast@kernel.org>

>

> Split CO-RE processing logic from libbpf into separate file

> with an interface that doesn't dependend on libbpf internal details.

> As the next step relo_core.c will be compiled with libbpf and with the kernel.


Interesting! What's the use case for having it in the kernel as well? :)

-Toke
Alexei Starovoitov July 24, 2021, 12:23 a.m. UTC | #2
On Thu, Jul 22, 2021 at 9:02 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>

> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

>

> > From: Alexei Starovoitov <ast@kernel.org>

> >

> > Split CO-RE processing logic from libbpf into separate file

> > with an interface that doesn't dependend on libbpf internal details.

> > As the next step relo_core.c will be compiled with libbpf and with the kernel.

>

> Interesting! What's the use case for having it in the kernel as well? :)


The main motivation is signed programs, of course.
Also there are other reasons:
- give the verifier precise path to the field in load/store instructions.
Currently it has to guess the field based on integer offset.
That guessing is random in case of a union.
- give the kermel ability to do CO-RE or symbolic field access.
The insn patching is a small part of the bpf_core_apply_relo_insn().
It can be done for x86 and any other archs just as well.
Imagine a true kernel struct randomization.
Not the existing one where gcc plugin does it at build time,
but the one where the kernel randomizes struct cred every single boot.
Or imagine kernel modules that are built once and then can be loaded
and run on a variety of kernels.
Andrii Nakryiko July 26, 2021, 7:37 p.m. UTC | #3
On Tue, Jul 20, 2021 at 5:08 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>

> From: Alexei Starovoitov <ast@kernel.org>

>

> Split CO-RE processing logic from libbpf into separate file

> with an interface that doesn't dependend on libbpf internal details.

> As the next step relo_core.c will be compiled with libbpf and with the kernel.

> The _internal_ interface between libbpf/CO-RE and kernel/CO-RE will be:

> int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,

>                              int insn_idx,

>                              const struct bpf_core_relo *relo,

>                              int relo_idx,

>                              const struct btf *local_btf,

>                              struct bpf_core_cand_list *cands);

> where bpf_core_relo and bpf_core_cand_list are simple types

> prepared by kernel and libbpf.

>

> Though diff stat shows a lot of lines inserted/deleted they are moved lines.

> Pls review with diff.colorMoved.

>

> Alexei Starovoitov (4):

>   libbpf: Cleanup the layering between CORE and bpf_program.

>   libbpf: Split bpf_core_apply_relo() into bpf_program indepdent helper.

>   libbpf: Move CO-RE types into relo_core.h.

>   libbpf: Split CO-RE logic into relo_core.c.

>


LGTM. Applied to bpf-next, fixed typo in patch 3 subject, and also
made few adjustments. Let me know if you object to any of them:

1. I felt like the original copyright year should be preserved when
moving code into a new file, so I've changed relo_core.h's year to
2019. Hope that's fine.
2. relo_core.c didn't have a Copyright line, so I added the /*
Copyright (c) 2019 Facebook */ as well.
3. I trimmed down the list of #includes in core_relo.c, because most
of them were absolutely irrelevant and just preserved as-is from
libbpf.c Everything seems to compile just fine without those.

>  tools/lib/bpf/Build             |    2 +-

>  tools/lib/bpf/libbpf.c          | 1344 +------------------------------

>  tools/lib/bpf/libbpf_internal.h |   81 +-

>  tools/lib/bpf/relo_core.c       | 1326 ++++++++++++++++++++++++++++++

>  tools/lib/bpf/relo_core.h       |  102 +++

>  5 files changed, 1473 insertions(+), 1382 deletions(-)

>  create mode 100644 tools/lib/bpf/relo_core.c

>  create mode 100644 tools/lib/bpf/relo_core.h

>

> --

> 2.30.2

>
Alexei Starovoitov July 28, 2021, 4:49 a.m. UTC | #4
On Mon, Jul 26, 2021 at 12:38 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>

> On Tue, Jul 20, 2021 at 5:08 PM Alexei Starovoitov

> <alexei.starovoitov@gmail.com> wrote:

> >

> > From: Alexei Starovoitov <ast@kernel.org>

> >

> > Split CO-RE processing logic from libbpf into separate file

> > with an interface that doesn't dependend on libbpf internal details.

> > As the next step relo_core.c will be compiled with libbpf and with the kernel.

> > The _internal_ interface between libbpf/CO-RE and kernel/CO-RE will be:

> > int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,

> >                              int insn_idx,

> >                              const struct bpf_core_relo *relo,

> >                              int relo_idx,

> >                              const struct btf *local_btf,

> >                              struct bpf_core_cand_list *cands);

> > where bpf_core_relo and bpf_core_cand_list are simple types

> > prepared by kernel and libbpf.

> >

> > Though diff stat shows a lot of lines inserted/deleted they are moved lines.

> > Pls review with diff.colorMoved.

> >

> > Alexei Starovoitov (4):

> >   libbpf: Cleanup the layering between CORE and bpf_program.

> >   libbpf: Split bpf_core_apply_relo() into bpf_program indepdent helper.

> >   libbpf: Move CO-RE types into relo_core.h.

> >   libbpf: Split CO-RE logic into relo_core.c.

> >

>

> LGTM. Applied to bpf-next, fixed typo in patch 3 subject, and also

> made few adjustments. Let me know if you object to any of them:

>

> 1. I felt like the original copyright year should be preserved when

> moving code into a new file, so I've changed relo_core.h's year to

> 2019. Hope that's fine.

> 2. relo_core.c didn't have a Copyright line, so I added the /*

> Copyright (c) 2019 Facebook */ as well.

> 3. I trimmed down the list of #includes in core_relo.c, because most

> of them were absolutely irrelevant and just preserved as-is from

> libbpf.c Everything seems to compile just fine without those.


Thanks! Much appreciate it.
It was on my todo list. I lazily copy-pasted them to avoid
accidental breakage on some archs that I don't have access to
(since I didn't wait for the kernel build bot to process them before I
sent them).
fyi intel folks can include your private tree as well, so you'd have to respin
your patches due to odd 32-bit build breakage. Just email them with
your git tree location.
Andrii Nakryiko July 29, 2021, 6:43 p.m. UTC | #5
On Tue, Jul 27, 2021 at 9:49 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>

> On Mon, Jul 26, 2021 at 12:38 PM Andrii Nakryiko

> <andrii.nakryiko@gmail.com> wrote:

> >

> > On Tue, Jul 20, 2021 at 5:08 PM Alexei Starovoitov

> > <alexei.starovoitov@gmail.com> wrote:

> > >

> > > From: Alexei Starovoitov <ast@kernel.org>

> > >

> > > Split CO-RE processing logic from libbpf into separate file

> > > with an interface that doesn't dependend on libbpf internal details.

> > > As the next step relo_core.c will be compiled with libbpf and with the kernel.

> > > The _internal_ interface between libbpf/CO-RE and kernel/CO-RE will be:

> > > int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,

> > >                              int insn_idx,

> > >                              const struct bpf_core_relo *relo,

> > >                              int relo_idx,

> > >                              const struct btf *local_btf,

> > >                              struct bpf_core_cand_list *cands);

> > > where bpf_core_relo and bpf_core_cand_list are simple types

> > > prepared by kernel and libbpf.

> > >

> > > Though diff stat shows a lot of lines inserted/deleted they are moved lines.

> > > Pls review with diff.colorMoved.

> > >

> > > Alexei Starovoitov (4):

> > >   libbpf: Cleanup the layering between CORE and bpf_program.

> > >   libbpf: Split bpf_core_apply_relo() into bpf_program indepdent helper.

> > >   libbpf: Move CO-RE types into relo_core.h.

> > >   libbpf: Split CO-RE logic into relo_core.c.

> > >

> >

> > LGTM. Applied to bpf-next, fixed typo in patch 3 subject, and also

> > made few adjustments. Let me know if you object to any of them:

> >

> > 1. I felt like the original copyright year should be preserved when

> > moving code into a new file, so I've changed relo_core.h's year to

> > 2019. Hope that's fine.

> > 2. relo_core.c didn't have a Copyright line, so I added the /*

> > Copyright (c) 2019 Facebook */ as well.

> > 3. I trimmed down the list of #includes in core_relo.c, because most

> > of them were absolutely irrelevant and just preserved as-is from

> > libbpf.c Everything seems to compile just fine without those.

>

> Thanks! Much appreciate it.

> It was on my todo list. I lazily copy-pasted them to avoid

> accidental breakage on some archs that I don't have access to

> (since I didn't wait for the kernel build bot to process them before I

> sent them).

> fyi intel folks can include your private tree as well, so you'd have to respin

> your patches due to odd 32-bit build breakage. Just email them with

> your git tree location.


yeah, that's a good idea, I'll email them