On Thu, Oct 13, 2022 at 11:52:47AM +0530, Kumar Kartikeya Dwivedi wrote:
> Add the basic support on the map side to parse, recognize, verify, and
> build metadata table for a new special field of the type struct
> bpf_list_head. To parameterize the bpf_list_head for a certain value
> type and the list_node member it will accept in that value type, we use
> BTF declaration tags.
>
> The definition of bpf_list_head in a map value will be done as follows:
>
> struct foo {
> struct bpf_list_node node;
> int data;
> };
>
> struct map_value {
> struct bpf_list_head head __contains(foo, node);
> };
>
> Then, the bpf_list_head only allows adding to the list 'head' using the
> bpf_list_node 'node' for the type struct foo.
>
> The 'contains' annotation is a BTF declaration tag composed of four
> parts, "contains:kind:name:node" where the kind and name is then used to
> look up the type in the map BTF. The node defines name of the member in
> this type that has the type struct bpf_list_node, which is actually used
> for linking into the linked list. For now, 'kind' part is hardcoded as
> struct.
...
> + value_type = btf_find_decl_tag_value(btf, pt, comp_idx, "contains:");
> + if (!value_type)
> + return -EINVAL;
> + if (strncmp(value_type, "struct:", sizeof("struct:") - 1))
> + return -EINVAL;
> + value_type += sizeof("struct:") - 1;
I don't get it.
The patch 24 does:
+#define __contains(name, node) __attribute__((btf_decl_tag("contains:struct:" #name ":" #node)))
The 'struct:' part is invisible to users. They won't make a mistake.
Why bother adding it to BTF and then check for it?
Backward compat concerns?
But it's in bpf_experimental.h.
That probably be the last thing to change and so easy to do.
Please drop it?
> diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
> new file mode 100644
> index 000000000000..4e31790e433d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/bpf_experimental.h
> @@ -0,0 +1,23 @@
> +#ifndef __KERNEL__
> +
> +#include <vmlinux.h>
> +#include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_core_read.h>
> +
Why bother with the above?
The below should be enough ?
> +#else
> +
> +struct bpf_list_head {
> + __u64 __a;
> + __u64 __b;
> +} __attribute__((aligned(8)));
> +
> +struct bpf_list_node {
> + __u64 __a;
> + __u64 __b;
> +} __attribute__((aligned(8)));
> +
> +#endif
> +
> +#ifndef __KERNEL__
> +#endif
hmm.
