From: David Marchevsky <david.marchevsky@linux.dev>
To: Yonghong Song <yonghong.song@linux.dev>, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
kernel-team@fb.com, Martin KaFai Lau <martin.lau@kernel.org>
Subject: Re: [PATCH bpf-next 02/15] bpf: Add BPF_KPTR_PERCPU_REF as a field type
Date: Fri, 18 Aug 2023 14:37:41 -0400 [thread overview]
Message-ID: <ee360b23-9768-9187-4eb0-d43b67bcd07c@linux.dev> (raw)
In-Reply-To: <20230814172820.1362751-1-yonghong.song@linux.dev>
On 8/14/23 1:28 PM, Yonghong Song wrote:
> BPF_KPTR_PERCPU_REF represents a percpu field type like below
>
> struct val_t {
> ... fields ...
> };
> struct t {
> ...
> struct val_t __percpu *percpu_data_ptr;
> ...
> };
>
> where
> #define __percpu __attribute__((btf_type_tag("percpu")))
nit: Maybe this should be __percpu_kptr (and similar for the actual tag)?
I don't feel strongly about this. It's certainly less concise, but given that
existing docs mention kptrs a few times, and anyone using percpu kptrs can
probably be expected to have some familiarity with "normal" kptrs, making
it more clear to BPF program writers that their existing mental model of
what a kptr is and how it should be used seems beneficial.
>
> While BPF_KPTR_REF points to a trusted kernel object or a trusted
> local object, BPF_KPTR_PERCPU_REF points to a trusted local
> percpu object.
>
> This patch added basic support for BPF_KPTR_PERCPU_REF
> related to percpu field parsing, recording and free operations.
> BPF_KPTR_PERCPU_REF also supports the same map types
> as BPF_KPTR_REF does.
>
> Note that unlike a local kptr, it is possible that
> a BPF_KTPR_PERCUP_REF struct may not contain any
nit: typo here ("BPF_KTPR_PERCUP_REF" -> "BPF_KPTR_PERCPU_REF")
> special fields like other kptr, bpf_spin_lock, bpf_list_head, etc.
>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
> include/linux/bpf.h | 18 ++++++++++++------
> kernel/bpf/btf.c | 5 +++++
> kernel/bpf/syscall.c | 7 ++++++-
> 3 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 60e80e90c37d..e6348fd0a785 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -180,14 +180,15 @@ enum btf_field_type {
> BPF_TIMER = (1 << 1),
> BPF_KPTR_UNREF = (1 << 2),
> BPF_KPTR_REF = (1 << 3),
> - BPF_KPTR = BPF_KPTR_UNREF | BPF_KPTR_REF,
> - BPF_LIST_HEAD = (1 << 4),
> - BPF_LIST_NODE = (1 << 5),
> - BPF_RB_ROOT = (1 << 6),
> - BPF_RB_NODE = (1 << 7),
> + BPF_KPTR_PERCPU_REF = (1 << 4),
> + BPF_KPTR = BPF_KPTR_UNREF | BPF_KPTR_REF | BPF_KPTR_PERCPU_REF,
> + BPF_LIST_HEAD = (1 << 5),
> + BPF_LIST_NODE = (1 << 6),
> + BPF_RB_ROOT = (1 << 7),
> + BPF_RB_NODE = (1 << 8),
> BPF_GRAPH_NODE_OR_ROOT = BPF_LIST_NODE | BPF_LIST_HEAD |
> BPF_RB_NODE | BPF_RB_ROOT,
> - BPF_REFCOUNT = (1 << 8),
> + BPF_REFCOUNT = (1 << 9),
> };
>
> typedef void (*btf_dtor_kfunc_t)(void *);
> @@ -300,6 +301,8 @@ static inline const char *btf_field_type_name(enum btf_field_type type)
> case BPF_KPTR_UNREF:
> case BPF_KPTR_REF:
> return "kptr";
> + case BPF_KPTR_PERCPU_REF:
> + return "kptr_percpu";
> case BPF_LIST_HEAD:
> return "bpf_list_head";
> case BPF_LIST_NODE:
> @@ -325,6 +328,7 @@ static inline u32 btf_field_type_size(enum btf_field_type type)
> return sizeof(struct bpf_timer);
> case BPF_KPTR_UNREF:
> case BPF_KPTR_REF:
> + case BPF_KPTR_PERCPU_REF:
> return sizeof(u64);
> case BPF_LIST_HEAD:
> return sizeof(struct bpf_list_head);
> @@ -351,6 +355,7 @@ static inline u32 btf_field_type_align(enum btf_field_type type)
> return __alignof__(struct bpf_timer);
> case BPF_KPTR_UNREF:
> case BPF_KPTR_REF:
> + case BPF_KPTR_PERCPU_REF:
> return __alignof__(u64);
> case BPF_LIST_HEAD:
> return __alignof__(struct bpf_list_head);
> @@ -389,6 +394,7 @@ static inline void bpf_obj_init_field(const struct btf_field *field, void *addr)
> case BPF_TIMER:
> case BPF_KPTR_UNREF:
> case BPF_KPTR_REF:
> + case BPF_KPTR_PERCPU_REF:
> break;
> default:
> WARN_ON_ONCE(1);
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 249657c466dd..bc1792edc64e 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -3293,6 +3293,8 @@ static int btf_find_kptr(const struct btf *btf, const struct btf_type *t,
> type = BPF_KPTR_UNREF;
> else if (!strcmp("kptr", __btf_name_by_offset(btf, t->name_off)))
> type = BPF_KPTR_REF;
> + else if (!strcmp("percpu", __btf_name_by_offset(btf, t->name_off)))
> + type = BPF_KPTR_PERCPU_REF;
> else
> return -EINVAL;
>
> @@ -3457,6 +3459,7 @@ static int btf_find_struct_field(const struct btf *btf,
> break;
> case BPF_KPTR_UNREF:
> case BPF_KPTR_REF:
> + case BPF_KPTR_PERCPU_REF:
> ret = btf_find_kptr(btf, member_type, off, sz,
> idx < info_cnt ? &info[idx] : &tmp);
> if (ret < 0)
> @@ -3523,6 +3526,7 @@ static int btf_find_datasec_var(const struct btf *btf, const struct btf_type *t,
> break;
> case BPF_KPTR_UNREF:
> case BPF_KPTR_REF:
> + case BPF_KPTR_PERCPU_REF:
> ret = btf_find_kptr(btf, var_type, off, sz,
> idx < info_cnt ? &info[idx] : &tmp);
> if (ret < 0)
> @@ -3783,6 +3787,7 @@ struct btf_record *btf_parse_fields(const struct btf *btf, const struct btf_type
> break;
> case BPF_KPTR_UNREF:
> case BPF_KPTR_REF:
> + case BPF_KPTR_PERCPU_REF:
> ret = btf_parse_kptr(btf, &rec->fields[i], &info_arr[i]);
> if (ret < 0)
> goto end;
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 7f4e8c357a6a..1c30b6ee84d4 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -514,6 +514,7 @@ void btf_record_free(struct btf_record *rec)
> switch (rec->fields[i].type) {
> case BPF_KPTR_UNREF:
> case BPF_KPTR_REF:
> + case BPF_KPTR_PERCPU_REF:
> if (rec->fields[i].kptr.module)
> module_put(rec->fields[i].kptr.module);
> btf_put(rec->fields[i].kptr.btf);
> @@ -560,6 +561,7 @@ struct btf_record *btf_record_dup(const struct btf_record *rec)
> switch (fields[i].type) {
> case BPF_KPTR_UNREF:
> case BPF_KPTR_REF:
> + case BPF_KPTR_PERCPU_REF:
> btf_get(fields[i].kptr.btf);
> if (fields[i].kptr.module && !try_module_get(fields[i].kptr.module)) {
> ret = -ENXIO;
> @@ -650,6 +652,7 @@ void bpf_obj_free_fields(const struct btf_record *rec, void *obj)
> WRITE_ONCE(*(u64 *)field_ptr, 0);
> break;
> case BPF_KPTR_REF:
> + case BPF_KPTR_PERCPU_REF:
> xchgd_field = (void *)xchg((unsigned long *)field_ptr, 0);
> if (!xchgd_field)
> break;
> @@ -657,7 +660,8 @@ void bpf_obj_free_fields(const struct btf_record *rec, void *obj)
> if (!btf_is_kernel(field->kptr.btf)) {
> pointee_struct_meta = btf_find_struct_meta(field->kptr.btf,
> field->kptr.btf_id);
> - WARN_ON_ONCE(!pointee_struct_meta);
> + if (field->type != BPF_KPTR_PERCPU_REF)
> + WARN_ON_ONCE(!pointee_struct_meta);
> migrate_disable();
> __bpf_obj_drop_impl(xchgd_field, pointee_struct_meta ?
> pointee_struct_meta->record :
> @@ -1046,6 +1050,7 @@ static int map_check_btf(struct bpf_map *map, const struct btf *btf,
> break;
> case BPF_KPTR_UNREF:
> case BPF_KPTR_REF:
> + case BPF_KPTR_PERCPU_REF:
> case BPF_REFCOUNT:
> if (map->map_type != BPF_MAP_TYPE_HASH &&
> map->map_type != BPF_MAP_TYPE_PERCPU_HASH &&
next prev parent reply other threads:[~2023-08-18 18:37 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-14 17:28 [PATCH bpf-next 00/15] Add support for local percpu kptr Yonghong Song
2023-08-14 17:28 ` [PATCH bpf-next 01/15] bpf: Add support for non-fix-size percpu mem allocation Yonghong Song
2023-08-14 17:28 ` [PATCH bpf-next 02/15] bpf: Add BPF_KPTR_PERCPU_REF as a field type Yonghong Song
2023-08-18 18:37 ` David Marchevsky [this message]
2023-08-18 23:24 ` Alexei Starovoitov
2023-08-20 3:46 ` Yonghong Song
2023-08-20 3:45 ` Yonghong Song
2023-08-14 17:28 ` [PATCH bpf-next 03/15] bpf: Add alloc/xchg/direct_access support for local percpu kptr Yonghong Song
2023-08-19 0:29 ` Alexei Starovoitov
2023-08-20 3:47 ` Yonghong Song
2023-08-19 1:24 ` Kumar Kartikeya Dwivedi
2023-08-20 4:04 ` Yonghong Song
2023-08-14 17:28 ` [PATCH bpf-next 04/15] bpf: Add bpf_this_cpu_ptr/bpf_per_cpu_ptr support for allocated percpu obj Yonghong Song
2023-08-19 1:01 ` Alexei Starovoitov
2023-08-20 4:16 ` Yonghong Song
2023-08-14 17:28 ` [PATCH bpf-next 05/15] selftests/bpf: Update error message in negative linked_list test Yonghong Song
2023-08-14 17:28 ` [PATCH bpf-next 06/15] libbpf: Add __percpu macro definition Yonghong Song
2023-08-14 17:28 ` [PATCH bpf-next 07/15] selftests/bpf: Add bpf_percpu_obj_{new,drop}() macro in bpf_experimental.h Yonghong Song
2023-08-14 17:28 ` [PATCH bpf-next 08/15] selftests/bpf: Add tests for array map with local percpu kptr Yonghong Song
2023-08-14 17:28 ` [PATCH bpf-next 09/15] bpf: Mark OBJ_RELEASE argument as MEM_RCU when possible Yonghong Song
2023-08-19 1:44 ` Kumar Kartikeya Dwivedi
2023-08-20 4:19 ` Yonghong Song
2023-08-14 17:29 ` [PATCH bpf-next 10/15] selftests/bpf: Remove unnecessary direct read of local percpu kptr Yonghong Song
2023-08-14 17:29 ` [PATCH bpf-next 11/15] selftests/bpf: Add tests for cgrp_local_storage with " Yonghong Song
2023-08-14 17:29 ` [PATCH bpf-next 12/15] bpf: Allow bpf_spin_lock and bpf_list_head in allocated percpu data structure Yonghong Song
2023-08-14 17:29 ` [PATCH bpf-next 13/15] selftests/bpf: Add tests for percpu struct with bpf list head Yonghong Song
2023-08-14 17:29 ` [PATCH bpf-next 14/15] selftests/bpf: Add some negative tests Yonghong Song
2023-08-14 17:29 ` [PATCH bpf-next 15/15] bpf: Mark BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE deprecated Yonghong Song
2023-08-18 15:54 ` Daniel Borkmann
2023-08-18 17:17 ` Yonghong Song
2023-08-18 18:26 ` Zvi Effron
2023-08-18 18:58 ` Yonghong Song
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=ee360b23-9768-9187-4eb0-d43b67bcd07c@linux.dev \
--to=david.marchevsky@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=martin.lau@kernel.org \
--cc=yonghong.song@linux.dev \
/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