Re: [PATCH mptcp-next v2 1/4] bpf: Add mptcp_subflow bpf_iter

[Geliang Tang <geliang@kernel.org>]

On Mon, 2024-09-09 at 09:36 +0800, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> It's necessary to traverse all subflows on the conn_list of an MPTCP
> socket and then call kfunc to modify the fields of each subflow. In
> kernel space, mptcp_for_each_subflow() helper is used for this:
> 
> 	mptcp_for_each_subflow(msk, subflow)
> 		kfunc(subflow);
> 
> But in the MPTCP BPF program, this has not yet been implemented. As
> Martin suggested recently, this conn_list walking + modify-by-kfunc
> usage fits the bpf_iter use case. So this patch adds a new bpf_iter
> type named "mptcp_subflow" to do this and implements its helpers
> bpf_iter_mptcp_subflow_new()/_next()/_destroy().
> 
> Then bpf_for_each() for mptcp_subflow can be used in BPF program like
> this:
> 
> 	bpf_rcu_read_lock();
> 	bpf_for_each(mptcp_subflow, subflow, msk)
> 		kfunc(subflow);
> 	bpf_rcu_read_unlock();
> 
> Suggested-by: Martin KaFai Lau <martin.lau@kernel.org>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/bpf.c      | 51
> ++++++++++++++++++++++++++++++++++++++++++++
>  net/mptcp/protocol.h |  6 ++++++
>  2 files changed, 57 insertions(+)
> 
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index 9672a70c24b0..799264119891 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -204,10 +204,59 @@ static const struct btf_kfunc_id_set
> bpf_mptcp_fmodret_set = {
>  	.set   = &bpf_mptcp_fmodret_ids,
>  };
>  
> +struct bpf_iter__mptcp_subflow {
> +	__bpf_md_ptr(struct bpf_iter_meta *, meta);
> +	__bpf_md_ptr(struct mptcp_sock *, msk);
> +	__bpf_md_ptr(struct list_head *, pos);
> +};

This bpf_iter__mptcp_subflow struct should be dropped too.

> +
> +struct bpf_iter_mptcp_subflow {
> +	__u64 __opaque[2];
> +} __attribute__((aligned(8)));
> +
> +struct bpf_iter_mptcp_subflow_kern {
> +	struct mptcp_sock *msk;
> +	struct list_head *pos;
> +} __attribute__((aligned(8)));
> +
>  __diag_push();
>  __diag_ignore_all("-Wmissing-prototypes",
>  		  "kfuncs which will be used in BPF programs");

Duplicate with __bpf_kfunc_start_defs/__bpf_kfunc_end_defs,
__diag_push, __diag_pop and __diag_ignore_all should be dropped.

>  
> +__bpf_kfunc_start_defs();
> +
> +__bpf_kfunc int bpf_iter_mptcp_subflow_new(struct
> bpf_iter_mptcp_subflow *it,
> +					   struct mptcp_sock *msk)
> +{
> +	struct bpf_iter_mptcp_subflow_kern *kit = (void *)it;
> +
> +	if (!msk)
> +		return -EINVAL;
> +
> +	kit->msk = msk;
> +	kit->pos = &msk->conn_list;
> +	return 0;
> +}
> +
> +__bpf_kfunc struct mptcp_subflow_context *
> +bpf_iter_mptcp_subflow_next(struct bpf_iter_mptcp_subflow *it)
> +{
> +	struct bpf_iter_mptcp_subflow_kern *kit = (void *)it;
> +	struct mptcp_subflow_context *subflow;
> +	struct mptcp_sock *msk = kit->msk;
> +
> +	subflow = list_entry((kit->pos)->next, struct
> mptcp_subflow_context, node);
> +	if (!msk || list_entry_is_head(subflow, &msk->conn_list,
> node))
> +		return NULL;
> +
> +	kit->pos = &subflow->node;
> +	return subflow;
> +}
> +
> +__bpf_kfunc void bpf_iter_mptcp_subflow_destroy(struct
> bpf_iter_mptcp_subflow *it)
> +{
> +}
> +
>  __bpf_kfunc struct mptcp_subflow_context *
>  bpf_mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data,
> unsigned int pos)
>  {
> @@ -221,6 +270,8 @@ __bpf_kfunc bool
> bpf_mptcp_subflow_queues_empty(struct sock *sk)
>  	return tcp_rtx_queue_empty(sk);
>  }
>  
> +__bpf_kfunc_end_defs();
> +
>  __diag_pop();
>  
>  BTF_KFUNCS_START(bpf_mptcp_sched_kfunc_ids)
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index d25d2dac88a5..b3f5254e3c0d 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -715,6 +715,12 @@ void mptcp_subflow_queue_clean(struct sock *sk,
> struct sock *ssk);
>  void mptcp_sock_graft(struct sock *sk, struct socket *parent);
>  u64 mptcp_wnd_end(const struct mptcp_sock *msk);
>  void mptcp_set_timeout(struct sock *sk);
> +struct bpf_iter_mptcp_subflow;
> +int bpf_iter_mptcp_subflow_new(struct bpf_iter_mptcp_subflow *it,
> +			       struct mptcp_sock *msk);
> +struct mptcp_subflow_context *
> +bpf_iter_mptcp_subflow_next(struct bpf_iter_mptcp_subflow *it);
> +void bpf_iter_mptcp_subflow_destroy(struct bpf_iter_mptcp_subflow
> *it);

No need to add these declarations, since "-Wmissing-declarations" is
ignored in __bpf_kfunc_start_defs.

Will update in v3.

Thanks,
-Geliang

>  bool bpf_mptcp_subflow_queues_empty(struct sock *sk);
>  struct mptcp_subflow_context *
>  bpf_mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data,
> unsigned int pos);