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

Geliang Tang posted 2 patches 1 year, 1 month ago
Maintainers: "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Matthieu Baerts <matthieu.baerts@tessares.net>, Mat Martineau <martineau@kernel.org>
There is a newer version of this series
[RFC mptcp-next v2 2/2] mptcp: add bpf_burst set/get params
Posted by Geliang Tang 1 year, 1 month ago
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;
 }
 
 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
Re: [RFC mptcp-next v2 2/2] mptcp: add bpf_burst set/get params
Posted by Mat Martineau 1 year, 1 month ago
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
>
>
>