diff mbox series

[bpf-next,1/1] bpf: Mark virtual BPF context structures as preserve_static_offset

Message ID 20231208000531.19179-2-eddyz87@gmail.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series use preserve_static_offset in bpf uapi headers | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/ynl success SINGLE THREAD; Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 7874 this patch: 7874
netdev/cc_maintainers warning 13 maintainers not CCed: kpsingh@kernel.org nathan@kernel.org sdf@google.com netdev@vger.kernel.org john.fastabend@gmail.com ndesaulniers@google.com llvm@lists.linux.dev song@kernel.org kuba@kernel.org haoluo@google.com hawk@kernel.org jolsa@kernel.org trix@redhat.com
netdev/build_clang success Errors and warnings before: 2577 this patch: 2577
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8409 this patch: 8409
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 234 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-9 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-16 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-llvm-16 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-16 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-16 / veristat
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-llvm-16 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-llvm-16 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-3 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-llvm-16 / build / build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-llvm-16 / veristat
bpf/vmtest-bpf-next-VM_Test-11 success Logs for x86_64-gcc / build / build for x86_64 with gcc

Commit Message

Eduard Zingerman Dec. 8, 2023, 12:05 a.m. UTC 
Add __attribute__((preserve_static_offset)) for the following BPF
related structures:
- __sk_buff
- bpf_cgroup_dev_ctx
- bpf_perf_event_data
- bpf_sk_lookup
- bpf_sock
- bpf_sock_addr
- bpf_sock_ops
- bpf_sockopt
- bpf_sysctl
- sk_msg_md
- sk_reuseport_md
- xdp_md

