Re: [PATCH v4 bpf-next 3/3] veristat: guess and substitue underlying program type for freplace (EXT) progs

Eduard Zingerman
2023-03-30T14:49:05Z

On Wed, 2023-03-29 at 22:38 -0700, Alexei Starovoitov wrote:
> On Wed, Mar 29, 2023 at 11:36 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > On Mon, 2023-03-27 at 11:52 -0700, Andrii Nakryiko wrote:
> > > SEC("freplace") (i.e., BPF_PROG_TYPE_EXT) programs are not loadable as
> > > is through veristat, as kernel expects actual program's FD during
> > > BPF_PROG_LOAD time, which veristat has no way of knowing.
> > >
> > > Unfortunately, freplace programs are a pretty important class of
> > > programs, especially when dealing with XDP chaining solutions, which
> > > rely on EXT programs.
> > >
> > > So let's do our best and teach veristat to try to guess the original
> > > program type, based on program's context argument type. And if guessing
> > > process succeeds, we manually override freplace/EXT with guessed program
> > > type using bpf_program__set_type() setter to increase chances of proper
> > > BPF verification.
> > >
> > > We rely on BTF and maintain a simple lookup table. This process is
> > > obviously not 100% bulletproof, as valid program might not use context
> > > and thus wouldn't have to specify correct type. Also, __sk_buff is very
> > > ambiguous and is the context type across many different program types.
> > > We pick BPF_PROG_TYPE_CGROUP_SKB for now, which seems to work fine in
> > > practice so far. Similarly, some program types require specifying attach
> > > type, and so we pick one out of possible few variants.
> > >
> > > Best effort at its best. But this makes veristat even more widely
> > > applicable.
> > >
> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> >
> > I left one nitpick below, otherwise looks good.
> >
> > I tried in on freplace programs from selftests and only 3 out 18
> > programs verified correctly, others complained about unavailable
> > functions or exit code not in range [0, 1], etc.
> > Not sure, if it's possible to select more permissive attachment kinds, though.
> >
> > Tested-by: Eduard Zingerman <eddyz87@gmail.com>
>
> Thanks for testing and important feedback.
> I've applied the set. The nits can be addressed in the follow up.
>
> What do you have in mind as 'more permissive attach' ?
> What are those 15 out of 18 with invalid exit code?
> What kind of attach_type will help?

TLDR: I apologize, it was a bad comment.
Should have done more analysis yesterday and withheld from commenting.

---

Inspected each program and it turned out that most of the failing ones
are either not programs but separate functions, or have __skb_buf parameter.
The summary table is at the end of the email, here is the list of those
that should load but fail:

- Have __skb_buf parameter and attach to SEC("tc")
- fexit_bpf2bpf.bpf.o:new_get_skb_len
- freplace_cls_redirect.bpf.o:freplace_cls_redirect_test
- freplace_global_func.bpf.o:new_test_pkt_access
- Need ifindex to be specified prior loading:
- xdp_metadata2.bpf.o:freplace_rx
Should
File Program Verdict fail? Reason
------------------------------ --------------------------------- ------- ------ -----------------------
fexit_bpf2bpf.bpf.o new_get_constant failure no Function, not a program
fexit_bpf2bpf.bpf.o new_get_skb_ifindex failure no Function, not a program
fexit_bpf2bpf.bpf.o new_get_skb_len failure no __sk_buff parameter
fexit_bpf2bpf.bpf.o new_test_pkt_write_access_subprog failure no Function, not a program
fexit_bpf2bpf.bpf.o test_main failure no Function, not a program
fexit_bpf2bpf.bpf.o test_subprog1 failure no Function, not a program
fexit_bpf2bpf.bpf.o test_subprog2 failure no Function, not a program
fexit_bpf2bpf.bpf.o test_subprog3 failure no Function, not a program
freplace_attach_probe.bpf.o new_handle_kprobe failure yes
freplace_cls_redirect.bpf.o freplace_cls_redirect_test failure no __sk_buff parameter
freplace_connect4.bpf.o new_do_bind failure yes
freplace_connect_v4_prog.bpf.o new_connect_v4_prog failure yes
freplace_get_constant.bpf.o security_new_get_constant failure no Function, not a program
freplace_global_func.bpf.o new_test_pkt_access failure no __sk_buff parameter
freplace_progmap.bpf.o xdp_cpumap_prog success
freplace_progmap.bpf.o xdp_drop_prog success
test_trace_ext.bpf.o test_pkt_md_access_new success
xdp_metadata2.bpf.o freplace_rx failure no needs ifindex
------------------------------ --------------------------------- ------- ------- -----------------------