Re: [PATCH mptcp-next v2 5/5] Squash to "selftests/bpf: add bpf_first scheduler"

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

On Mon, 23 May 2022, Geliang Tang wrote:

> Add set_bit() helper and use new get_subflow API.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> tools/testing/selftests/bpf/bpf_tcp_helpers.h       | 11 +++++++++++
> tools/testing/selftests/bpf/progs/mptcp_bpf_first.c | 10 ++++++----
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> index cb3db7ea36b9..9c7d33e106a4 100644
> --- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> @@ -260,4 +260,15 @@ struct mptcp_sock {
> 	char		ca_name[TCP_CA_NAME_MAX];
> } __attribute__((preserve_access_index));
>
> +#define _AC(X,Y)	(X##Y)
> +#define UL(x)		(_AC(x, UL))
> +
> +static inline void set_bit(unsigned int nr, volatile unsigned long *addr)
> +{
> +        unsigned long *p = ((unsigned long *)addr) + (nr / sizeof(unsigned long));
> +        unsigned long mask = UL(1) << (nr % sizeof(unsigned long));
> +
> +        *p  |= mask;
> +}
> +
> #endif
> diff --git a/tools/testing/selftests/bpf/progs/mptcp_bpf_first.c b/tools/testing/selftests/bpf/progs/mptcp_bpf_first.c
> index fd67b5f42964..e5dc53965642 100644
> --- a/tools/testing/selftests/bpf/progs/mptcp_bpf_first.c
> +++ b/tools/testing/selftests/bpf/progs/mptcp_bpf_first.c
> @@ -16,11 +16,13 @@ void BPF_PROG(mptcp_sched_first_release, const struct mptcp_sock *msk)
> {
> }
>
> -void BPF_STRUCT_OPS(bpf_first_get_subflow, const struct mptcp_sock *msk,
> -		    bool reinject, struct mptcp_sched_data *data)
> +unsigned long BPF_STRUCT_OPS(bpf_first_get_subflow, const struct mptcp_sock *msk,
> +			     bool reinject, struct mptcp_sched_data *data)
> {
> -	data->sock = msk->first;
> -	data->call_again = 0;
> +	unsigned long bitmap = 0;
> +
> +	set_bit(0, &bitmap);
> +	return bitmap;

It might be more realistic to return the first non-backup subflow (or 
first backup if there are only backup subflows). Do you think that would 
be better as a replacement for this test or as an additional test?

- Mat

> }
>
> SEC(".struct_ops")
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel