Re: [PATCH mptcp-next v2 08/13] mptcp: add subflow_set_stale helper

[Mat Martineau <martineau@kernel.org>]

On Tue, 18 Jul 2023, Geliang Tang wrote:

> Add mptcp_subflow_set_stale() helper.
>

I was trying to remember the use case for this. The closest thing I can 
find is this:

     => the scheduler should be able to interact with the "stale" logic
        we have in the upstream kernel: mark a subflow as stale / back
        active.

from 
https://lore.kernel.org/mptcp/dd4363e7-d057-97e8-0b5f-8570f39aa538@tessares.net/

I think that need is met by the change in patch 11 to export 
mptcp_pm_subflow_chk_stale - if a particular scheduler wants to track some 
stale-like metric, it can do so with its own sk_storage values. There's 
also the matter of coordinating the values of stale_count and 
stale_rcv_tstamp.

My suggestion is to drop patches 8, 9, and 10 unless there's a compelling 
reason to allow a BPF scheduler to control this bit. If there's a need for 
it please speak up!


- Mat



> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/bpf.c      | 1 +
> net/mptcp/protocol.h | 2 ++
> net/mptcp/sched.c    | 6 ++++++
> 3 files changed, 9 insertions(+)
>
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index f388baf08d49..e6f94d5e22fb 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -146,6 +146,7 @@ struct bpf_struct_ops bpf_mptcp_sched_ops = {
>
> BTF_SET8_START(bpf_mptcp_sched_kfunc_ids)
> BTF_ID_FLAGS(func, mptcp_subflow_set_scheduled)
> +BTF_ID_FLAGS(func, mptcp_subflow_set_stale)
> BTF_ID_FLAGS(func, mptcp_sched_data_set_contexts)
> BTF_ID_FLAGS(func, mptcp_subflow_ctx_by_pos)
> BTF_SET8_END(bpf_mptcp_sched_kfunc_ids)
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 1b4457c44fe8..78b3beeb7d7a 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -662,6 +662,8 @@ int mptcp_init_sched(struct mptcp_sock *msk,
> void mptcp_release_sched(struct mptcp_sock *msk);
> void mptcp_subflow_set_scheduled(struct mptcp_subflow_context *subflow,
> 				 bool scheduled);
> +void mptcp_subflow_set_stale(struct mptcp_subflow_context *subflow,
> +			     int stale);
> void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
> 				   struct mptcp_sched_data *data);
> struct mptcp_subflow_context *
> diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
> index a80cf0481edf..3f361e75e38c 100644
> --- a/net/mptcp/sched.c
> +++ b/net/mptcp/sched.c
> @@ -127,6 +127,12 @@ void mptcp_subflow_set_scheduled(struct mptcp_subflow_context *subflow,
> 	WRITE_ONCE(subflow->scheduled, scheduled);
> }
>
> +void mptcp_subflow_set_stale(struct mptcp_subflow_context *subflow,
> +			     int stale)
> +{
> +	subflow->stale = stale;
> +}
> +
> void mptcp_sched_data_set_contexts(const struct mptcp_sock *msk,
> 				   struct mptcp_sched_data *data)
> {
> -- 
> 2.35.3
>
>
>