Access to these structures is rewritten by BPF verifier.
(See verifier.c:convert_ctx_access).
The rewrite requires that offsets used in access to fields of these
structures are constant values. __attribute__((preserve_static_offset))
is a hint to clang that ensures that constant offsets are used.
(See https://reviews.llvm.org/D133361 for details).

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 include/uapi/linux/bpf.h                  | 28 ++++++++++++++---------
 include/uapi/linux/bpf_perf_event.h       |  8 ++++++-
 tools/include/uapi/linux/bpf.h            | 28 ++++++++++++++---------
 tools/include/uapi/linux/bpf_perf_event.h |  8 ++++++-
 4 files changed, 48 insertions(+), 24 deletions(-)

Comments

Yonghong Song Dec. 8, 2023, 3:36 a.m. UTC | #1 
On 12/7/23 4:05 PM, Eduard Zingerman wrote:
> Add __attribute__((preserve_static_offset)) for the following BPF
> related structures:
> - __sk_buff
> - bpf_cgroup_dev_ctx
> - bpf_perf_event_data
> - bpf_sk_lookup
> - bpf_sock
> - bpf_sock_addr
> - bpf_sock_ops
> - bpf_sockopt
> - bpf_sysctl
> - sk_msg_md
> - sk_reuseport_md
> - xdp_md
>
> Access to these structures is rewritten by BPF verifier.
> (See verifier.c:convert_ctx_access).
> The rewrite requires that offsets used in access to fields of these
> structures are constant values. __attribute__((preserve_static_offset))
> is a hint to clang that ensures that constant offsets are used.
> (See https://reviews.llvm.org/D133361 for details).
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
>   include/uapi/linux/bpf.h                  | 28 ++++++++++++++---------
>   include/uapi/linux/bpf_perf_event.h       |  8 ++++++-
>   tools/include/uapi/linux/bpf.h            | 28 ++++++++++++++---------
>   tools/include/uapi/linux/bpf_perf_event.h |  8 ++++++-
>   4 files changed, 48 insertions(+), 24 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index e0545201b55f..75eee56ed732 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -69,6 +69,12 @@ enum {
>   /* BPF has 10 general purpose 64-bit registers and stack frame. */
>   #define MAX_BPF_REG	__MAX_BPF_REG
>   
> +#if __has_attribute(preserve_static_offset) && defined(__bpf__)
> +#define __bpf_ctx __attribute__((preserve_static_offset))
> +#else
> +#define __bpf_ctx
> +#endif
> +
>   struct bpf_insn {
>   	__u8	code;		/* opcode */
>   	__u8	dst_reg:4;	/* dest register */
> @@ -6190,7 +6196,7 @@ struct __sk_buff {
>   	__u8  tstamp_type;
>   	__u32 :24;		/* Padding, future use. */
>   	__u64 hwtstamp;
> -};
> +} __bpf_ctx;
>   
>   struct bpf_tunnel_key {
>   	__u32 tunnel_id;
> @@ -6271,7 +6277,7 @@ struct bpf_sock {
>   	__u32 dst_ip6[4];
>   	__u32 state;
>   	__s32 rx_queue_mapping;
> -};
> +} __bpf_ctx;
>   
>   struct bpf_tcp_sock {
>   	__u32 snd_cwnd;		/* Sending congestion window		*/
> @@ -6379,7 +6385,7 @@ struct xdp_md {
>   	__u32 rx_queue_index;  /* rxq->queue_index  */
>   
>   	__u32 egress_ifindex;  /* txq->dev->ifindex */
> -};
> +} __bpf_ctx;
>   
>   /* DEVMAP map-value layout
>    *
> @@ -6429,7 +6435,7 @@ struct sk_msg_md {
>   	__u32 size;		/* Total size of sk_msg */
>   
>   	__bpf_md_ptr(struct bpf_sock *, sk); /* current socket */
> -};
> +} __bpf_ctx;
>   
>   struct sk_reuseport_md {
>   	/*
> @@ -6468,7 +6474,7 @@ struct sk_reuseport_md {
>   	 */
>   	__bpf_md_ptr(struct bpf_sock *, sk);
>   	__bpf_md_ptr(struct bpf_sock *, migrating_sk);
> -};
> +} __bpf_ctx;
>   
>   #define BPF_TAG_SIZE	8
>   
> @@ -6678,7 +6684,7 @@ struct bpf_sock_addr {
>   				 * Stored in network byte order.
>   				 */
>   	__bpf_md_ptr(struct bpf_sock *, sk);
> -};
> +} __bpf_ctx;
>   
>   /* User bpf_sock_ops struct to access socket values and specify request ops
>    * and their replies.
> @@ -6761,7 +6767,7 @@ struct bpf_sock_ops {
>   				 * been written yet.
>   				 */
>   	__u64 skb_hwtstamp;
> -};
> +} __bpf_ctx;
>   
>   /* Definitions for bpf_sock_ops_cb_flags */
>   enum {
> @@ -7034,7 +7040,7 @@ struct bpf_cgroup_dev_ctx {
>   	__u32 access_type;
>   	__u32 major;
>   	__u32 minor;
> -};
> +} __bpf_ctx;
>   
>   struct bpf_raw_tracepoint_args {
>   	__u64 args[0];
> @@ -7245,7 +7251,7 @@ struct bpf_sysctl {
>   	__u32	file_pos;	/* Sysctl file position to read from, write to.
>   				 * Allows 1,2,4-byte read an 4-byte write.
>   				 */
> -};
> +} __bpf_ctx;
>   
>   struct bpf_sockopt {
>   	__bpf_md_ptr(struct bpf_sock *, sk);
> @@ -7256,7 +7262,7 @@ struct bpf_sockopt {
>   	__s32	optname;
>   	__s32	optlen;
>   	__s32	retval;
> -};
> +} __bpf_ctx;
>   
>   struct bpf_pidns_info {
>   	__u32 pid;
> @@ -7280,7 +7286,7 @@ struct bpf_sk_lookup {
>   	__u32 local_ip6[4];	/* Network byte order */
>   	__u32 local_port;	/* Host byte order */
>   	__u32 ingress_ifindex;		/* The arriving interface. Determined by inet_iif. */
> -};
> +} __bpf_ctx;

