Re: [PATCH mptcp-next v10 1/3] mptcp: add subflows array in sched data

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

On Sun, 1 May 2022, Geliang Tang wrote:

> This patch adds a subflow pointers array in struct mptcp_sched_data. Set
> the array before invoking get_subflow(), then get it in get_subflow() in
> the BPF contexts.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> include/net/mptcp.h                             | 3 +++
> net/mptcp/sched.c                               | 7 +++++++
> tools/testing/selftests/bpf/bpf_mptcp_helpers.h | 3 +++
> 3 files changed, 13 insertions(+)
>
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index bea7608d72d3..1a48e31f3ac7 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -96,10 +96,13 @@ 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;
> +	u8		subflows;
> +	struct mptcp_subflow_context *array[MPTCP_SUBFLOWS_MAX];

I suggest a more descriptive name here: 'contexts' instead of 'array'?

> };
>
> struct mptcp_sched_ops {
> diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
> index 7a5654132ed3..1efbcfe80fe7 100644
> --- a/net/mptcp/sched.c
> +++ b/net/mptcp/sched.c
> @@ -93,9 +93,16 @@ void mptcp_release_sched(struct mptcp_sock *msk)
> static int mptcp_sched_data_init(struct mptcp_sock *msk,
> 				 struct mptcp_sched_data *data)
> {
> +	struct mptcp_subflow_context *subflow;
> +	int i = 0;
> +
> 	data->sock = NULL;
> 	data->call_again = 0;
>
> +	mptcp_for_each_subflow(msk, subflow)
> +		data->array[i++] = subflow;

Two things here:

The unused array elements should be set to NULL.

There should be strict checking of the MPTCP_SUBFLOWS_MAX array size 
limit, and if the length of conn_list is larger then pr_warn_once() about 
that unexpected condition.

- Mat

> +	data->subflows = i;
> +
> 	return 0;
> }
>
> diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> index e863954de701..1f991ff2e325 100644
> --- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> @@ -7,10 +7,13 @@
> #include "bpf_tcp_helpers.h"
>
> #define MPTCP_SCHED_NAME_MAX	16
> +#define MPTCP_SUBFLOWS_MAX	8
>
> struct mptcp_sched_data {
> 	struct sock	*sock;
> 	bool		call_again;
> +	__u8		subflows;
> +	struct mptcp_subflow_context *array[MPTCP_SUBFLOWS_MAX];
> };
>
> struct mptcp_sched_ops {
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel