diff mbox series

[v4,bpf-next,03/14] bpf: Add BTF_ID_LIST/BTF_ID macros

Message ID 20200625221304.2817194-4-jolsa@kernel.org
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series bpf: Add d_path helper | expand

Commit Message

Jiri Olsa June 25, 2020, 10:12 p.m. UTC
Adding support to generate .BTF_ids section that will hold BTF
ID lists for verifier.

Adding macros that will help to define lists of BTF ID values
placed in .BTF_ids section. They are initially filled with zeros
(during compilation) and resolved later during the linking phase
by resolve_btfids tool.

Following defines list of one BTF ID value:

  BTF_ID_LIST(bpf_skb_output_btf_ids)
  BTF_ID(struct, sk_buff)

It also defines following variable to access the list:

  extern int bpf_skb_output_btf_ids[];

Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/asm-generic/vmlinux.lds.h |  4 ++
 include/linux/btf_ids.h           | 69 +++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 include/linux/btf_ids.h

Comments

Andrii Nakryiko June 26, 2020, 9:32 p.m. UTC | #1
On Thu, Jun 25, 2020 at 3:13 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding support to generate .BTF_ids section that will hold BTF
> ID lists for verifier.
>
> Adding macros that will help to define lists of BTF ID values
> placed in .BTF_ids section. They are initially filled with zeros
> (during compilation) and resolved later during the linking phase
> by resolve_btfids tool.
>
> Following defines list of one BTF ID value:
>
>   BTF_ID_LIST(bpf_skb_output_btf_ids)
>   BTF_ID(struct, sk_buff)
>
> It also defines following variable to access the list:
>
>   extern int bpf_skb_output_btf_ids[];
>
> Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---

Looks good, with few nits below.

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  include/asm-generic/vmlinux.lds.h |  4 ++
>  include/linux/btf_ids.h           | 69 +++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
>  create mode 100644 include/linux/btf_ids.h
>

[...]

> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> new file mode 100644
> index 000000000000..f7f9dc4d9a9f
> --- /dev/null
> +++ b/include/linux/btf_ids.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _LINUX_BTF_IDS_H
> +#define _LINUX_BTF_IDS_H 1

this "1", is it necessary? I think it's always just `#define HEADER_GUARD`?

> +
> +#include <linux/compiler.h> /* for __PASTE */
> +

[...]

> +#define __BTF_ID_LIST(name)                            \
> +asm(                                                   \
> +".pushsection " BTF_IDS_SECTION ",\"a\";       \n"     \
> +".local " #name ";                             \n"     \
> +#name ":;                                      \n"     \
> +".popsection;                                  \n");   \
> +
> +#define BTF_ID_LIST(name)                              \
> +__BTF_ID_LIST(name)                                    \
> +extern int name[];

nit: extern u32 (or __u32) perhaps?

> +
> +#endif
> --
> 2.25.4
>
Jiri Olsa June 28, 2020, 7:50 p.m. UTC | #2
On Fri, Jun 26, 2020 at 02:32:48PM -0700, Andrii Nakryiko wrote:

SNIP

> 
> [...]
> 
> > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> > new file mode 100644
> > index 000000000000..f7f9dc4d9a9f
> > --- /dev/null
> > +++ b/include/linux/btf_ids.h
> > @@ -0,0 +1,69 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef _LINUX_BTF_IDS_H
> > +#define _LINUX_BTF_IDS_H 1
> 
> this "1", is it necessary? I think it's always just `#define HEADER_GUARD`?

I followed btf.h header:

#ifndef _LINUX_BTF_H
#define _LINUX_BTF_H 1

I don't mind changing it

> 
> > +
> > +#include <linux/compiler.h> /* for __PASTE */
> > +
> 
> [...]
> 
> > +#define __BTF_ID_LIST(name)                            \
> > +asm(                                                   \
> > +".pushsection " BTF_IDS_SECTION ",\"a\";       \n"     \
> > +".local " #name ";                             \n"     \
> > +#name ":;                                      \n"     \
> > +".popsection;                                  \n");   \
> > +
> > +#define BTF_ID_LIST(name)                              \
> > +__BTF_ID_LIST(name)                                    \
> > +extern int name[];
> 
> nit: extern u32 (or __u32) perhaps?

right, should be u32 

thanks,
jirka
diff mbox series

Patch

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index db600ef218d7..a9b77a4314a8 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -641,6 +641,10 @@ 
 		__start_BTF = .;					\
 		*(.BTF)							\
 		__stop_BTF = .;						\
+	}								\
+	. = ALIGN(4);							\
+	.BTF.ids : AT(ADDR(.BTF.ids) - LOAD_OFFSET) {			\
+		*(.BTF.ids)						\
 	}
 #else
 #define BTF
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
new file mode 100644
index 000000000000..f7f9dc4d9a9f
--- /dev/null
+++ b/include/linux/btf_ids.h
@@ -0,0 +1,69 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_BTF_IDS_H
+#define _LINUX_BTF_IDS_H 1
+
+#include <linux/compiler.h> /* for __PASTE */
+
+/*
+ * Following macros help to define lists of BTF IDs placed
+ * in .BTF_ids section. They are initially filled with zeros
+ * (during compilation) and resolved later during the
+ * linking phase by btfid tool.
+ *
+ * Any change in list layout must be reflected in btfid
+ * tool logic.
+ */
+
+#define BTF_IDS_SECTION ".BTF.ids"
+
+#define ____BTF_ID(symbol)				\
+asm(							\
+".pushsection " BTF_IDS_SECTION ",\"a\";       \n"	\
+".local " #symbol " ;                          \n"	\
+".type  " #symbol ", @object;                  \n"	\
+".size  " #symbol ", 4;                        \n"	\
+#symbol ":                                     \n"	\
+".zero 4                                       \n"	\
+".popsection;                                  \n");
+
+#define __BTF_ID(symbol) \
+	____BTF_ID(symbol)
+
+#define __ID(prefix) \
+	__PASTE(prefix, __COUNTER__)
+
+/*
+ * The BTF_ID defines unique symbol for each ID pointing
+ * to 4 zero bytes.
+ */
+#define BTF_ID(prefix, name) \
+	__BTF_ID(__ID(__BTF_ID__##prefix##__##name##__))
+
+/*
+ * The BTF_ID_LIST macro defines pure (unsorted) list
+ * of BTF IDs, with following layout:
+ *
+ * BTF_ID_LIST(list1)
+ * BTF_ID(type1, name1)
+ * BTF_ID(type2, name2)
+ *
+ * list1:
+ * __BTF_ID__type1__name1__1:
+ * .zero 4
+ * __BTF_ID__type2__name2__2:
+ * .zero 4
+ *
+ */
+#define __BTF_ID_LIST(name)				\
+asm(							\
+".pushsection " BTF_IDS_SECTION ",\"a\";       \n"	\
+".local " #name ";                             \n"	\
+#name ":;                                      \n"	\
+".popsection;                                  \n");	\
+
+#define BTF_ID_LIST(name)				\
+__BTF_ID_LIST(name)					\
+extern int name[];
+
+#endif