From: Martin KaFai Lau <martin.lau@linux.dev>
To: thinker.li@gmail.com
Cc: sinquersw@gmail.com, kuifeng@meta.com, bpf@vger.kernel.org,
ast@kernel.org, song@kernel.org, kernel-team@meta.com,
andrii@kernel.org
Subject: Re: [RFC bpf-next v3 07/11] bpf, net: switch to storing struct_ops in btf
Date: Mon, 25 Sep 2023 17:02:26 -0700 [thread overview]
Message-ID: <cc0aa287-e4de-13fe-1727-b1e0e8dbc3ba@linux.dev> (raw)
In-Reply-To: <20230920155923.151136-8-thinker.li@gmail.com>
On 9/20/23 8:59 AM, thinker.li@gmail.com wrote:
> From: Kui-Feng Lee <thinker.li@gmail.com>
>
> Use struct_ops registered and stored in module btf instead of static ones.
>
> Both bpf_dummy_ops and bpf_tcp_ca switches to calling the registration
> function instead of listed in bpf_struct_ops_types.h.
>
> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> ---
> kernel/bpf/bpf_struct_ops.c | 114 ++++++++++++++++++------------
> kernel/bpf/bpf_struct_ops_types.h | 12 ----
> net/bpf/bpf_dummy_struct_ops.c | 12 +++-
> net/ipv4/bpf_tcp_ca.c | 20 +++++-
> 4 files changed, 94 insertions(+), 64 deletions(-)
> delete mode 100644 kernel/bpf/bpf_struct_ops_types.h
>
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index fb684d2ee99d..8b5c859377e9 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -59,35 +59,6 @@ static DEFINE_MUTEX(update_mutex);
> #define VALUE_PREFIX "bpf_struct_ops_"
> #define VALUE_PREFIX_LEN (sizeof(VALUE_PREFIX) - 1)
>
> -/* bpf_struct_ops_##_name (e.g. bpf_struct_ops_tcp_congestion_ops) is
> - * the map's value exposed to the userspace and its btf-type-id is
> - * stored at the map->btf_vmlinux_value_type_id.
> - *
> - */
> -#define BPF_STRUCT_OPS_TYPE(_name) \
> -extern struct bpf_struct_ops bpf_##_name; \
> - \
> -struct bpf_struct_ops_##_name { \
> - BPF_STRUCT_OPS_COMMON_VALUE; \
> - struct _name data ____cacheline_aligned_in_smp; \
> -};
> -#include "bpf_struct_ops_types.h"
> -#undef BPF_STRUCT_OPS_TYPE
> -
> -enum {
> -#define BPF_STRUCT_OPS_TYPE(_name) BPF_STRUCT_OPS_TYPE_##_name,
> -#include "bpf_struct_ops_types.h"
> -#undef BPF_STRUCT_OPS_TYPE
> - __NR_BPF_STRUCT_OPS_TYPE,
> -};
> -
> -static struct bpf_struct_ops * const bpf_struct_ops[] = {
> -#define BPF_STRUCT_OPS_TYPE(_name) \
> - [BPF_STRUCT_OPS_TYPE_##_name] = &bpf_##_name,
> -#include "bpf_struct_ops_types.h"
> -#undef BPF_STRUCT_OPS_TYPE
> -};
> -
> const struct bpf_verifier_ops bpf_struct_ops_verifier_ops = {
> };
>
> @@ -264,14 +235,11 @@ static void bpf_struct_ops_init_one(struct bpf_struct_ops *st_ops,
>
> void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
> {
> - struct bpf_struct_ops *st_ops;
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_NET)
> + extern struct bpf_struct_ops_mod bpf_testmod_struct_ops;
> + int ret;
> +#endif
> s32 module_id;
> - u32 i;
> -
> - /* Ensure BTF type is emitted for "struct bpf_struct_ops_##_name" */
> -#define BPF_STRUCT_OPS_TYPE(_name) BTF_TYPE_EMIT(struct bpf_struct_ops_##_name);
> -#include "bpf_struct_ops_types.h"
> -#undef BPF_STRUCT_OPS_TYPE
>
> module_id = btf_find_by_name_kind(btf, "module", BTF_KIND_STRUCT);
> if (module_id < 0) {
> @@ -280,43 +248,95 @@ void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
> }
> module_type = btf_type_by_id(btf, module_id);
>
> - for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
> - st_ops = bpf_struct_ops[i];
> - bpf_struct_ops_init_one(st_ops, btf, log);
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_NET)
> + ret = register_bpf_struct_ops(&bpf_testmod_struct_ops);
What is stopping the 'register_bpf_struct_ops(&bpf_testmod_struct_ops)' to be
done in bpf_dummy_struct_ops.c instead of here?
I am hoping bpf_dummy_struct_ops.c can eventually be moved out to
bpf_testmod_struct_ops.c but it is better to leave it as a followup later.
> + if (ret)
> + pr_warn("Cannot register bpf_testmod_struct_ops\n");
> +#endif
> +}
> +
> +int register_bpf_struct_ops(struct bpf_struct_ops_mod *mod)
> +{
> + struct bpf_struct_ops *st_ops = mod->st_ops;
> + struct bpf_verifier_log *log;
> + struct btf *btf;
> + int err;
> +
> + if (mod->st_ops == NULL ||
> + mod->owner == NULL)
> + return -EINVAL;
> +
> + log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);
> + if (!log) {
> + err = -ENOMEM;
> + goto errout;
> + }
> +
> + log->level = BPF_LOG_KERNEL;
> +
> + btf = btf_get_module_btf(mod->owner);
> + if (!btf) {
> + err = -EINVAL;
> + goto errout;
> }
> +
> + bpf_struct_ops_init_one(st_ops, btf, log);
> +
> + btf_put(btf);
> +
> + st_ops->owner = mod->owner;
> + err = btf_add_struct_ops(st_ops, st_ops->owner);
> +
> +errout:
> + kfree(log);
> +
> + return err;
> }
> +EXPORT_SYMBOL_GPL(register_bpf_struct_ops);
>
> extern struct btf *btf_vmlinux;
>
> static const struct bpf_struct_ops *
> bpf_struct_ops_find_value(u32 value_id, struct btf *btf)
> {
> + const struct bpf_struct_ops *st_ops = NULL;
> + const struct bpf_struct_ops **st_ops_list;
> unsigned int i;
> + u32 cnt = 0;
>
> if (!value_id || !btf_vmlinux)
> return NULL;
>
> - for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
> - if (bpf_struct_ops[i]->value_id == value_id)
> - return bpf_struct_ops[i];
> + st_ops_list = btf_get_struct_ops(btf, &cnt);
> + for (i = 0; i < cnt; i++) {
> + if (st_ops_list[i]->value_id == value_id) {
> + st_ops = st_ops_list[i];
> + break;
> + }
> }
>
> - return NULL;
> + return st_ops;
> }
>
> const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id, struct btf *btf)
> {
> + const struct bpf_struct_ops *st_ops = NULL;
> + const struct bpf_struct_ops **st_ops_list;
> unsigned int i;
> + u32 cnt;
>
> if (!type_id || !btf_vmlinux)
> return NULL;
>
> - for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
> - if (bpf_struct_ops[i]->type_id == type_id)
> - return bpf_struct_ops[i];
> + st_ops_list = btf_get_struct_ops(btf, &cnt);
> + for (i = 0; i < cnt; i++) {
> + if (st_ops_list[i]->type_id == type_id) {
> + st_ops = st_ops_list[i];
> + break;
> + }
> }
>
> - return NULL;
> + return st_ops;
> }
>
> static int bpf_struct_ops_map_get_next_key(struct bpf_map *map, void *key,
> diff --git a/kernel/bpf/bpf_struct_ops_types.h b/kernel/bpf/bpf_struct_ops_types.h
> deleted file mode 100644
> index 5678a9ddf817..000000000000
> --- a/kernel/bpf/bpf_struct_ops_types.h
> +++ /dev/null
> @@ -1,12 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -/* internal file - do not include directly */
> -
> -#ifdef CONFIG_BPF_JIT
> -#ifdef CONFIG_NET
> -BPF_STRUCT_OPS_TYPE(bpf_dummy_ops)
> -#endif
> -#ifdef CONFIG_INET
> -#include <net/tcp.h>
> -BPF_STRUCT_OPS_TYPE(tcp_congestion_ops)
> -#endif
> -#endif
> diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c
> index 5918d1b32e19..9cb982c67c4c 100644
> --- a/net/bpf/bpf_dummy_struct_ops.c
> +++ b/net/bpf/bpf_dummy_struct_ops.c
> @@ -7,7 +7,7 @@
> #include <linux/bpf.h>
> #include <linux/btf.h>
>
> -extern struct bpf_struct_ops bpf_bpf_dummy_ops;
> +static struct bpf_struct_ops bpf_bpf_dummy_ops;
>
> /* A common type for test_N with return value in bpf_dummy_ops */
> typedef int (*dummy_ops_test_ret_fn)(struct bpf_dummy_ops_state *state, ...);
> @@ -218,9 +218,12 @@ static int bpf_dummy_reg(void *kdata)
>
> static void bpf_dummy_unreg(void *kdata)
> {
> + BTF_STRUCT_OPS_TYPE_EMIT(bpf_dummy_ops);
> }
>
> -struct bpf_struct_ops bpf_bpf_dummy_ops = {
> +DEFINE_STRUCT_OPS_VALUE_TYPE(bpf_dummy_ops);
> +
> +static struct bpf_struct_ops bpf_bpf_dummy_ops = {
> .verifier_ops = &bpf_dummy_verifier_ops,
> .init = bpf_dummy_init,
> .check_member = bpf_dummy_ops_check_member,
> @@ -229,3 +232,8 @@ struct bpf_struct_ops bpf_bpf_dummy_ops = {
> .unreg = bpf_dummy_unreg,
> .name = "bpf_dummy_ops",
> };
> +
> +struct bpf_struct_ops_mod bpf_testmod_struct_ops = {
> + .st_ops = &bpf_bpf_dummy_ops,
> + .owner = THIS_MODULE,
> +};
> diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
> index 39dcccf0f174..9947323f3e22 100644
> --- a/net/ipv4/bpf_tcp_ca.c
> +++ b/net/ipv4/bpf_tcp_ca.c
> @@ -12,7 +12,7 @@
> #include <net/bpf_sk_storage.h>
>
> /* "extern" is to avoid sparse warning. It is only used in bpf_struct_ops.c. */
> -extern struct bpf_struct_ops bpf_tcp_congestion_ops;
> +static struct bpf_struct_ops bpf_tcp_congestion_ops;
>
> static u32 unsupported_ops[] = {
> offsetof(struct tcp_congestion_ops, get_info),
> @@ -271,7 +271,9 @@ static int bpf_tcp_ca_validate(void *kdata)
> return tcp_validate_congestion_control(kdata);
> }
>
> -struct bpf_struct_ops bpf_tcp_congestion_ops = {
> +DEFINE_STRUCT_OPS_VALUE_TYPE(tcp_congestion_ops);
> +
> +static struct bpf_struct_ops bpf_tcp_congestion_ops = {
> .verifier_ops = &bpf_tcp_ca_verifier_ops,
> .reg = bpf_tcp_ca_reg,
> .unreg = bpf_tcp_ca_unreg,
> @@ -283,8 +285,20 @@ struct bpf_struct_ops bpf_tcp_congestion_ops = {
> .name = "tcp_congestion_ops",
> };
>
> +static struct bpf_struct_ops_mod bpf_tcp_ca_ops_mod = {
> + .st_ops = &bpf_tcp_congestion_ops,
> + .owner = THIS_MODULE,
> +};
> +
> static int __init bpf_tcp_ca_kfunc_init(void)
> {
> - return register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_tcp_ca_kfunc_set);
> + int ret;
> +
> + BTF_STRUCT_OPS_TYPE_EMIT(tcp_congestion_ops);
> +
> + ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_tcp_ca_kfunc_set);
> + ret = ret ?: register_bpf_struct_ops(&bpf_tcp_ca_ops_mod);
> +
> + return ret;
> }
> late_initcall(bpf_tcp_ca_kfunc_init);
next prev parent reply other threads:[~2023-09-26 0:02 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-20 15:59 [RFC bpf-next v3 00/11] Registrating struct_ops types from modules thinker.li
2023-09-20 15:59 ` [RFC bpf-next v3 01/11] bpf: refactory struct_ops type initialization to a function thinker.li
2023-09-20 15:59 ` [RFC bpf-next v3 02/11] bpf: add struct_ops_tab to btf thinker.li
2023-09-25 21:10 ` Martin KaFai Lau
2023-09-25 21:45 ` Kui-Feng Lee
2023-09-20 15:59 ` [RFC bpf-next v3 03/11] bpf: add register and unregister functions for struct_ops thinker.li
2023-09-25 23:07 ` Martin KaFai Lau
2023-09-25 23:13 ` Kui-Feng Lee
2023-09-25 23:31 ` Martin KaFai Lau
2023-09-26 0:19 ` Kui-Feng Lee
2023-09-20 15:59 ` [RFC bpf-next v3 04/11] bpf: attach a module BTF to a bpf_struct_ops thinker.li
2023-09-25 22:57 ` Martin KaFai Lau
2023-09-25 23:25 ` Kui-Feng Lee
2023-09-20 15:59 ` [RFC bpf-next v3 05/11] bpf: hold module for bpf_struct_ops_map thinker.li
2023-09-25 23:23 ` Martin KaFai Lau
2023-09-25 23:42 ` Kui-Feng Lee
2023-09-20 15:59 ` [RFC bpf-next v3 06/11] bpf: validate value_type thinker.li
2023-09-26 1:03 ` Martin KaFai Lau
2023-09-27 20:27 ` Kui-Feng Lee
2023-09-20 15:59 ` [RFC bpf-next v3 07/11] bpf, net: switch to storing struct_ops in btf thinker.li
2023-09-20 23:45 ` kernel test robot
2023-09-24 3:23 ` kernel test robot
2023-09-25 8:30 ` kernel test robot
2023-09-26 0:02 ` Martin KaFai Lau [this message]
2023-09-26 0:18 ` Kui-Feng Lee
2023-09-20 15:59 ` [RFC bpf-next v3 08/11] bpf: pass attached BTF to find correct type info of struct_ops progs thinker.li
2023-09-25 22:58 ` Andrii Nakryiko
2023-09-25 23:50 ` Kui-Feng Lee
2023-09-26 0:24 ` Martin KaFai Lau
2023-09-26 0:58 ` Kui-Feng Lee
2023-09-20 15:59 ` [RFC bpf-next v3 09/11] libbpf: Find correct module BTFs for struct_ops maps and progs thinker.li
2023-09-25 23:09 ` Andrii Nakryiko
2023-09-26 0:12 ` Kui-Feng Lee
2023-09-20 15:59 ` [RFC bpf-next v3 10/11] bpf: export btf_ctx_access to modules thinker.li
2023-09-20 15:59 ` [RFC bpf-next v3 11/11] selftests/bpf: test case for register_bpf_struct_ops() thinker.li
2023-09-26 1:19 ` Martin KaFai Lau
2023-09-26 1:33 ` [RFC bpf-next v3 00/11] Registrating struct_ops types from modules Martin KaFai Lau
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=cc0aa287-e4de-13fe-1727-b1e0e8dbc3ba@linux.dev \
--to=martin.lau@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=kernel-team@meta.com \
--cc=kuifeng@meta.com \
--cc=sinquersw@gmail.com \
--cc=song@kernel.org \
--cc=thinker.li@gmail.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.