From: Andrii Nakryiko <andrii@kernel.org>
To: <bpf@vger.kernel.org>, <ast@kernel.org>, <daniel@iogearbox.net>,
<martin.lau@kernel.org>
Cc: <andrii@kernel.org>, <kernel-team@meta.com>
Subject: [PATCH RFC bpf-next 2/3] bpf: use new named bpf_attr substructs for few commands
Date: Wed, 24 May 2023 14:02:42 -0700 [thread overview]
Message-ID: <20230524210243.605832-3-andrii@kernel.org> (raw)
In-Reply-To: <20230524210243.605832-1-andrii@kernel.org>
Switch to new named bpf_xxx_attr substructs for a few commands to
demonstrate how minimal are the changes, especially for simple commands.
For more "sprawling" commands like BPF_MAP_CREATE and BPF_PROG_LOAD
changes will be of a similar nature (replacing union bpf_attr with
command-specific struct bpf_xxx_attr), but will touch more of helper
functions. Given the RFC status I wanted to avoid unnecessary mundane
work before getting a green light for the entire approach.
Note the changes to CHECK_ATTR() implementation. It's equivalent in
functionality, but, IMO, more straightforward and actually support both
old-style `union bpf_attr` and new-style `struct bpf_xxx_attr` usage
transparently.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/bpf/syscall.c | 37 +++++++++++++++++--------------------
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index babf1d56c2d9..fbc5c86c7bca 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -974,11 +974,8 @@ int bpf_get_file_flag(int flags)
/* helper macro to check that unused fields 'union bpf_attr' are zero */
#define CHECK_ATTR(CMD) \
- memchr_inv((void *) &attr->CMD##_LAST_FIELD + \
- sizeof(attr->CMD##_LAST_FIELD), 0, \
- sizeof(*attr) - \
- offsetof(union bpf_attr, CMD##_LAST_FIELD) - \
- sizeof(attr->CMD##_LAST_FIELD)) != NULL
+ memchr_inv((void *)attr + offsetofend(union bpf_attr, CMD##_LAST_FIELD), 0, \
+ sizeof(union bpf_attr) - offsetofend(union bpf_attr, CMD##_LAST_FIELD)) != NULL
/* dst and src must have at least "size" number of bytes.
* Return strlen on success and < 0 on error.
@@ -1354,7 +1351,7 @@ static void *___bpf_copy_key(bpfptr_t ukey, u64 key_size)
/* last field in 'union bpf_attr' used by this command */
#define BPF_MAP_LOOKUP_ELEM_LAST_FIELD map_elem.flags
-static int map_lookup_elem(union bpf_attr *attr)
+static int map_lookup_elem(struct bpf_map_elem_attr *attr)
{
void __user *ukey = u64_to_user_ptr(attr->key);
void __user *uvalue = u64_to_user_ptr(attr->value);
@@ -1429,7 +1426,7 @@ static int map_lookup_elem(union bpf_attr *attr)
#define BPF_MAP_UPDATE_ELEM_LAST_FIELD map_elem.flags
-static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
+static int map_update_elem(struct bpf_map_elem_attr *attr, bpfptr_t uattr)
{
bpfptr_t ukey = make_bpfptr(attr->key, uattr.is_kernel);
bpfptr_t uvalue = make_bpfptr(attr->value, uattr.is_kernel);
@@ -1485,7 +1482,7 @@ static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
#define BPF_MAP_DELETE_ELEM_LAST_FIELD map_elem.key
-static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
+static int map_delete_elem(struct bpf_map_elem_attr *attr, bpfptr_t uattr)
{
bpfptr_t ukey = make_bpfptr(attr->key, uattr.is_kernel);
int ufd = attr->map_fd;
@@ -1540,7 +1537,7 @@ static int map_delete_elem(union bpf_attr *attr, bpfptr_t uattr)
/* last field in 'union bpf_attr' used by this command */
#define BPF_MAP_GET_NEXT_KEY_LAST_FIELD map_next_key.next_key
-static int map_get_next_key(union bpf_attr *attr)
+static int map_get_next_key(struct bpf_map_next_key_attr *attr)
{
void __user *ukey = u64_to_user_ptr(attr->key);
void __user *unext_key = u64_to_user_ptr(attr->next_key);
@@ -1912,7 +1909,7 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
#define BPF_MAP_FREEZE_LAST_FIELD map_freeze.map_fd
-static int map_freeze(const union bpf_attr *attr)
+static int map_freeze(const struct bpf_map_freeze_attr *attr)
{
int err = 0, ufd = attr->map_fd;
struct bpf_map *map;
@@ -3710,7 +3707,7 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
#define BPF_OBJ_GET_NEXT_ID_LAST_FIELD obj_next_id.next_id
-static int bpf_obj_get_next_id(const union bpf_attr *attr,
+static int bpf_obj_get_next_id(const struct bpf_obj_next_id_attr *attr,
union bpf_attr __user *uattr,
struct idr *idr,
spinlock_t *lock)
@@ -5064,19 +5061,19 @@ static int __sys_bpf(int cmd, bpfptr_t uattr, unsigned int size)
err = map_create(&attr);
break;
case BPF_MAP_LOOKUP_ELEM:
- err = map_lookup_elem(&attr);
+ err = map_lookup_elem(&attr.map_elem);
break;
case BPF_MAP_UPDATE_ELEM:
- err = map_update_elem(&attr, uattr);
+ err = map_update_elem(&attr.map_elem, uattr);
break;
case BPF_MAP_DELETE_ELEM:
- err = map_delete_elem(&attr, uattr);
+ err = map_delete_elem(&attr.map_elem, uattr);
break;
case BPF_MAP_GET_NEXT_KEY:
- err = map_get_next_key(&attr);
+ err = map_get_next_key(&attr.map_next_key);
break;
case BPF_MAP_FREEZE:
- err = map_freeze(&attr);
+ err = map_freeze(&attr.map_freeze);
break;
case BPF_PROG_LOAD:
err = bpf_prog_load(&attr, uattr, size);
@@ -5100,15 +5097,15 @@ static int __sys_bpf(int cmd, bpfptr_t uattr, unsigned int size)
err = bpf_prog_test_run(&attr, uattr.user);
break;
case BPF_PROG_GET_NEXT_ID:
- err = bpf_obj_get_next_id(&attr, uattr.user,
+ err = bpf_obj_get_next_id(&attr.obj_next_id, uattr.user,
&prog_idr, &prog_idr_lock);
break;
case BPF_MAP_GET_NEXT_ID:
- err = bpf_obj_get_next_id(&attr, uattr.user,
+ err = bpf_obj_get_next_id(&attr.obj_next_id, uattr.user,
&map_idr, &map_idr_lock);
break;
case BPF_BTF_GET_NEXT_ID:
- err = bpf_obj_get_next_id(&attr, uattr.user,
+ err = bpf_obj_get_next_id(&attr.obj_next_id, uattr.user,
&btf_idr, &btf_idr_lock);
break;
case BPF_PROG_GET_FD_BY_ID:
@@ -5158,7 +5155,7 @@ static int __sys_bpf(int cmd, bpfptr_t uattr, unsigned int size)
err = bpf_link_get_fd_by_id(&attr);
break;
case BPF_LINK_GET_NEXT_ID:
- err = bpf_obj_get_next_id(&attr, uattr.user,
+ err = bpf_obj_get_next_id(&attr.obj_next_id, uattr.user,
&link_idr, &link_idr_lock);
break;
case BPF_ENABLE_STATS:
--
2.34.1
next prev parent reply other threads:[~2023-05-24 21:03 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-24 21:02 [PATCH RFC bpf-next 0/3] Revamp bpf_attr and make it easier to evolve Andrii Nakryiko
2023-05-24 21:02 ` [PATCH RFC bpf-next 1/3] bpf: revamp bpf_attr and name each command's field and substruct Andrii Nakryiko
2023-05-25 3:18 ` Alexei Starovoitov
2023-05-25 17:19 ` Andrii Nakryiko
2023-05-25 21:51 ` Daniel Borkmann
2023-05-25 23:39 ` Andrii Nakryiko
2023-05-30 17:41 ` Alexei Starovoitov
2023-05-30 18:26 ` Andrii Nakryiko
2023-05-24 21:02 ` Andrii Nakryiko [this message]
2023-05-24 21:02 ` [PATCH RFC bpf-next 3/3] libbpf: use new bpf_xxx_attr structs for bpf() commands Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230524210243.605832-3-andrii@kernel.org \
--to=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@meta.com \
--cc=martin.lau@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).