should we undef __bpf_ctx at the end of the file?
The same for below bpf_perf_event.h file.

>   
>   /*
>    * struct btf_ptr is used for typed pointer representation; the
> diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h
> index eb1b9d21250c..cf614ddf0381 100644
> --- a/include/uapi/linux/bpf_perf_event.h
> +++ b/include/uapi/linux/bpf_perf_event.h
> @@ -10,10 +10,16 @@
>   
>   #include <asm/bpf_perf_event.h>
>   
> +#if __has_attribute(preserve_static_offset) && defined(__bpf__)
> +#define __bpf_ctx __attribute__((preserve_static_offset))
> +#else
> +#define __bpf_ctx
> +#endif
> +
>   struct bpf_perf_event_data {
>   	bpf_user_pt_regs_t regs;
>   	__u64 sample_period;
>   	__u64 addr;
> -};
> +} __bpf_ctx;
>   
>   #endif /* _UAPI__LINUX_BPF_PERF_EVENT_H__ */
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index e0545201b55f..75eee56ed732 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h

[...]
Eduard Zingerman Dec. 8, 2023, 2:23 p.m. UTC | #2 
On Thu, 2023-12-07 at 19:36 -0800, Yonghong Song wrote:
[...]

> >   struct bpf_pidns_info {
> >   	__u32 pid;
> > @@ -7280,7 +7286,7 @@ struct bpf_sk_lookup {
> >   	__u32 local_ip6[4];	/* Network byte order */
> >   	__u32 local_port;	/* Host byte order */
> >   	__u32 ingress_ifindex;		/* The arriving interface. Determined by inet_iif. */
> > -};
> > +} __bpf_ctx;
> 
> should we undef __bpf_ctx at the end of the file?
> The same for below bpf_perf_event.h file.

Yes, that makes sense, thank you.
diff mbox series

Patch

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e0545201b55f..75eee56ed732 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -69,6 +69,12 @@  enum {
 /* BPF has 10 general purpose 64-bit registers and stack frame. */
 #define MAX_BPF_REG	__MAX_BPF_REG
 
+#if __has_attribute(preserve_static_offset) && defined(__bpf__)
+#define __bpf_ctx __attribute__((preserve_static_offset))
+#else
+#define __bpf_ctx
+#endif
+
 struct bpf_insn {
 	__u8	code;		/* opcode */
 	__u8	dst_reg:4;	/* dest register */
@@ -6190,7 +6196,7 @@  struct __sk_buff {
 	__u8  tstamp_type;
 	__u32 :24;		/* Padding, future use. */
 	__u64 hwtstamp;
-};
+} __bpf_ctx;
 
 struct bpf_tunnel_key {
 	__u32 tunnel_id;
@@ -6271,7 +6277,7 @@  struct bpf_sock {
 	__u32 dst_ip6[4];
 	__u32 state;
 	__s32 rx_queue_mapping;
-};
+} __bpf_ctx;
 
 struct bpf_tcp_sock {
 	__u32 snd_cwnd;		/* Sending congestion window		*/
@@ -6379,7 +6385,7 @@  struct xdp_md {
 	__u32 rx_queue_index;  /* rxq->queue_index  */
 
 	__u32 egress_ifindex;  /* txq->dev->ifindex */
-};
+} __bpf_ctx;
 
 /* DEVMAP map-value layout
  *
@@ -6429,7 +6435,7 @@  struct sk_msg_md {
 	__u32 size;		/* Total size of sk_msg */
 
 	__bpf_md_ptr(struct bpf_sock *, sk); /* current socket */
-};
+} __bpf_ctx;
 
 struct sk_reuseport_md {
 	/*
@@ -6468,7 +6474,7 @@  struct sk_reuseport_md {
 	 */
 	__bpf_md_ptr(struct bpf_sock *, sk);
 	__bpf_md_ptr(struct bpf_sock *, migrating_sk);
-};
+} __bpf_ctx;
 
 #define BPF_TAG_SIZE	8
 
