Re: [PATCH mptcp-next v4 01/10] Squash to "mptcp: add struct mptcp_sched_ops"

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Tue, 31 May 2022, Geliang Tang wrote:

> Use bitmap instead of sock in struct mptcp_sched_data.
>
> Please update the commit log:
>
> '''
> This patch defines struct mptcp_sched_ops, which has three struct members,
> name, owner and list, and three function pointers, init, release and
> get_subflow.
>
> Add the scheduler registering, unregistering and finding functions to add,
> delete and find a packet scheduler on the global list mptcp_sched_list.
>
> The BPF scheduler function get_subflow() has a struct mptcp_sched_data
> parameter, which contains a mptcp_subflow_context array. Add a new member
> scheduled for mptcp_subflow_context, which will be set in the MPTCP
> scheduler context when the scheduler picks this subflow to send data.
> '''
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> include/net/mptcp.h                           |  7 ++++---
> net/mptcp/protocol.h                          |  1 +
> tools/testing/selftests/bpf/bpf_tcp_helpers.h | 11 ++++++++---
> 3 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 6456ea26e4c7..7af7fd48acc7 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -97,14 +97,15 @@ struct mptcp_out_options {
> };
>
> #define MPTCP_SCHED_NAME_MAX	16
> +#define MPTCP_SUBFLOWS_MAX	8
>
> struct mptcp_sched_data {
> -	struct sock	*sock;
> -	bool		call_again;
> +	bool	reinject;
> +	struct mptcp_subflow_context *contexts[MPTCP_SUBFLOWS_MAX];

Ok - the BPF verifier is able to handle this array? That's good news.

> };
>
> struct mptcp_sched_ops {
> -	void (*get_subflow)(const struct mptcp_sock *msk, bool reinject,
> +	void (*get_subflow)(const struct mptcp_sock *msk,
> 			    struct mptcp_sched_data *data);
>
> 	char			name[MPTCP_SCHED_NAME_MAX];
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 8739794166d8..48c5261b7b15 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -469,6 +469,7 @@ struct mptcp_subflow_context {
> 		valid_csum_seen : 1;        /* at least one csum validated */
> 	enum mptcp_data_avail data_avail;
> 	bool	mp_fail_response_expect;
> +	bool	scheduled;
> 	u32	remote_nonce;
> 	u64	thmac;
> 	u32	local_nonce;
> diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> index aca4e3c6ac48..a705054f38c5 100644
> --- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> @@ -231,10 +231,15 @@ extern __u32 tcp_slow_start(struct tcp_sock *tp, __u32 acked) __ksym;
> extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym;
>
> #define MPTCP_SCHED_NAME_MAX	16
> +#define MPTCP_SUBFLOWS_MAX	8
> +
> +struct mptcp_subflow_context {
> +	bool	scheduled;
> +} __attribute__((preserve_access_index));

Ah, is this the array "magic"?

>
> struct mptcp_sched_data {
> -	struct sock	*sock;
> -	bool		call_again;
> +	bool	reinject;
> +	struct mptcp_subflow_context *contexts[MPTCP_SUBFLOWS_MAX];
> };
>
> struct mptcp_sched_ops {
> @@ -243,7 +248,7 @@ struct mptcp_sched_ops {
> 	void (*init)(const struct mptcp_sock *msk);
> 	void (*release)(const struct mptcp_sock *msk);
>
> -	void (*get_subflow)(const struct mptcp_sock *msk, bool reinject,
> +	void (*get_subflow)(const struct mptcp_sock *msk,
> 			    struct mptcp_sched_data *data);
> 	void *owner;
> };
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel