diff mbox series

[mptcp-next,v2,03/11] mptcp: add bpf_iter_task for mptcp_sched_data

Message ID 620223de348631c8b8bf798b46c18002160c9235.1741347233.git.tanggeliang@kylinos.cn (mailing list archive)
State Changes Requested
Headers show
Series add bpf_iter_task | expand

Checks

Context Check Description
matttbe/checkpatch warning total: 0 errors, 2 warnings, 0 checks, 30 lines checked
matttbe/shellcheck success MPTCP selftests files have not been modified
matttbe/build success Build and static analysis OK
matttbe/KVM_Validation__normal success Success! ✅
matttbe/KVM_Validation__debug success Success! ✅
matttbe/KVM_Validation__btf-normal__only_bpftest_all_ success Success! ✅
matttbe/KVM_Validation__btf-debug__only_bpftest_all_ success Success! ✅

Commit Message

Geliang Tang March 7, 2025, 11:36 a.m. UTC 
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_sched_data.

2. Do a WRITE_ONCE(data->bpf_iter_task, current) before calling
a MPTCP BPF hook, and WRITE_ONCE(data->bpf_iter_task, NULL) after
the hook returns.

3. In bpf_iter_mptcp_subflow_new(), check

	"READ_ONCE(data->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>
---
 include/net/mptcp.h  |  1 +
 net/mptcp/protocol.h | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

Comments

Matthieu Baerts (NGI0) March 8, 2025, 10:49 a.m. UTC | #1 
Hi Geliang,

On 07/03/2025 12:36, Geliang Tang wrote:
> 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_sched_data.
> 
> 2. Do a WRITE_ONCE(data->bpf_iter_task, current) before calling
> a MPTCP BPF hook, and WRITE_ONCE(data->bpf_iter_task, NULL) after
> the hook returns.
> 
> 3. In bpf_iter_mptcp_subflow_new(), check
> 
> 	"READ_ONCE(data->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.

I think it is better not to have dedicated patches introducing helpers
without using them: for the reviewers, it is not clear how they are
exactly going to be used, then it is needed to check the next patch(es),
go back to this one, etc.

Maybe better to introduce the helper when you actually use them?
Especially for such simple helpers.

Cheers,
Matt
diff mbox series

Patch

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 2c85ca92bb1c..bd3d1b3654dd 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -105,6 +105,7 @@  struct mptcp_out_options {
 struct mptcp_sched_data {
 	u8	subflows;
 	struct mptcp_subflow_context *contexts[MPTCP_SUBFLOWS_MAX];
+	struct task_struct *bpf_iter_task;
 };
 
 struct mptcp_sched_ops {
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3492b256ecba..0875b3cf4aba 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1291,4 +1291,23 @@  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_sched_set_bpf_iter_task(struct mptcp_sched_data *data)
+{
+	WRITE_ONCE(data->bpf_iter_task, current);
+}
+
+static inline void mptcp_sched_clear_bpf_iter_task(struct mptcp_sched_data *data)
+{
+	WRITE_ONCE(data->bpf_iter_task, NULL);
+}
+
+static inline bool mptcp_sched_check_bpf_iter_task(struct mptcp_sched_data *data)
+{
+	struct task_struct *task = READ_ONCE(data->bpf_iter_task);
+
+	if (task && task == current)
+		return true;
+	return false;
+}
+
 #endif /* __MPTCP_PROTOCOL_H */