Re: [PATCH bpf-next 4/6] bpf: Support access to bpf map fields

[Martin KaFai Lau <kafai@fb.com>]

On Thu, Jun 18, 2020 at 12:42:36PM -0700, Andrey Ignatov wrote:
> Martin KaFai Lau <kafai@fb.com> [Wed, 2020-06-17 23:18 -0700]:
> > On Wed, Jun 17, 2020 at 01:43:45PM -0700, Andrey Ignatov wrote:
> > [ ... ]
> > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > > index e5c5305e859c..fa21b1e766ae 100644
> > > --- a/kernel/bpf/btf.c
> > > +++ b/kernel/bpf/btf.c
> > > @@ -3577,6 +3577,67 @@ btf_get_prog_ctx_type(struct bpf_verifier_log *log, struct btf *btf,
> > >  	return ctx_type;
> > >  }
> > >  
> > > +#define BPF_PROG_TYPE(_id, _name, prog_ctx_type, kern_ctx_type)
> > > +#define BPF_LINK_TYPE(_id, _name)
> > > +static const struct bpf_map_ops * const btf_vmlinux_map_ops[] = {
> > > +#define BPF_MAP_TYPE(_id, _ops) \
> > > +	[_id] = &_ops,
> > > +#include <linux/bpf_types.h>
> > > +#undef BPF_MAP_TYPE
> > > +};
> > > +static u32 btf_vmlinux_map_ids[] = {
> > > +#define BPF_MAP_TYPE(_id, _ops) \
> > > +	[_id] = (u32)-1,
> > > +#include <linux/bpf_types.h>
> > > +#undef BPF_MAP_TYPE
> > > +};
> > > +#undef BPF_PROG_TYPE
> > > +#undef BPF_LINK_TYPE
> > > +
> > > +static int btf_vmlinux_map_ids_init(const struct btf *btf,
> > > +				    struct bpf_verifier_log *log)
> > > +{
> > > +	int base_btf_id, btf_id, i;
> > > +	const char *btf_name;
> > > +
> > > +	base_btf_id = btf_find_by_name_kind(btf, "bpf_map", BTF_KIND_STRUCT);
> > > +	if (base_btf_id < 0)
> > > +		return base_btf_id;
> > > +
> > > +	BUILD_BUG_ON(ARRAY_SIZE(btf_vmlinux_map_ops) !=
> > > +		     ARRAY_SIZE(btf_vmlinux_map_ids));
> > > +
> > > +	for (i = 0; i < ARRAY_SIZE(btf_vmlinux_map_ops); ++i) {
> > > +		if (!btf_vmlinux_map_ops[i])
> > > +			continue;
> > > +		btf_name = btf_vmlinux_map_ops[i]->map_btf_name;
> > > +		if (!btf_name) {
> > > +			btf_vmlinux_map_ids[i] = base_btf_id;
> > > +			continue;
> > > +		}
> > > +		btf_id = btf_find_by_name_kind(btf, btf_name, BTF_KIND_STRUCT);
> > > +		if (btf_id < 0)
> > > +			return btf_id;
> > > +		btf_vmlinux_map_ids[i] = btf_id;
> > Since map_btf_name is already in map_ops, how about storing map_btf_id in
> > map_ops also?
> > btf_id 0 is "void" which is as good as -1, so there is no need
> > to modify all map_ops to init map_btf_id to -1.
> 
> Yeah, btf_id == 0 being a valid id was the reason I used -1.
> 
> I realized that having a map type specific struct with btf_id == 0
> should be practically impossible, but is it guaranteed to always be
> "void" or it just happened so and can change in the future?
It is always "void" and cannot be changed in the future.