@@ -6678,7 +6684,7 @@  struct bpf_sock_addr {
 				 * Stored in network byte order.
 				 */
 	__bpf_md_ptr(struct bpf_sock *, sk);
-};
+} __bpf_ctx;
 
 /* User bpf_sock_ops struct to access socket values and specify request ops
  * and their replies.
@@ -6761,7 +6767,7 @@  struct bpf_sock_ops {
 				 * been written yet.
 				 */
 	__u64 skb_hwtstamp;
-};
+} __bpf_ctx;
 
 /* Definitions for bpf_sock_ops_cb_flags */
 enum {
@@ -7034,7 +7040,7 @@  struct bpf_cgroup_dev_ctx {
 	__u32 access_type;
 	__u32 major;
 	__u32 minor;
-};
+} __bpf_ctx;
 
 struct bpf_raw_tracepoint_args {
 	__u64 args[0];
@@ -7245,7 +7251,7 @@  struct bpf_sysctl {
 	__u32	file_pos;	/* Sysctl file position to read from, write to.
 				 * Allows 1,2,4-byte read an 4-byte write.
 				 */
-};
+} __bpf_ctx;
 
 struct bpf_sockopt {
 	__bpf_md_ptr(struct bpf_sock *, sk);
@@ -7256,7 +7262,7 @@  struct bpf_sockopt {
 	__s32	optname;
 	__s32	optlen;
 	__s32	retval;
-};
+} __bpf_ctx;
 
 struct bpf_pidns_info {
 	__u32 pid;
@@ -7280,7 +7286,7 @@  struct bpf_sk_lookup {
 	__u32 local_ip6[4];	/* Network byte order */
 	__u32 local_port;	/* Host byte order */
 	__u32 ingress_ifindex;		/* The arriving interface. Determined by inet_iif. */
-};
+} __bpf_ctx;
 
 /*
  * struct btf_ptr is used for typed pointer representation; the
diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h
index eb1b9d21250c..cf614ddf0381 100644
--- a/include/uapi/linux/bpf_perf_event.h
+++ b/include/uapi/linux/bpf_perf_event.h
@@ -10,10 +10,16 @@ 
 
 #include <asm/bpf_perf_event.h>
 
+#if __has_attribute(preserve_static_offset) && defined(__bpf__)
+#define __bpf_ctx __attribute__((preserve_static_offset))
+#else
+#define __bpf_ctx
+#endif
+
 struct bpf_perf_event_data {
 	bpf_user_pt_regs_t regs;
 	__u64 sample_period;
 	__u64 addr;
-};
+} __bpf_ctx;
 
 #endif /* _UAPI__LINUX_BPF_PERF_EVENT_H__ */
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index e0545201b55f..75eee56ed732 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -69,6 +69,12 @@  enum {
 /* BPF has 10 general purpose 64-bit registers and stack frame. */
 #define MAX_BPF_REG	__MAX_BPF_REG
 
+#if __has_attribute(preserve_static_offset) && defined(__bpf__)
+#define __bpf_ctx __attribute__((preserve_static_offset))
+#else
+#define __bpf_ctx
+#endif
+
 struct bpf_insn {
 	__u8	code;		/* opcode */
 	__u8	dst_reg:4;	/* dest register */
@@ -6190,7 +6196,7 @@  struct __sk_buff {
 	__u8  tstamp_type;
 	__u32 :24;		/* Padding, future use. */
 	__u64 hwtstamp;
-};
+} __bpf_ctx;
 
 struct bpf_tunnel_key {
 	__u32 tunnel_id;
@@ -6271,7 +6277,7 @@  struct bpf_sock {
 	__u32 dst_ip6[4];
 	__u32 state;
 	__s32 rx_queue_mapping;
-};
+} __bpf_ctx;
 
 struct bpf_tcp_sock {
 	__u32 snd_cwnd;		/* Sending congestion window		*/
@@ -6379,7 +6385,7 @@  struct xdp_md {
 	__u32 rx_queue_index;  /* rxq->queue_index  */
 
 	__u32 egress_ifindex;  /* txq->dev->ifindex */
-};
+} __bpf_ctx;
 
 /* DEVMAP map-value layout
  *
@@ -6429,7 +6435,7 @@  struct sk_msg_md {
 	__u32 size;		/* Total size of sk_msg */
 
 	__bpf_md_ptr(struct bpf_sock *, sk); /* current socket */
-};
+} __bpf_ctx;
 
 struct sk_reuseport_md {
 	/*
@@ -6468,7 +6474,7 @@  struct sk_reuseport_md {
 	 */
 	__bpf_md_ptr(struct bpf_sock *, sk);
 	__bpf_md_ptr(struct bpf_sock *, migrating_sk);
-};
+} __bpf_ctx;
 
 #define BPF_TAG_SIZE	8
 
