Re: [PATCH v6 bpf-next 1/5] bpf: Add bloom filter map implementation

[Martin KaFai Lau <kafai@xxxxxx>]

On Thu, Oct 28, 2021 at 08:17:23PM -0700, Joanne Koong wrote:
> On 10/28/21 2:14 PM, Martin KaFai Lau wrote:
> 
> > On Wed, Oct 27, 2021 at 04:45:00PM -0700, Joanne Koong wrote:
> [...]
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index c10820037883..8bead4aa3ad0 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -906,6 +906,7 @@ enum bpf_map_type {
> > >   	BPF_MAP_TYPE_RINGBUF,
> > >   	BPF_MAP_TYPE_INODE_STORAGE,
> > >   	BPF_MAP_TYPE_TASK_STORAGE,
> > > +	BPF_MAP_TYPE_BLOOM_FILTER,
> > >   };
> > >   /* Note that tracing related programs such as
> > > @@ -1274,6 +1275,13 @@ union bpf_attr {
> > >   						   * struct stored as the
> > >   						   * map value
> > >   						   */
> > > +		/* Any per-map-type extra fields
> > > +		 *
> > > +		 * BPF_MAP_TYPE_BLOOM_FILTER - the lowest 4 bits indicate the
> > > +		 * number of hash functions (if 0, the bloom filter will default
> > > +		 * to using 5 hash functions).
> > > +		 */
> > > +		__u64	map_extra;
> > >   	};
> When I run pahole (on an x86-64 machine), I see that there's an 8 byte hole
> right before map_extra in the "union bpf_attr" struct (above this
> paragraph).
> It seems like this should be padded as well with a "__u64 :64;"? I will add
> that in.
hmm... I don't see it.

pahole tools/lib/bpf/libbpf.a:

union bpf_attr {
	struct {
		__u32              map_type;           /*     0     4 */
		__u32              key_size;           /*     4     4 */
		__u32              value_size;         /*     8     4 */
		__u32              max_entries;        /*    12     4 */
		__u32              map_flags;          /*    16     4 */
		__u32              inner_map_fd;       /*    20     4 */
		__u32              numa_node;          /*    24     4 */
		char               map_name[16];       /*    28    16 */
		__u32              map_ifindex;        /*    44     4 */
		__u32              btf_fd;             /*    48     4 */
		__u32              btf_key_type_id;    /*    52     4 */
		__u32              btf_value_type_id;  /*    56     4 */
		__u32              btf_vmlinux_value_type_id; /*    60     4 */
		/* --- cacheline 1 boundary (64 bytes) --- */
		__u64              map_extra;          /*    64     8 */
	};                                             /*     0    72 */

or you meant another struct/union?

> > >   	struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
> > > @@ -5638,6 +5646,7 @@ struct bpf_map_info {
> > >   	__u32 btf_id;
> > >   	__u32 btf_key_type_id;
> > >   	__u32 btf_value_type_id;
> > There is also a 4 byte hole here.  A "__u32 :32" is needed.
> > You can find details in 36f9814a494a ("bpf: fix uapi hole for 32 bit compat applications")
> > 
> > > +	__u64 map_extra;
> > >   } __attribute__((aligned(8)));