On 10/28/21 2:14 PM, Martin KaFai Lau wrote:

On Wed, Oct 27, 2021 at 04:45:00PM -0700, Joanne Koong wrote:

[...]

quoted

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.

quoted

  	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")

quoted

+	__u64 map_extra;
  } __attribute__((aligned(8)));

[ ... ]

quoted

+static int peek_elem(struct bpf_map *map, void *value)

These generic map-ops names could be confusing in tracing and
in perf-report.  There was a 'bloom_filter_map_' prefix in the earlier version.
I could have missed something in the earlier discussion threads.
What was the reason in dropping the prefix?

The reason I dropped the prefix was so that the function names would be
less verbose. Your point about it being confusing in tracing and in 
perf-report
makes a lot of sense - I will add it back in!

[...]