@@ -6678,7 +6684,7 @@  struct bpf_sock_addr {
 				 * Stored in network byte order.
 				 */
 	__bpf_md_ptr(struct bpf_sock *, sk);
-};
+} __bpf_ctx;
 
 /* User bpf_sock_ops struct to access socket values and specify request ops
  * and their replies.
@@ -6761,7 +6767,7 @@  struct bpf_sock_ops {
 				 * been written yet.
 				 */
 	__u64 skb_hwtstamp;
-};
+} __bpf_ctx;
 
 /* Definitions for bpf_sock_ops_cb_flags */
 enum {
@@ -7034,7 +7040,7 @@  struct bpf_cgroup_dev_ctx {
 	__u32 access_type;
 	__u32 major;
 	__u32 minor;
-};
+} __bpf_ctx;
 
 struct bpf_raw_tracepoint_args {
 	__u64 args[0];
@@ -7245,7 +7251,7 @@  struct bpf_sysctl {
 	__u32	file_pos;	/* Sysctl file position to read from, write to.
 				 * Allows 1,2,4-byte read an 4-byte write.
 				 */
-};
+} __bpf_ctx;
 
 struct bpf_sockopt {
 	__bpf_md_ptr(struct bpf_sock *, sk);
@@ -7256,7 +7262,7 @@  struct bpf_sockopt {
 	__s32	optname;
 	__s32	optlen;
 	__s32	retval;
-};
+} __bpf_ctx;
 
 struct bpf_pidns_info {
 	__u32 pid;
@@ -7280,7 +7286,7 @@  struct bpf_sk_lookup {
 	__u32 local_ip6[4];	/* Network byte order */
 	__u32 local_port;	/* Host byte order */
 	__u32 ingress_ifindex;		/* The arriving interface. Determined by inet_iif. */
-};
+} __bpf_ctx;
 
 /*
  * struct btf_ptr is used for typed pointer representation; the
diff --git a/tools/include/uapi/linux/bpf_perf_event.h b/tools/include/uapi/linux/bpf_perf_event.h
index eb1b9d21250c..cf614ddf0381 100644
--- a/tools/include/uapi/linux/bpf_perf_event.h
+++ b/tools/include/uapi/linux/bpf_perf_event.h
@@ -10,10 +10,16 @@ 
 
 #include <asm/bpf_perf_event.h>
 
+#if __has_attribute(preserve_static_offset) && defined(__bpf__)
+#define __bpf_ctx __attribute__((preserve_static_offset))
+#else
+#define __bpf_ctx
+#endif
+
 struct bpf_perf_event_data {
 	bpf_user_pt_regs_t regs;
 	__u64 sample_period;
 	__u64 addr;
-};
+} __bpf_ctx;
 
 #endif /* _UAPI__LINUX_BPF_PERF_EVENT_H__ */