On Tue, Jul 19, 2022 at 1:33 PM Stanislav Fomichev <sdf@xxxxxxxxxx> wrote:
>
> On Tue, Jul 19, 2022 at 1:21 PM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
> >
> > On Mon, Jul 18, 2022 at 12:07 PM Stanislav Fomichev <sdf@xxxxxxxxxx> wrote:
> > >
> > > Motivation:
> > >
> > > Our bpf programs have a bunch of options which are set at the loading
> > > time. After loading, they don't change. We currently use array map
> > > to store them and bpf program does the following:
> > >
> > > val = bpf_map_lookup_elem(&config_map, &key);
> > > if (likely(val && *val)) {
> > > // do some optional feature
> > > }
> > >
> > > Since the configuration is static and we have a lot of those features,
> > > I feel like we're wasting precious cycles doing dynamic lookups
> > > (and stalling on memory loads).
> > >
> > > I was assuming that converting those to some fake kconfig options
> > > would solve it, but it still seems like kconfig is stored in the
> > > global map and kconfig entries are resolved dynamically.
> > >
> > > Proposal:
> > >
> > > Resolve kconfig options statically upon loading. Basically rewrite
> > > ld+ldx to two nops and 'mov val, x'.
> > >
> > > I'm also trying to rewrite conditional jump when the condition is
> > > !imm. This seems to be catching all the cases in my program, but
> > > it's probably too hacky.
> > >
> > > I've attached very raw RFC patch to demonstrate the idea. Anything
> > > I'm missing? Any potential problems with this approach?
> >
> > Have you considered using global variables for that?
> > With skeleton the user space has a natural way to set
> > all of these knobs after doing skel_open and before skel_load.
> > Then the verifier sees them as readonly vars and
> > automatically converts LDX into fixed constants and if the code
> > looks like if (my_config_var) then the verifier will remove
> > all the dead code too.
>
> Hm, that's a good alternative, let me try it out. Thanks!
Turns out we already freeze kconfig map in libbpf:
if (map_type == LIBBPF_MAP_RODATA || map_type == LIBBPF_MAP_KCONFIG) {
err = bpf_map_freeze(map->fd);
And I've verified that I do hit bpf_map_direct_read in the verifier.
But the code still stays the same (bpftool dump xlated):
72: (18) r1 = map[id:24][0]+20
74: (61) r1 = *(u32 *)(r1 +0)
75: (bf) r2 = r9
76: (b7) r0 = 0
77: (15) if r1 == 0x0 goto pc+9
I guess there is nothing for sanitize_dead_code to do because my
conditional is "if (likely(some_condition)) { do something }" and the
branch instruction itself is '.seen' by the verifier.
And, most annoyingly for me, 72 and 74 lookups still stay :-(
I can try to unwrap/resolve these on the verifier side instead of libbpf maybe..
