From: Geliang Tang <tanggeliang@kylinos.cn> v3: - patch 3, continue to use sock_owned_by_user_nocheck() and spin_is_locked() checks instead of using msk_owned_by_me(). - patch 5, drop declaration of bpf_mptcp_subflow_tcp_sock. It's no longer used. - patch 5, update the comment for mptcp_subflow_tcp_sock(), which is a BPF helper, not a kfunc. The commit log of "bpf: Register mptcp common kfunc set" doesn't match the code, please update it as: ''' bpf: Register mptcp common kfunc set MPTCP helper mptcp_subflow_ctx() is used to convert struct sock to struct mptcp_subflow_context. It will be used in MPTCP BPF programs. This patch defines corresponding wrapper of this helper, and put it into the newly defined mptcp common kfunc set and register this set with the flag BPF_PROG_TYPE_CGROUP_SOCKOPT to let it accessible to the 'cgroup/getsockopt' type of BPF programs. ''' v2: - Drop bpf_skc_to_mptcp_sock - Check the owner before assigning the msk as Mat suggested. - Use bpf_core_cast() in mptcp_subflow bpf_iter subtest instead of using bpf_skc_to_mptcp_sock(). Address Martin's suggestions for "Add mptcp_subflow bpf_iter support" v2. Geliang Tang (5): Revert "bpf: Extend bpf_skc_to_mptcp_sock to MPTCP sock" Revert "bpf: Allow use of skc_to_mptcp_sock in cg_sockopt" Squash to "bpf: Add mptcp_subflow bpf_iter" Revert "bpf: Acquire and release mptcp socket" Squash to "selftests/bpf: Add mptcp_subflow bpf_iter subtest" include/net/mptcp.h | 4 +- kernel/bpf/cgroup.c | 2 - net/core/filter.c | 2 +- net/mptcp/bpf.c | 41 +++++-------------- .../testing/selftests/bpf/bpf_experimental.h | 2 +- tools/testing/selftests/bpf/progs/mptcp_bpf.h | 5 --- .../selftests/bpf/progs/mptcp_bpf_iters.c | 10 ++--- 7 files changed, 17 insertions(+), 49 deletions(-) -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> bpf_core_cast() can be used in mptcp_subflow bpf_iter selftests to get the msk, instead of using bpf_skc_to_mptcp_sock(). No need to add this patch anymore, revert it. Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> --- include/net/mptcp.h | 4 ++-- net/core/filter.c | 2 +- net/mptcp/bpf.c | 10 ++-------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/include/net/mptcp.h b/include/net/mptcp.h index XXXXXXX..XXXXXXX 100644 --- a/include/net/mptcp.h +++ b/include/net/mptcp.h @@ -XXX,XX +XXX,XX @@ static inline void mptcpv6_handle_mapped(struct sock *sk, bool mapped) { } #endif #if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_SYSCALL) -struct mptcp_sock *bpf_mptcp_sock_from_sock(struct sock *sk); +struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk); #else -static inline struct mptcp_sock *bpf_mptcp_sock_from_sock(struct sock *sk) { return NULL; } +static inline struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk) { return NULL; } #endif #if !IS_ENABLED(CONFIG_MPTCP) diff --git a/net/core/filter.c b/net/core/filter.c index XXXXXXX..XXXXXXX 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -XXX,XX +XXX,XX @@ const struct bpf_func_proto bpf_skc_to_unix_sock_proto = { BPF_CALL_1(bpf_skc_to_mptcp_sock, struct sock *, sk) { BTF_TYPE_EMIT(struct mptcp_sock); - return (unsigned long)bpf_mptcp_sock_from_sock(sk); + return (unsigned long)bpf_mptcp_sock_from_subflow(sk); } const struct bpf_func_proto bpf_skc_to_mptcp_sock_proto = { diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/bpf.c +++ b/net/mptcp/bpf.c @@ -XXX,XX +XXX,XX @@ static struct bpf_struct_ops bpf_mptcp_sched_ops = { }; #endif /* CONFIG_BPF_JIT */ -struct mptcp_sock *bpf_mptcp_sock_from_sock(struct sock *sk) +struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk) { - if (unlikely(!sk || !sk_fullsock(sk))) - return NULL; - - if (sk->sk_protocol == IPPROTO_MPTCP) - return mptcp_sk(sk); - - if (sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) + if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) return mptcp_sk(mptcp_subflow_ctx(sk)->conn); return NULL; -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> bpf_core_cast() can be used in mptcp_subflow bpf_iter selftests to get the msk, instead of using bpf_skc_to_mptcp_sock(). No need to add this patch anymore, revert it. This reverts commit d8d42f5a4542c2dc31e4e00c58508a821c6ab789. --- kernel/bpf/cgroup.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index XXXXXXX..XXXXXXX 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -XXX,XX +XXX,XX @@ cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) #ifdef CONFIG_INET case BPF_FUNC_tcp_sock: return &bpf_tcp_sock_proto; - case BPF_FUNC_skc_to_mptcp_sock: - return &bpf_skc_to_mptcp_sock_proto; #endif case BPF_FUNC_perf_event_output: return &bpf_event_output_data_proto; -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> Drop the NULL check for 'msk' as Martin suggested, add more checks for 'sk'. Use the "struct sock *sk" instead of "struct mptcp-sock *msk" as the argument in the bpf_iter_mptcp_subflow_new as Martin suggested. v3: - continue to use sock_owned_by_user_nocheck and spin_is_locked checks instead of using msk_owned_by_me(). v2: - check the owner before assigning the msk as Mat suggested. Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> --- net/mptcp/bpf.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/bpf.c +++ b/net/mptcp/bpf.c @@ -XXX,XX +XXX,XX @@ bpf_mptcp_subflow_ctx(const struct sock *sk) __bpf_kfunc static int bpf_iter_mptcp_subflow_new(struct bpf_iter_mptcp_subflow *it, - struct mptcp_sock *msk) + struct sock *sk) { struct bpf_iter_mptcp_subflow_kern *kit = (void *)it; - struct sock *sk = (struct sock *)msk; + struct mptcp_sock *msk; BUILD_BUG_ON(sizeof(struct bpf_iter_mptcp_subflow_kern) > sizeof(struct bpf_iter_mptcp_subflow)); BUILD_BUG_ON(__alignof__(struct bpf_iter_mptcp_subflow_kern) != __alignof__(struct bpf_iter_mptcp_subflow)); - kit->msk = msk; - if (!msk) + if (unlikely(!sk || !sk_fullsock(sk))) + return -EINVAL; + + if (sk->sk_protocol != IPPROTO_MPTCP) return -EINVAL; if (!sock_owned_by_user_nocheck(sk) && !spin_is_locked(&sk->sk_lock.slock)) return -EINVAL; + msk = mptcp_sk(sk); + kit->msk = msk; kit->pos = &msk->conn_list; return 0; } -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> Drop this patch as Martin suggested. Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> --- net/mptcp/bpf.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/bpf.c +++ b/net/mptcp/bpf.c @@ -XXX,XX +XXX,XX @@ bpf_iter_mptcp_subflow_destroy(struct bpf_iter_mptcp_subflow *it) { } -__bpf_kfunc static struct -mptcp_sock *bpf_mptcp_sock_acquire(struct mptcp_sock *msk) -{ - struct sock *sk = (struct sock *)msk; - - if (sk && refcount_inc_not_zero(&sk->sk_refcnt)) - return msk; - return NULL; -} - -__bpf_kfunc static void bpf_mptcp_sock_release(struct mptcp_sock *msk) -{ - struct sock *sk = (struct sock *)msk; - - WARN_ON_ONCE(!sk || !refcount_dec_not_one(&sk->sk_refcnt)); -} - __bpf_kfunc struct mptcp_subflow_context * bpf_mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos) { @@ -XXX,XX +XXX,XX @@ BTF_ID_FLAGS(func, bpf_mptcp_subflow_ctx, KF_RET_NULL) BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_new, KF_ITER_NEW | KF_TRUSTED_ARGS) BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_next, KF_ITER_NEXT | KF_RET_NULL) BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_destroy, KF_ITER_DESTROY) -BTF_ID_FLAGS(func, bpf_mptcp_sock_acquire, KF_ACQUIRE | KF_RET_NULL) -BTF_ID_FLAGS(func, bpf_mptcp_sock_release, KF_RELEASE) BTF_KFUNCS_END(bpf_mptcp_common_kfunc_ids) static const struct btf_kfunc_id_set bpf_mptcp_common_kfunc_set = { -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> Use bpf_core_cast() instead of bpf_skc_to_mptcp_sock(). Change the 2nd parameter type of bpf_for_each() as 'struct sock'. Drop use of bpf_mptcp_sock_acquire/release. Drop declaration of bpf_mptcp_subflow_tcp_sock. It's no longer used. Update the comment for mptcp_subflow_tcp_sock(), which is a BPF helper, not a kfunc. Please update the commit log as: ''' This patch adds a "cgroup/getsockopt" program "iters_subflow" to test the newly added mptcp_subflow bpf_iter. Export mptcp_subflow helpers bpf_iter_mptcp_subflow_new/_next/_destroy and other helpers into bpf_experimental.h. Use bpf_for_each() to walk the subflow list of this msk. MPTCP-specific packet scheduler kfunc can be called in the loop. In this test, just add all subflow ids to local variable local_ids, then invoke the helper mptcp_subflow_tcp_sock() in the loop to pick a subsocket. Out of the loop, use bpf_mptcp_subflow_ctx() to get the subflow context of the picked subsocket and do some verification. Finally, assign local_ids to global variable ids so that the application can obtain this value. Add a subtest named test_iters_subflow to load and verify the newly added mptcp_subflow type bpf_iter example in test_mptcp. Use the helper endpoint_init() to add 3 new subflow endpoints. Send a byte of message to start the mptcp connection, and wait for new subflows to be added. getsockopt() is invoked to trigger the "cgroup/getsockopt" test program "iters_subflow". Check if skel->bss->ids equals 10 to verify whether this mptcp_subflow bpf_iter loops correctly as expected. ''' Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> --- tools/testing/selftests/bpf/bpf_experimental.h | 2 +- tools/testing/selftests/bpf/progs/mptcp_bpf.h | 5 ----- tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c | 10 +++------- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h index XXXXXXX..XXXXXXX 100644 --- a/tools/testing/selftests/bpf/bpf_experimental.h +++ b/tools/testing/selftests/bpf/bpf_experimental.h @@ -XXX,XX +XXX,XX @@ extern void bpf_iter_css_destroy(struct bpf_iter_css *it) __weak __ksym; struct bpf_iter_mptcp_subflow; extern int bpf_iter_mptcp_subflow_new(struct bpf_iter_mptcp_subflow *it, - struct mptcp_sock *msk) __weak __ksym; + struct sock *sk) __weak __ksym; extern struct mptcp_subflow_context * bpf_iter_mptcp_subflow_next(struct bpf_iter_mptcp_subflow *it) __weak __ksym; extern void diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf.h b/tools/testing/selftests/bpf/progs/mptcp_bpf.h index XXXXXXX..XXXXXXX 100644 --- a/tools/testing/selftests/bpf/progs/mptcp_bpf.h +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf.h @@ -XXX,XX +XXX,XX @@ mptcp_subflow_tcp_sock(const struct mptcp_subflow_context *subflow) } /* ksym */ -extern struct mptcp_sock *bpf_mptcp_sock_acquire(struct mptcp_sock *msk) __ksym; -extern void bpf_mptcp_sock_release(struct mptcp_sock *msk) __ksym; - extern struct mptcp_subflow_context * bpf_mptcp_subflow_ctx(const struct sock *sk) __ksym; -extern struct sock * -bpf_mptcp_subflow_tcp_sock(const struct mptcp_subflow_context *subflow) __ksym; extern void mptcp_subflow_set_scheduled(struct mptcp_subflow_context *subflow, bool scheduled) __ksym; diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c index XXXXXXX..XXXXXXX 100644 --- a/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c @@ -XXX,XX +XXX,XX @@ int iters_subflow(struct bpf_sockopt *ctx) if (ctx->level != SOL_TCP || ctx->optname != TCP_IS_MPTCP) return 1; - msk = bpf_skc_to_mptcp_sock(sk); + msk = bpf_core_cast(sk, struct mptcp_sock); if (!msk || msk->pm.server_side || !msk->pm.subflows) return 1; - msk = bpf_mptcp_sock_acquire(msk); - if (!msk) - return 1; - bpf_for_each(mptcp_subflow, subflow, msk) { + bpf_for_each(mptcp_subflow, subflow, (struct sock *)sk) { /* Here MPTCP-specific packet scheduler kfunc can be called: * this test is not doing anything really useful, only to * verify the iteration works. @@ -XXX,XX +XXX,XX @@ int iters_subflow(struct bpf_sockopt *ctx) local_ids += subflow->subflow_id; - /* only to check the following kfunc works */ + /* only to check the following helper works */ ssk = mptcp_subflow_tcp_sock(subflow); } @@ -XXX,XX +XXX,XX @@ int iters_subflow(struct bpf_sockopt *ctx) ids = local_ids; out: - bpf_mptcp_sock_release(msk); return 1; } -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> v5: - check bpf_iter_task in mptcp_subflow_new() as Mat suggested. v4: - drop sock_owned_by_user_nocheck and spin_is_locked. According to comments from Mat and Martin, in this set mptcp_subflow bpf_iter only used from a cg sockopt bpf prog, no need to add these check at this moment. v3: - patch 3, continue to use sock_owned_by_user_nocheck() and spin_is_locked() checks instead of using msk_owned_by_me(). - patch 5, drop declaration of bpf_mptcp_subflow_tcp_sock. It's no longer used. - patch 5, update the comment for mptcp_subflow_tcp_sock(), which is a BPF helper, not a kfunc. The commit log of "bpf: Register mptcp common kfunc set" doesn't match the code, please update it as: ''' bpf: Register mptcp common kfunc set MPTCP helper mptcp_subflow_ctx() is used to convert struct sock to struct mptcp_subflow_context. It will be used in MPTCP BPF programs. This patch defines corresponding wrapper of this helper, and put it into the newly defined mptcp common kfunc set and register this set with the flag BPF_PROG_TYPE_CGROUP_SOCKOPT to let it accessible to the 'cgroup/getsockopt' type of BPF programs. ''' v2: - Drop bpf_skc_to_mptcp_sock - Check the owner before assigning the msk as Mat suggested. - Use bpf_core_cast() in mptcp_subflow bpf_iter subtest instead of using bpf_skc_to_mptcp_sock(). Address Martin's suggestions for "Add mptcp_subflow bpf_iter support" v2. Geliang Tang (5): mptcp: add bpf_iter_task for mptcp_sock Squash to "bpf: Extend bpf_skc_to_mptcp_sock to MPTCP sock" Squash to "bpf: Add mptcp_subflow bpf_iter" Revert "bpf: Acquire and release mptcp socket" Squash to "selftests/bpf: Add mptcp_subflow bpf_iter subtest" net/mptcp/bpf.c | 56 +++++++++---------- net/mptcp/protocol.c | 1 + net/mptcp/protocol.h | 16 ++++++ .../testing/selftests/bpf/bpf_experimental.h | 2 +- tools/testing/selftests/bpf/progs/mptcp_bpf.h | 5 -- .../selftests/bpf/progs/mptcp_bpf_iters.c | 8 +-- 6 files changed, 47 insertions(+), 41 deletions(-) -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> To make sure the mptcp_subflow bpf_iter is running in the MPTCP context. This patch adds a simplified version of tracking for it: 1. Add a 'struct task_struct *bpf_iter_task' field to struct mptcp_sock. 2. Do a WRITE_ONCE(msk->bpf_iter_task, current) before calling a MPTCP BPF hook, and WRITE_ONCE(msk->bpf_iter_task, NULL) after the hook returns. 3. In bpf_iter_mptcp_subflow_new(), check "READ_ONCE(msk->bpf_scheduler_task) == current" to confirm the correct task, return -EINVAL if it doesn't match. Also creates helpers for setting, clearing and checking that value. Suggested-by: Mat Martineau <martineau@kernel.org> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> --- net/mptcp/protocol.c | 1 + net/mptcp/protocol.h | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -XXX,XX +XXX,XX @@ static void __mptcp_init_sock(struct sock *sk) msk->scaling_ratio = TCP_DEFAULT_SCALING_RATIO; WRITE_ONCE(msk->first, NULL); + WRITE_ONCE(msk->bpf_iter_task, NULL); inet_csk(sk)->icsk_sync_mss = mptcp_sync_mss; WRITE_ONCE(msk->csum_enabled, mptcp_is_checksum_enabled(sock_net(sk))); WRITE_ONCE(msk->allow_infinite_fallback, true); diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -XXX,XX +XXX,XX @@ struct mptcp_sock { struct list_head conn_list; struct list_head rtx_queue; struct mptcp_data_frag *first_pending; + struct task_struct *bpf_iter_task; struct list_head join_list; struct sock *first; /* The mptcp ops can safely dereference, using suitable * ONCE annotation, the subflow outside the socket @@ -XXX,XX +XXX,XX @@ mptcp_token_join_cookie_init_state(struct mptcp_subflow_request_sock *subflow_re static inline void mptcp_join_cookie_init(void) {} #endif +static inline void mptcp_set_bpf_iter_task(struct mptcp_sock *msk) +{ + WRITE_ONCE(msk->bpf_iter_task, current); +} + +static inline void mptcp_clear_bpf_iter_task(struct mptcp_sock *msk) +{ + WRITE_ONCE(msk->bpf_iter_task, NULL); +} + +static inline struct task_struct *mptcp_get_bpf_iter_task(struct mptcp_sock *msk) +{ + return READ_ONCE(msk->bpf_iter_task); +} + #endif /* __MPTCP_PROTOCOL_H */ -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> Set msk->bpf_iter_task in bpf_mptcp_sock_from_sock() to allow mptcp_subflow bpt_iter can be used in cgroup/getsockopt, otherwise, the selftest in this set fails. Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> --- net/mptcp/bpf.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/bpf.c +++ b/net/mptcp/bpf.c @@ -XXX,XX +XXX,XX @@ static struct bpf_struct_ops bpf_mptcp_sched_ops = { struct mptcp_sock *bpf_mptcp_sock_from_sock(struct sock *sk) { + struct mptcp_sock *msk; + if (unlikely(!sk || !sk_fullsock(sk))) return NULL; - if (sk->sk_protocol == IPPROTO_MPTCP) - return mptcp_sk(sk); + if (sk->sk_protocol == IPPROTO_MPTCP) { + msk = mptcp_sk(sk); + mptcp_set_bpf_iter_task(msk); + return msk; + } - if (sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) - return mptcp_sk(mptcp_subflow_ctx(sk)->conn); + if (sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) { + msk = mptcp_sk(mptcp_subflow_ctx(sk)->conn); + mptcp_set_bpf_iter_task(msk); + return msk; + } return NULL; } -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> Drop the NULL check for 'msk' as Martin suggested, add more checks for 'sk'. Use the "struct sock *sk" instead of "struct mptcp-sock *msk" as the argument in the bpf_iter_mptcp_subflow_new as Martin suggested. v5: - check bpf_iter_task in mptcp_subflow_new() v4: - drop sock_owned_by_user_nocheck and spin_is_locked. According to comments from Mat [2] and Martin [1], in this set mptcp_subflow bpf_iter only used from a cg sockopt bpf prog, no need to add these check at this moment. [1] https://lore.kernel.org/all/fdf0ddbe-e007-4a5f-bbdf-9a144e8fbe35@linux.dev/ [2] https://patchwork.kernel.org/project/mptcp/patch/f6469225598beecbf0bda12a4c33fafa86c0ff15.1739787744.git.tanggeliang@kylinos.cn/ v3: - continue to use sock_owned_by_user_nocheck and spin_is_locked checks instead of using msk_owned_by_me(). v2: - check the owner before assigning the msk as Mat suggested. Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> --- net/mptcp/bpf.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/bpf.c +++ b/net/mptcp/bpf.c @@ -XXX,XX +XXX,XX @@ bpf_mptcp_subflow_ctx(const struct sock *sk) __bpf_kfunc static int bpf_iter_mptcp_subflow_new(struct bpf_iter_mptcp_subflow *it, - struct mptcp_sock *msk) + struct sock *sk) { struct bpf_iter_mptcp_subflow_kern *kit = (void *)it; - struct sock *sk = (struct sock *)msk; + struct task_struct *task; + struct mptcp_sock *msk; BUILD_BUG_ON(sizeof(struct bpf_iter_mptcp_subflow_kern) > sizeof(struct bpf_iter_mptcp_subflow)); BUILD_BUG_ON(__alignof__(struct bpf_iter_mptcp_subflow_kern) != __alignof__(struct bpf_iter_mptcp_subflow)); - kit->msk = msk; - if (!msk) + if (unlikely(!sk || !sk_fullsock(sk))) return -EINVAL; - if (!sock_owned_by_user_nocheck(sk) && - !spin_is_locked(&sk->sk_lock.slock)) + if (sk->sk_protocol != IPPROTO_MPTCP) return -EINVAL; + msk = mptcp_sk(sk); + task = mptcp_get_bpf_iter_task(msk); + if (!task || task != current) + return -EINVAL; + + mptcp_clear_bpf_iter_task(msk); + + msk_owned_by_me(msk); + + kit->msk = msk; kit->pos = &msk->conn_list; return 0; } -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> Drop this patch as Martin suggested. From Martin's review [1], this mptcp_sock_acquire() helper was a workaround only to please the verifier, but they were not needed. [1] https://lore.kernel.org/9b373a23-c093-42d8-b4ae-99f2e62e7681@linux.dev Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> --- net/mptcp/bpf.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/bpf.c +++ b/net/mptcp/bpf.c @@ -XXX,XX +XXX,XX @@ bpf_iter_mptcp_subflow_destroy(struct bpf_iter_mptcp_subflow *it) { } -__bpf_kfunc static struct -mptcp_sock *bpf_mptcp_sock_acquire(struct mptcp_sock *msk) -{ - struct sock *sk = (struct sock *)msk; - - if (sk && refcount_inc_not_zero(&sk->sk_refcnt)) - return msk; - return NULL; -} - -__bpf_kfunc static void bpf_mptcp_sock_release(struct mptcp_sock *msk) -{ - struct sock *sk = (struct sock *)msk; - - WARN_ON_ONCE(!sk || !refcount_dec_not_one(&sk->sk_refcnt)); -} - __bpf_kfunc struct mptcp_subflow_context * bpf_mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos) { @@ -XXX,XX +XXX,XX @@ BTF_ID_FLAGS(func, bpf_mptcp_subflow_ctx, KF_RET_NULL) BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_new, KF_ITER_NEW | KF_TRUSTED_ARGS) BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_next, KF_ITER_NEXT | KF_RET_NULL) BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_destroy, KF_ITER_DESTROY) -BTF_ID_FLAGS(func, bpf_mptcp_sock_acquire, KF_ACQUIRE | KF_RET_NULL) -BTF_ID_FLAGS(func, bpf_mptcp_sock_release, KF_RELEASE) BTF_KFUNCS_END(bpf_mptcp_common_kfunc_ids) static const struct btf_kfunc_id_set bpf_mptcp_common_kfunc_set = { -- 2.43.0 From: Geliang Tang <tanggeliang@kylinos.cn> Use bpf_core_cast() instead of bpf_skc_to_mptcp_sock(). Change the 2nd parameter type of bpf_for_each() as 'struct sock'. Drop use of bpf_mptcp_sock_acquire/release. Drop declaration of bpf_mptcp_subflow_tcp_sock. It's no longer used. Update the comment for mptcp_subflow_tcp_sock(), which is a BPF helper, not a kfunc. Please update the commit log as: ''' This patch adds a "cgroup/getsockopt" program "iters_subflow" to test the newly added mptcp_subflow bpf_iter. Export mptcp_subflow helpers bpf_iter_mptcp_subflow_new/_next/_destroy and other helpers into bpf_experimental.h. Use bpf_for_each() to walk the subflow list of this msk. MPTCP-specific packet scheduler kfunc can be called in the loop. In this test, just add all subflow ids to local variable local_ids, then invoke the helper mptcp_subflow_tcp_sock() in the loop to pick a subsocket. Out of the loop, use bpf_mptcp_subflow_ctx() to get the subflow context of the picked subsocket and do some verification. Finally, assign local_ids to global variable ids so that the application can obtain this value. Add a subtest named test_iters_subflow to load and verify the newly added mptcp_subflow type bpf_iter example in test_mptcp. Use the helper endpoint_init() to add 3 new subflow endpoints. Send a byte of message to start the mptcp connection, and wait for new subflows to be added. getsockopt() is invoked to trigger the "cgroup/getsockopt" test program "iters_subflow". Check if skel->bss->ids equals 10 to verify whether this mptcp_subflow bpf_iter loops correctly as expected. ''' Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> --- tools/testing/selftests/bpf/bpf_experimental.h | 2 +- tools/testing/selftests/bpf/progs/mptcp_bpf.h | 5 ----- tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c | 8 ++------ 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h index XXXXXXX..XXXXXXX 100644 --- a/tools/testing/selftests/bpf/bpf_experimental.h +++ b/tools/testing/selftests/bpf/bpf_experimental.h @@ -XXX,XX +XXX,XX @@ extern void bpf_iter_css_destroy(struct bpf_iter_css *it) __weak __ksym; struct bpf_iter_mptcp_subflow; extern int bpf_iter_mptcp_subflow_new(struct bpf_iter_mptcp_subflow *it, - struct mptcp_sock *msk) __weak __ksym; + struct sock *sk) __weak __ksym; extern struct mptcp_subflow_context * bpf_iter_mptcp_subflow_next(struct bpf_iter_mptcp_subflow *it) __weak __ksym; extern void diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf.h b/tools/testing/selftests/bpf/progs/mptcp_bpf.h index XXXXXXX..XXXXXXX 100644 --- a/tools/testing/selftests/bpf/progs/mptcp_bpf.h +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf.h @@ -XXX,XX +XXX,XX @@ mptcp_subflow_tcp_sock(const struct mptcp_subflow_context *subflow) } /* ksym */ -extern struct mptcp_sock *bpf_mptcp_sock_acquire(struct mptcp_sock *msk) __ksym; -extern void bpf_mptcp_sock_release(struct mptcp_sock *msk) __ksym; - extern struct mptcp_subflow_context * bpf_mptcp_subflow_ctx(const struct sock *sk) __ksym; -extern struct sock * -bpf_mptcp_subflow_tcp_sock(const struct mptcp_subflow_context *subflow) __ksym; extern void mptcp_subflow_set_scheduled(struct mptcp_subflow_context *subflow, bool scheduled) __ksym; diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c index XXXXXXX..XXXXXXX 100644 --- a/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_iters.c @@ -XXX,XX +XXX,XX @@ int iters_subflow(struct bpf_sockopt *ctx) if (!msk || msk->pm.server_side || !msk->pm.subflows) return 1; - msk = bpf_mptcp_sock_acquire(msk); - if (!msk) - return 1; - bpf_for_each(mptcp_subflow, subflow, msk) { + bpf_for_each(mptcp_subflow, subflow, (struct sock *)sk) { /* Here MPTCP-specific packet scheduler kfunc can be called: * this test is not doing anything really useful, only to * verify the iteration works. @@ -XXX,XX +XXX,XX @@ int iters_subflow(struct bpf_sockopt *ctx) local_ids += subflow->subflow_id; - /* only to check the following kfunc works */ + /* only to check the following helper works */ ssk = mptcp_subflow_tcp_sock(subflow); } @@ -XXX,XX +XXX,XX @@ int iters_subflow(struct bpf_sockopt *ctx) ids = local_ids; out: - bpf_mptcp_sock_release(msk); return 1; } -- 2.43.0