Re: [PATCH mptcp-next v13 7/9] mptcp: add call_me_again flag

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

On Thu, 21 Apr 2022, Geliang Tang wrote:

> For supporting a "redundant" packet scheduler in the future, this patch
> adds a flag of struct mptcp_sock named call_me_again to indicate that
> get_subflow() function needs to be called again.
>
> Export it in bpf_mptcp_helpers.h, and add BPF write access to it.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/bpf.c                                 | 16 ++++++++++++++++
> net/mptcp/protocol.h                            |  1 +
> net/mptcp/sched.c                               |  1 +
> tools/testing/selftests/bpf/bpf_mptcp_helpers.h |  1 +
> 4 files changed, 19 insertions(+)
>
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index e849fc3fb6c5..1611dbe63eb2 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -40,6 +40,7 @@ static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
> {
> 	const struct btf_type *state;
> 	u32 type_id;
> +	size_t end;
>
> 	if (atype == BPF_READ)
> 		return btf_struct_access(log, btf, t, off, size, atype,
> @@ -55,6 +56,21 @@ static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
> 		return -EACCES;
> 	}
>
> +	switch (off) {
> +	case offsetofend(struct mptcp_sock, sched):
> +		end = offsetofend(struct mptcp_sock, sched) + sizeof(u8);
> +		break;
> +	default:
> +		bpf_log(log, "no write support to mptcp_sock at off %d\n", off);
> +		return -EACCES;
> +	}
> +
> +	if (off + size > end) {
> +		bpf_log(log, "access beyond mptcp_sock at off %u size %u ended at %zu",
> +			off, size, end);
> +		return -EACCES;
> +	}
> +
> 	return NOT_INIT;
> }
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index f31bc3271bcc..13c6ad5fbade 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -299,6 +299,7 @@ struct mptcp_sock {
> 	struct sock	*first;
> 	struct mptcp_pm_data	pm;
> 	struct mptcp_sched_ops	*sched;
> +	u8		call_me_again:1;

This should be part of the get_subflow function call in mptcp_sched_ops, 
not part of the socket state. We need some BPF-compatible way to return 
both a subflow pointer and a boolean from the call to the scheduler.

Seems like it should work to add a new struct like this:

struct mptcp_sched_data {
 	struct sock *sock;
 	bool call_again;
};

void (*get_subflow)(struct mptcp_sock *msk, bool reinject, struct mptcp_sched_data *data);


and then modify bpf_mptcp_sched_btf_struct_access() to allow writing 
values? What do you think?


The other part we need to figure out is how to update the transmit code to 
send the same data across multiple subflows with this call_again flag is 
used. I'm not yet sure what to recommend for that yet.

- Mat

> 	struct {
> 		u32	space;	/* bytes copied in last measurement window */
> 		u32	copied; /* bytes copied in this measurement window */
> diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
> index 8025dc51fbe9..05ab45505f88 100644
> --- a/net/mptcp/sched.c
> +++ b/net/mptcp/sched.c
> @@ -83,6 +83,7 @@ int mptcp_init_sched(struct mptcp_sock *msk,
> 	msk->sched = sched_init;
> 	if (msk->sched->init)
> 		msk->sched->init(msk);
> +	msk->call_me_again = 0;
>
> 	pr_debug("sched=%s", msk->sched->name);
>
> diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> index 81a9c5d91aae..dacee63455f5 100644
> --- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> @@ -24,6 +24,7 @@ struct mptcp_sock {
> 	__u32		token;
> 	struct sock	*first;
> 	struct mptcp_sched_ops *sched;
> +	__u8		call_me_again:1;
> 	char		ca_name[TCP_CA_NAME_MAX];
> } __attribute__((preserve_access_index));
>
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel