Re: [PATCH bpf-next 4/4] selftests/bpf: Test static data relocation

From: Joe Stringer <joe@wand.net.nz>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Joe Stringer <joe@wand.net.nz>,
	bpf@vger.kernel.org, netdev <netdev@vger.kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	ast@kernel.org
Subject: Re: [PATCH bpf-next 4/4] selftests/bpf: Test static data relocation
Date: Tue, 12 Feb 2019 12:43:21 -0800	[thread overview]
Message-ID: <CAOftzPjffHZUK=uxiTsuOGLX4A5ag5oqRz6gorhREj6hxMFA4A@mail.gmail.com> (raw)
In-Reply-To: <20190212050113.qkryxb34vpnst6w6@ast-mbp>

On Mon, Feb 11, 2019 at 9:01 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Feb 11, 2019 at 04:47:29PM -0800, Joe Stringer wrote:
> > Add tests for libbpf relocation of static variable references into the
> > .data and .bss sections of the ELF.
> >
> > Signed-off-by: Joe Stringer <joe@wand.net.nz>
> ...
> > +#define __fetch(x) (__u32)(&(x))
> > +
> > +static __u32 static_bss = 0; /* Reloc reference to .bss section */
> > +static __u32 static_data = 42;       /* Reloc reference to .data section */
> > +
> > +/**
> > + * Load a u32 value from a static variable into a map, for the userland test
> > + * program to validate.
> > + */
> > +SEC("static_data_load")
> > +int load_static_data(struct __sk_buff *skb)
> > +{
> > +     __u32 key, value;
> > +
> > +     key = 0;
> > +     value = __fetch(static_bss);
>
> If we proceed with this approach we will not be able to add support
> for normal 'value = static_bss;' C code in the future.

Hmm, completely agree that breaking future use of standard code is a
non-starter.

Digging around a bit more, I think I could drop the .bss patch/code
here and still end up with something that will work for my use case.
Just need to ensure that all template values are non-zero when run
through the compiler.

> Let's figure out the way to do it right from the start.
> Support for global and static variables is must have feature to add asap,
> but let's not cut the corner like this.
> We did such hacks in the past and every time it came back to bite us.

Do you see any value in having incremental support in libbpf that
could be used as a fallback for older kernels like in patch #2 of this
series? I could imagine libbpf probing kernel support for
global/static variables and attempting to handle references to .data
via some more comprehensive mechanism in-kernel, or falling back to
this approach if it is not available.