[mptcp-next,v2,05/11] mptcp: add mptcp_subflow_sched bpf_iter

Commit Message

Geliang Tang March 7, 2025, 11:36 a.m. UTC

From: Geliang Tang <[e-mail]>

Add a new bpt_iter mptcp_subflow_sched, it's a wrapper of mptcp_subflow.

Signed-off-by: Geliang Tang <[e-mail]>
---
 net/mptcp/bpf.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Matthieu Baerts (NGI0) March 8, 2025, 11 a.m. UTC | #1

Hi Geliang,

On 07/03/2025 12:36, Geliang Tang wrote:
> From: Geliang Tang <[e-mail]>
> 
> Add a new bpt_iter mptcp_subflow_sched, it's a wrapper of mptcp_subflow.
> 
> Signed-off-by: Geliang Tang <[e-mail]>
> ---
>  net/mptcp/bpf.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index 3921261b4fd4..c31f7b407b84 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -288,6 +288,31 @@ bpf_iter_mptcp_subflow_destroy(struct bpf_iter_mptcp_subflow *it)
>  {
>  }
>  
> +struct bpf_iter_mptcp_subflow_sched {
> +	struct bpf_iter_mptcp_subflow it;
> +};
> +
> +__bpf_kfunc static int
> +bpf_iter_mptcp_subflow_sched_new(struct bpf_iter_mptcp_subflow_sched *it,
> +				 struct sock *sk, struct mptcp_sched_data *data)
> +{
> +	if (!mptcp_sched_check_bpf_iter_task(data))
> +		return -EINVAL;
> +
> +	return bpf_iter_mptcp_subflow_new(&it->it, sk);

We would need the same thing for the BPF PM I suppose, no?

Then, why not having something more generic, either:

 - add "struct task_struct *" in the msk structure

 - or pass a "const struct task_struct *bpf_iter_task" instead of
something sched specific?

For the first idea, I guess it should work because the helper will be
called when the msk lock will be owned by the caller. So we should not
have the PM and scheduler setting it at the same time from different
context, right?
Also, if this helper receives a msk in argument, no need to check that
it is a valid MPTCP socket, etc., right?

If one of these two ideas is used, no need to re-introduce
mptcp_sched_data. The API is not settled yet, so no need to worry about
that. It is even a good time to change it.

Cheers,
Matt

Patch

diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index 3921261b4fd4..c31f7b407b84 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -288,6 +288,31 @@  bpf_iter_mptcp_subflow_destroy(struct bpf_iter_mptcp_subflow *it)
 {
 }
 
+struct bpf_iter_mptcp_subflow_sched {
+	struct bpf_iter_mptcp_subflow it;
+};
+
+__bpf_kfunc static int
+bpf_iter_mptcp_subflow_sched_new(struct bpf_iter_mptcp_subflow_sched *it,
+				 struct sock *sk, struct mptcp_sched_data *data)
+{
+	if (!mptcp_sched_check_bpf_iter_task(data))
+		return -EINVAL;
+
+	return bpf_iter_mptcp_subflow_new(&it->it, sk);
+}
+
+__bpf_kfunc static struct mptcp_subflow_context *
+bpf_iter_mptcp_subflow_sched_next(struct bpf_iter_mptcp_subflow_sched *it)
+{
+	return bpf_iter_mptcp_subflow_next(&it->it);
+}
+
+__bpf_kfunc static void
+bpf_iter_mptcp_subflow_sched_destroy(struct bpf_iter_mptcp_subflow_sched *it)
+{
+}
+
 __bpf_kfunc static bool bpf_mptcp_subflow_queues_empty(struct sock *sk)
 {
 	return tcp_rtx_queue_empty(sk);
@@ -312,6 +337,9 @@  BTF_ID_FLAGS(func, bpf_mptcp_subflow_tcp_sock, 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_iter_mptcp_subflow_sched_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_sched_next, KF_ITER_NEXT | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_iter_mptcp_subflow_sched_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, mptcp_subflow_set_scheduled)
 BTF_ID_FLAGS(func, mptcp_subflow_active)
 BTF_ID_FLAGS(func, mptcp_set_timeout)