Re: [RFC mptcp-next v2 2/2] mptcp: add bpf_burst set/get params

[Mat Martineau <martineau@kernel.org>]

On Wed, 23 Aug 2023, Geliang Tang wrote:

> This patch implements the set/get params interfaces for bpf_burst
> scheduler.
>
> Export bpf_sk_storage_lookup(), use it to get bpf_local_storage_data
> from sk->sk_bpf_storage, then get or set its params ptr->snd_burst.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> include/net/bpf_sk_storage.h |  7 +++++++
> net/core/bpf_sk_storage.c    |  2 +-
> net/mptcp/sched.c            | 39 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bpf_sk_storage.h b/include/net/bpf_sk_storage.h
> index 2926f1f00d65..805142d59d02 100644
> --- a/include/net/bpf_sk_storage.h
> +++ b/include/net/bpf_sk_storage.h
> @@ -37,6 +37,8 @@ int bpf_sk_storage_diag_put(struct bpf_sk_storage_diag *diag,
> 			    struct sock *sk, struct sk_buff *skb,
> 			    int stg_array_type,
> 			    unsigned int *res_diag_size);
> +struct bpf_local_storage_data *
> +bpf_sk_storage_lookup(struct sock *sk, struct bpf_map *map, bool cacheit_lockit);
> #else
> static inline int bpf_sk_storage_clone(const struct sock *sk,
> 				       struct sock *newsk)
> @@ -58,6 +60,11 @@ static inline int bpf_sk_storage_diag_put(struct bpf_sk_storage_diag *diag,
> {
> 	return 0;
> }
> +struct bpf_local_storage_data *
> +bpf_sk_storage_lookup(struct sock *sk, struct bpf_map *map, bool cacheit_lockit)
> +{
> +	return NULL;
> +}
> #endif
>
> #endif /* _BPF_SK_STORAGE_H */
> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> index cca7594be92e..1c023f65017f 100644
> --- a/net/core/bpf_sk_storage.c
> +++ b/net/core/bpf_sk_storage.c
> @@ -17,7 +17,7 @@
>
> DEFINE_BPF_STORAGE_CACHE(sk_cache);
>
> -static struct bpf_local_storage_data *
> +struct bpf_local_storage_data *
> bpf_sk_storage_lookup(struct sock *sk, struct bpf_map *map, bool cacheit_lockit)
> {
> 	struct bpf_local_storage *sk_storage;
> diff --git a/net/mptcp/sched.c b/net/mptcp/sched.c
> index c78ed61290d2..067df28899a6 100644
> --- a/net/mptcp/sched.c
> +++ b/net/mptcp/sched.c
> @@ -11,6 +11,7 @@
> #include <linux/list.h>
> #include <linux/rculist.h>
> #include <linux/spinlock.h>
> +#include <net/bpf_sk_storage.h>
> #include "protocol.h"
>
> static DEFINE_SPINLOCK(mptcp_sched_list_lock);
> @@ -202,18 +203,56 @@ mptcp_subflow_ctx_by_pos(const struct mptcp_sched_data *data, unsigned int pos)
>
> int mptcp_sched_get_params(struct mptcp_sock *msk)
> {
> +	struct sock *sk = (struct sock *)msk;
> +
> 	if (msk->sched == &mptcp_sched_default)
> 		return msk->snd_burst;
>
> +	if (sk->sk_bpf_storage && sk->sk_bpf_storage->smap &&
> +	    !strcmp(sk->sk_bpf_storage->smap->map.name, "mptcp_burst_map")) {
> +		struct bpf_local_storage_data *sdata;
> +
> +		sdata = bpf_sk_storage_lookup(sk, &sk->sk_bpf_storage->smap->map, true);
> +		if (sdata) {
> +			struct mptcp_burst_storage {
> +				int snd_burst;
> +			} *ptr;
> +
> +			ptr = (struct mptcp_burst_storage *)sdata->data;
> +			if (ptr)
> +				return ptr->snd_burst;
> +		}
> +	}
> +
> 	return 0;
> }

This seems very complex. Why implement it this way versus adding snd_burst 
to 'struct mptcp_sock' in bpf_tcp_helpers.h?

- Mat


>
> int mptcp_sched_set_params(struct mptcp_sock *msk, int burst)
> {
> +	struct sock *sk = (struct sock *)msk;
> +
> 	if (msk->sched == &mptcp_sched_default) {
> 		msk->snd_burst = burst;
> 		return 0;
> 	}
>
> +	if (sk->sk_bpf_storage && sk->sk_bpf_storage->smap &&
> +	    !strcmp(sk->sk_bpf_storage->smap->map.name, "mptcp_burst_map")) {
> +		struct bpf_local_storage_data *sdata;
> +
> +		sdata = bpf_sk_storage_lookup(sk, &sk->sk_bpf_storage->smap->map, true);
> +		if (sdata) {
> +			struct mptcp_burst_storage {
> +				int snd_burst;
> +			} *ptr;
> +
> +			ptr = (struct mptcp_burst_storage *)sdata->data;
> +			if (ptr) {
> +				ptr->snd_burst = burst;
> +				return 0;
> +			}
> +		}
> +	}
> +
> 	return 0;
> }
> -- 
> 2.35.3
>
>
>