On Fri, 21 Jun 2019 at 05:20, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Thu, Jun 20, 2019 at 7:49 AM Lorenz Bauer <lmb@xxxxxxxxxxxxxx> wrote:
> >
> > On Tue, 18 Jun 2019 at 22:37, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote:
> >
> > > > I would just drop the object-scope pinning. We avoided using it and I'm not
> > > > aware if anyone else make use. It also has the ugly side-effect that this
> > > > relies on AF_ALG which e.g. on some cloud provider shipped kernels is disabled.
> > > > The pinning attribute should be part of the standard set of map attributes for
> > > > libbpf though as it's generally useful for networking applications.
> > >
> > > Sounds good. I'll do some more surveying of use cases inside FB to see
> > > if anyone needs object-scope pinning, just to be sure we are not
> > > short-cutting anyone.
> >
> > I'm also curious what the use cases for declarative pinning are. From my
> > limited POV it doesn't seem that useful? There are a couple of factors:
>
> Cilium is using it pretty extensively, so there are clearly use cases.
> The most straigtforward use case is using a map created and shared by
> another BPF program (to communicate, read stats, what have you).
I think Cilium is in the quirky position that it has a persistent daemon, but
shells out to tc for loading programs. They are probably also the most
advanced (open-source) users of BPF out there. If I understood their comments
correctly they want to move to using a library for loading their ELF. At that
point whether something is possible in a declarative way is less important,
because you have the much more powerful APIs at your disposal.
Maybe Daniel or someone else from the Cilium team can chime in here?
> > * Systemd mounts the default location only accessible to root, so I have to
> > used my own bpffs mount.
> > * Since I don't want to hard code that, I put it in a config file.
> > * After loading the ELF we pin maps from the daemon managing the XDP.
>
> So mounting root would be specified per bpf_object, before maps are
> created, so user-land driving application will have an opportunity to
> tune everything. Declarative is only the per-map decision of whether
> that map should be exposed to outer world (for sharing) or not.
So `tc filter add bpf obj foo.elf pin-root /gobbledygook`?
> Then check tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
> for more crazy syntax ;)
>
> typedef char * (* const (* const fn_ptr_arr2_t[5])())(char * (*)(int));
Not on a Friday ;P
> > What if this did
> >
> > __type(value, struct my_value)[1000];
> > struct my_value __member(value)[1000]; // alternative
> >
> > instead, and skipped max_entries?
>
> I considered that, but decided for now to keep all those attributes
> orthogonal for more flexibility and uniformity. This syntax might be
> considered a nice "syntax sugar" and can be added in the future, if
> necessary.
Ack.
> > At that point you have to understand that value is a pointer so all of
> > our efforts
> > are for naught. I suspect there is other weirdness like this, but I need to play
> > with it a little bit more.
>
> Yes, C can let you do crazy stuff, if you wish, but I think that
> shouldn't be a blocker for this proposal. I haven't seen any BPF
> program doing that, usually you duplicate the type of inner value
> inside your function anyway, so there is no point in taking
> sizeof(map.value) from BPF program side. From outside, though, all the
> types will make sense, as expected.
Right, but in my mind that is a bit of a cop out. I like BTF map definitions,
and I want them to be as unsurprising as possible, so that they are
easy to use and adopt.
If a type encodes all the information we need via the array dimension hack,
couldn't we make the map variable itself a pointer, and drop the inner pointers?
struct my_map_def {
int type[BPF_MAP_TYPE_HASH];
int value;
struct foo key;
...
}
struct my_map_def *my_map;
--
Lorenz Bauer | Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
www.cloudflare.com
