linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf 2/2] bpf: enforce usage of __aligned_u64 in the UAPI header
@ 2018-05-27 11:28 Eugene Syromiatnikov
  2018-05-29 17:35 ` Song Liu
  0 siblings, 1 reply; 3+ messages in thread
From: Eugene Syromiatnikov @ 2018-05-27 11:28 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, Martin KaFai Lau, Daniel Borkmann,
	Alexei Starovoitov, David S. Miller, Jiri Olsa, Ingo Molnar,
	Lawrence Brakmo, Andrey Ignatov, Jakub Kicinski, John Fastabend,
	Dmitry V. Levin

Use __aligned_u64 instead of __u64 everywhere in the UAPI header,
similarly to v4.17-rc1~94^2~58^2 "RDMA: Change all uapi headers to use
__aligned_u64 instead of __u64".

This commit doesn't change structure layouts, but imposes additional
alignment requirements on struct bpf_stack_build_id, struct
bpf_sock_ops, struct bpf_perf_event_value, and struct
bpf_raw_tracepoint_args; of them only struct bpf_sock_ops and struct
bpf_perf_event_value have 64-bit fields that were present in a released
kernel without this additional alignment requirement (bytes_received
and bytes_acked were added to struct bpf_sock_ops in commit
v4.16-rc1~123^2~33^2~5^2~4, struct bpf_perf_event_value was added
in commit v4.15-rc1~84^2~532^2~3).

Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 include/uapi/linux/bpf.h       | 22 +++++++++++-----------
 tools/include/uapi/linux/bpf.h | 22 +++++++++++-----------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 903010a..174e99a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -259,8 +259,8 @@ struct bpf_stack_build_id {
 	__s32		status;
 	unsigned char	build_id[BPF_BUILD_ID_SIZE];
 	union {
-		__u64	offset;
-		__u64	ip;
+		__aligned_u64	offset;
+		__aligned_u64	ip;
 	};
 };
 
@@ -288,7 +288,7 @@ union bpf_attr {
 			__aligned_u64 value;
 			__aligned_u64 next_key;
 		};
-		__u64		flags;
+		__aligned_u64	flags;
 	};
 
 	struct { /* anonymous struct used by BPF_PROG_LOAD command */
@@ -360,7 +360,7 @@ union bpf_attr {
 	} query;
 
 	struct {
-		__u64 name;
+		__aligned_u64 name;
 		__u32 prog_fd;
 	} raw_tracepoint;
 } __attribute__((aligned(8)));
@@ -1011,7 +1011,7 @@ struct bpf_prog_info {
 	__u32 xlated_prog_len;
 	__aligned_u64 jited_prog_insns;
 	__aligned_u64 xlated_prog_insns;
-	__u64 load_time;	/* ns since boottime */
+	__aligned_u64 load_time;	/* ns since boottime */
 	__u32 created_by_uid;
 	__u32 nr_map_ids;
 	__aligned_u64 map_ids;
@@ -1101,8 +1101,8 @@ struct bpf_sock_ops {
 	__u32 lost_out;
 	__u32 sacked_out;
 	__u32 sk_txhash;
-	__u64 bytes_received;
-	__u64 bytes_acked;
+	__aligned_u64 bytes_received;
+	__aligned_u64 bytes_acked;
 };
 
 /* Definitions for bpf_sock_ops_cb_flags */
@@ -1189,9 +1189,9 @@ enum {
 #define TCP_BPF_SNDCWND_CLAMP	1002	/* Set sndcwnd_clamp */
 
 struct bpf_perf_event_value {
-	__u64 counter;
-	__u64 enabled;
-	__u64 running;
+	__aligned_u64 counter;
+	__aligned_u64 enabled;
+	__aligned_u64 running;
 };
 
 #define BPF_DEVCG_ACC_MKNOD	(1ULL << 0)
@@ -1209,7 +1209,7 @@ struct bpf_cgroup_dev_ctx {
 };
 
 struct bpf_raw_tracepoint_args {
-	__u64 args[0];
+	__aligned_u64 args[0];
 };
 
 #endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 903010a..174e99a 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -259,8 +259,8 @@ struct bpf_stack_build_id {
 	__s32		status;
 	unsigned char	build_id[BPF_BUILD_ID_SIZE];
 	union {
-		__u64	offset;
-		__u64	ip;
+		__aligned_u64	offset;
+		__aligned_u64	ip;
 	};
 };
 
@@ -288,7 +288,7 @@ union bpf_attr {
 			__aligned_u64 value;
 			__aligned_u64 next_key;
 		};
-		__u64		flags;
+		__aligned_u64	flags;
 	};
 
 	struct { /* anonymous struct used by BPF_PROG_LOAD command */
@@ -360,7 +360,7 @@ union bpf_attr {
 	} query;
 
 	struct {
-		__u64 name;
+		__aligned_u64 name;
 		__u32 prog_fd;
 	} raw_tracepoint;
 } __attribute__((aligned(8)));
@@ -1011,7 +1011,7 @@ struct bpf_prog_info {
 	__u32 xlated_prog_len;
 	__aligned_u64 jited_prog_insns;
 	__aligned_u64 xlated_prog_insns;
-	__u64 load_time;	/* ns since boottime */
+	__aligned_u64 load_time;	/* ns since boottime */
 	__u32 created_by_uid;
 	__u32 nr_map_ids;
 	__aligned_u64 map_ids;
@@ -1101,8 +1101,8 @@ struct bpf_sock_ops {
 	__u32 lost_out;
 	__u32 sacked_out;
 	__u32 sk_txhash;
-	__u64 bytes_received;
-	__u64 bytes_acked;
+	__aligned_u64 bytes_received;
+	__aligned_u64 bytes_acked;
 };
 
 /* Definitions for bpf_sock_ops_cb_flags */
@@ -1189,9 +1189,9 @@ enum {
 #define TCP_BPF_SNDCWND_CLAMP	1002	/* Set sndcwnd_clamp */
 
 struct bpf_perf_event_value {
-	__u64 counter;
-	__u64 enabled;
-	__u64 running;
+	__aligned_u64 counter;
+	__aligned_u64 enabled;
+	__aligned_u64 running;
 };
 
 #define BPF_DEVCG_ACC_MKNOD	(1ULL << 0)
@@ -1209,7 +1209,7 @@ struct bpf_cgroup_dev_ctx {
 };
 
 struct bpf_raw_tracepoint_args {
-	__u64 args[0];
+	__aligned_u64 args[0];
 };
 
 #endif /* _UAPI__LINUX_BPF_H__ */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf 2/2] bpf: enforce usage of __aligned_u64 in the UAPI header
  2018-05-27 11:28 [PATCH bpf 2/2] bpf: enforce usage of __aligned_u64 in the UAPI header Eugene Syromiatnikov
@ 2018-05-29 17:35 ` Song Liu
  2018-05-30 10:03   ` Eugene Syromiatnikov
  0 siblings, 1 reply; 3+ messages in thread
From: Song Liu @ 2018-05-29 17:35 UTC (permalink / raw)
  To: Eugene Syromiatnikov
  Cc: netdev, open list, Martin KaFai Lau, Daniel Borkmann,
	Alexei Starovoitov, David S. Miller, Jiri Olsa, Ingo Molnar,
	Lawrence Brakmo, Andrey Ignatov, Jakub Kicinski, John Fastabend,
	Dmitry V. Levin

On Sun, May 27, 2018 at 4:28 AM, Eugene Syromiatnikov <esyr@redhat.com> wrote:
> Use __aligned_u64 instead of __u64 everywhere in the UAPI header,
> similarly to v4.17-rc1~94^2~58^2 "RDMA: Change all uapi headers to use
> __aligned_u64 instead of __u64".
>
> This commit doesn't change structure layouts, but imposes additional
> alignment requirements on struct bpf_stack_build_id, struct
> bpf_sock_ops, struct bpf_perf_event_value, and struct
> bpf_raw_tracepoint_args; of them only struct bpf_sock_ops and struct
> bpf_perf_event_value have 64-bit fields that were present in a released
> kernel without this additional alignment requirement (bytes_received
> and bytes_acked were added to struct bpf_sock_ops in commit
> v4.16-rc1~123^2~33^2~5^2~4, struct bpf_perf_event_value was added
> in commit v4.15-rc1~84^2~532^2~3).
>
> Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
> ---
>  include/uapi/linux/bpf.h       | 22 +++++++++++-----------
>  tools/include/uapi/linux/bpf.h | 22 +++++++++++-----------
>  2 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 903010a..174e99a 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -259,8 +259,8 @@ struct bpf_stack_build_id {
>         __s32           status;
>         unsigned char   build_id[BPF_BUILD_ID_SIZE];
>         union {
> -               __u64   offset;
> -               __u64   ip;
> +               __aligned_u64   offset;
> +               __aligned_u64   ip;
>         };
>  };
>
> @@ -288,7 +288,7 @@ union bpf_attr {
>                         __aligned_u64 value;
>                         __aligned_u64 next_key;
>                 };
> -               __u64           flags;
> +               __aligned_u64   flags;
>         };
>
>         struct { /* anonymous struct used by BPF_PROG_LOAD command */
> @@ -360,7 +360,7 @@ union bpf_attr {
>         } query;
>
>         struct {
> -               __u64 name;
> +               __aligned_u64 name;
>                 __u32 prog_fd;
>         } raw_tracepoint;
>  } __attribute__((aligned(8)));
> @@ -1011,7 +1011,7 @@ struct bpf_prog_info {
>         __u32 xlated_prog_len;
>         __aligned_u64 jited_prog_insns;
>         __aligned_u64 xlated_prog_insns;
> -       __u64 load_time;        /* ns since boottime */
> +       __aligned_u64 load_time;        /* ns since boottime */
>         __u32 created_by_uid;
>         __u32 nr_map_ids;
>         __aligned_u64 map_ids;
> @@ -1101,8 +1101,8 @@ struct bpf_sock_ops {
>         __u32 lost_out;
>         __u32 sacked_out;
>         __u32 sk_txhash;
> -       __u64 bytes_received;
> -       __u64 bytes_acked;
> +       __aligned_u64 bytes_received;
> +       __aligned_u64 bytes_acked;
>  };
>
>  /* Definitions for bpf_sock_ops_cb_flags */
> @@ -1189,9 +1189,9 @@ enum {
>  #define TCP_BPF_SNDCWND_CLAMP  1002    /* Set sndcwnd_clamp */
>
>  struct bpf_perf_event_value {
> -       __u64 counter;
> -       __u64 enabled;
> -       __u64 running;
> +       __aligned_u64 counter;
> +       __aligned_u64 enabled;
> +       __aligned_u64 running;
>  };
>
>  #define BPF_DEVCG_ACC_MKNOD    (1ULL << 0)
> @@ -1209,7 +1209,7 @@ struct bpf_cgroup_dev_ctx {
>  };
>
>  struct bpf_raw_tracepoint_args {
> -       __u64 args[0];
> +       __aligned_u64 args[0];
>  };
>
>  #endif /* _UAPI__LINUX_BPF_H__ */
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 903010a..174e99a 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -259,8 +259,8 @@ struct bpf_stack_build_id {
>         __s32           status;
>         unsigned char   build_id[BPF_BUILD_ID_SIZE];
>         union {
> -               __u64   offset;
> -               __u64   ip;
> +               __aligned_u64   offset;
> +               __aligned_u64   ip;
>         };
>  };
>
> @@ -288,7 +288,7 @@ union bpf_attr {
>                         __aligned_u64 value;
>                         __aligned_u64 next_key;
>                 };
> -               __u64           flags;
> +               __aligned_u64   flags;
>         };
>
>         struct { /* anonymous struct used by BPF_PROG_LOAD command */
> @@ -360,7 +360,7 @@ union bpf_attr {
>         } query;
>
>         struct {
> -               __u64 name;
> +               __aligned_u64 name;
>                 __u32 prog_fd;
>         } raw_tracepoint;
>  } __attribute__((aligned(8)));
> @@ -1011,7 +1011,7 @@ struct bpf_prog_info {
>         __u32 xlated_prog_len;
>         __aligned_u64 jited_prog_insns;
>         __aligned_u64 xlated_prog_insns;
> -       __u64 load_time;        /* ns since boottime */
> +       __aligned_u64 load_time;        /* ns since boottime */
>         __u32 created_by_uid;
>         __u32 nr_map_ids;
>         __aligned_u64 map_ids;
> @@ -1101,8 +1101,8 @@ struct bpf_sock_ops {
>         __u32 lost_out;
>         __u32 sacked_out;
>         __u32 sk_txhash;
> -       __u64 bytes_received;
> -       __u64 bytes_acked;
> +       __aligned_u64 bytes_received;
> +       __aligned_u64 bytes_acked;
>  };
>
>  /* Definitions for bpf_sock_ops_cb_flags */
> @@ -1189,9 +1189,9 @@ enum {
>  #define TCP_BPF_SNDCWND_CLAMP  1002    /* Set sndcwnd_clamp */
>
>  struct bpf_perf_event_value {
> -       __u64 counter;
> -       __u64 enabled;
> -       __u64 running;
> +       __aligned_u64 counter;
> +       __aligned_u64 enabled;
> +       __aligned_u64 running;
>  };
>
>  #define BPF_DEVCG_ACC_MKNOD    (1ULL << 0)
> @@ -1209,7 +1209,7 @@ struct bpf_cgroup_dev_ctx {
>  };
>
>  struct bpf_raw_tracepoint_args {
> -       __u64 args[0];
> +       __aligned_u64 args[0];
>  };
>
>  #endif /* _UAPI__LINUX_BPF_H__ */
> --
> 2.1.4
>

I think these changes are not necessary. Is it a general guidance to
only use 64-bit aligned
variables in UAPI headers?

Thanks,
Song

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf 2/2] bpf: enforce usage of __aligned_u64 in the UAPI header
  2018-05-29 17:35 ` Song Liu
@ 2018-05-30 10:03   ` Eugene Syromiatnikov
  0 siblings, 0 replies; 3+ messages in thread
From: Eugene Syromiatnikov @ 2018-05-30 10:03 UTC (permalink / raw)
  To: Song Liu
  Cc: netdev, open list, Martin KaFai Lau, Daniel Borkmann,
	Alexei Starovoitov, David S. Miller, Jiri Olsa, Ingo Molnar,
	Lawrence Brakmo, Andrey Ignatov, Jakub Kicinski, John Fastabend,
	Dmitry V. Levin

On Tue, May 29, 2018 at 10:35:09AM -0700, Song Liu wrote:
> I think these changes are not necessary. Is it a general guidance to
> only use 64-bit aligned
> variables in UAPI headers?

Not really, but it allows avoiding most alignment issues like the one
mentioned in the previous patch and in the referenced RDMA patch.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-05-30 10:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-27 11:28 [PATCH bpf 2/2] bpf: enforce usage of __aligned_u64 in the UAPI header Eugene Syromiatnikov
2018-05-29 17:35 ` Song Liu
2018-05-30 10:03   ` Eugene Syromiatnikov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).