All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next v4 0/7] bpf: mptcp: Support for mptcp_sock
@ 2022-05-13 22:48 Mat Martineau
  2022-05-13 22:48 ` [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto Mat Martineau
                   ` (6 more replies)
  0 siblings, 7 replies; 25+ messages in thread
From: Mat Martineau @ 2022-05-13 22:48 UTC (permalink / raw)
  To: netdev, bpf; +Cc: Mat Martineau, ast, daniel, andrii, mptcp, geliang.tang

This patch set adds BPF access to mptcp_sock structures, along with
associated self tests. You may recognize some of the code from earlier
(https://lore.kernel.org/bpf/20200918121046.190240-6-nicolas.rybowski@tessares.net/)
but it has been reworked quite a bit.


v1 -> v2: Emit BTF type, add func_id checks in verifier.c and bpf_trace.c,
remove build check for CONFIG_BPF_JIT, add selftest check for CONFIG_MPTCP,
and add a patch to include CONFIG_IKCONFIG/CONFIG_IKCONFIG_PROC for the
BPF self tests.

v2 -> v3: Access sysctl through the filesystem to work around CI use of
the more limited busybox sysctl command.

v3 -> v4: Dropped special case kernel code for tcp_sock is_mptcp, use
existing bpf_tcp_helpers.h, and add check for 'ip mptcp monitor' support.

Geliang Tang (6):
  bpf: add bpf_skc_to_mptcp_sock_proto
  selftests/bpf: Enable CONFIG_IKCONFIG_PROC in config
  selftests/bpf: test bpf_skc_to_mptcp_sock
  selftests/bpf: verify token of struct mptcp_sock
  selftests/bpf: verify ca_name of struct mptcp_sock
  selftests/bpf: verify first of struct mptcp_sock

Nicolas Rybowski (1):
  selftests/bpf: add MPTCP test base

 MAINTAINERS                                   |   1 +
 include/linux/bpf.h                           |   1 +
 include/linux/btf_ids.h                       |   3 +-
 include/net/mptcp.h                           |   6 +
 include/uapi/linux/bpf.h                      |   7 +
 kernel/bpf/verifier.c                         |   1 +
 kernel/trace/bpf_trace.c                      |   2 +
 net/core/filter.c                             |  18 ++
 net/mptcp/Makefile                            |   2 +
 net/mptcp/bpf.c                               |  22 ++
 scripts/bpf_doc.py                            |   2 +
 tools/include/uapi/linux/bpf.h                |   7 +
 tools/testing/selftests/bpf/bpf_tcp_helpers.h |  13 +
 tools/testing/selftests/bpf/config            |   3 +
 tools/testing/selftests/bpf/network_helpers.c |  43 ++-
 tools/testing/selftests/bpf/network_helpers.h |   4 +
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 265 ++++++++++++++++++
 .../testing/selftests/bpf/progs/mptcp_sock.c  |  82 ++++++
 18 files changed, 473 insertions(+), 9 deletions(-)
 create mode 100644 net/mptcp/bpf.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/mptcp.c
 create mode 100644 tools/testing/selftests/bpf/progs/mptcp_sock.c


base-commit: 0d2d2648931bdb1a629bf0df4e339e6a326a6136
-- 
2.36.1


^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto
  2022-05-13 22:48 [PATCH bpf-next v4 0/7] bpf: mptcp: Support for mptcp_sock Mat Martineau
@ 2022-05-13 22:48 ` Mat Martineau
  2022-05-17  1:07   ` Martin KaFai Lau
  2022-05-13 22:48 ` [PATCH bpf-next v4 2/7] selftests/bpf: Enable CONFIG_IKCONFIG_PROC in config Mat Martineau
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Mat Martineau @ 2022-05-13 22:48 UTC (permalink / raw)
  To: netdev, bpf
  Cc: Geliang Tang, ast, daniel, andrii, mptcp, Nicolas Rybowski,
	Matthieu Baerts, Mat Martineau

From: Geliang Tang <geliang.tang@suse.com>

This patch implements a new struct bpf_func_proto, named
bpf_skc_to_mptcp_sock_proto. Define a new bpf_id BTF_SOCK_TYPE_MPTCP,
and a new helper bpf_skc_to_mptcp_sock(), which invokes another new
helper bpf_mptcp_sock_from_subflow() in net/mptcp/bpf.c to get struct
mptcp_sock from a given subflow socket.

v2: Emit BTF type, add func_id checks in verifier.c and bpf_trace.c,
remove build check for CONFIG_BPF_JIT

Co-developed-by: Nicolas Rybowski <nicolas.rybowski@tessares.net>
Signed-off-by: Nicolas Rybowski <nicolas.rybowski@tessares.net>
Co-developed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/linux/bpf.h            |  1 +
 include/linux/btf_ids.h        |  3 ++-
 include/net/mptcp.h            |  6 ++++++
 include/uapi/linux/bpf.h       |  7 +++++++
 kernel/bpf/verifier.c          |  1 +
 kernel/trace/bpf_trace.c       |  2 ++
 net/core/filter.c              | 18 ++++++++++++++++++
 net/mptcp/Makefile             |  2 ++
 net/mptcp/bpf.c                | 22 ++++++++++++++++++++++
 scripts/bpf_doc.py             |  2 ++
 tools/include/uapi/linux/bpf.h |  7 +++++++
 11 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 net/mptcp/bpf.c

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5061ccd8b2dc..c129c2e2dd95 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2229,6 +2229,7 @@ extern const struct bpf_func_proto bpf_skc_to_tcp_timewait_sock_proto;
 extern const struct bpf_func_proto bpf_skc_to_tcp_request_sock_proto;
 extern const struct bpf_func_proto bpf_skc_to_udp6_sock_proto;
 extern const struct bpf_func_proto bpf_skc_to_unix_sock_proto;
+extern const struct bpf_func_proto bpf_skc_to_mptcp_sock_proto;
 extern const struct bpf_func_proto bpf_copy_from_user_proto;
 extern const struct bpf_func_proto bpf_snprintf_btf_proto;
 extern const struct bpf_func_proto bpf_snprintf_proto;
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index bc5d9cc34e4c..335a19092368 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -178,7 +178,8 @@ extern struct btf_id_set name;
 	BTF_SOCK_TYPE(BTF_SOCK_TYPE_TCP6, tcp6_sock)			\
 	BTF_SOCK_TYPE(BTF_SOCK_TYPE_UDP, udp_sock)			\
 	BTF_SOCK_TYPE(BTF_SOCK_TYPE_UDP6, udp6_sock)			\
-	BTF_SOCK_TYPE(BTF_SOCK_TYPE_UNIX, unix_sock)
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_UNIX, unix_sock)			\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_MPTCP, mptcp_sock)
 
 enum {
 #define BTF_SOCK_TYPE(name, str) name,
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 8b1afd6f5cc4..2ba09de955c7 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -284,4 +284,10 @@ static inline int mptcpv6_init(void) { return 0; }
 static inline void mptcpv6_handle_mapped(struct sock *sk, bool mapped) { }
 #endif
 
+#if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_SYSCALL)
+struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk);
+#else
+static inline struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk) { return NULL; }
+#endif
+
 #endif /* __NET_MPTCP_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0210f85131b3..56688bee20d9 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5172,6 +5172,12 @@ union bpf_attr {
  * 	Return
  * 		Map value associated to *key* on *cpu*, or **NULL** if no entry
  * 		was found or *cpu* is invalid.
+ *
+ * struct mptcp_sock *bpf_skc_to_mptcp_sock(void *sk)
+ *	Description
+ *		Dynamically cast a *sk* pointer to a *mptcp_sock* pointer.
+ *	Return
+ *		*sk* if casting is valid, or **NULL** otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -5370,6 +5376,7 @@ union bpf_attr {
 	FN(ima_file_hash),		\
 	FN(kptr_xchg),			\
 	FN(map_lookup_percpu_elem),     \
+	FN(skc_to_mptcp_sock),		\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 05c1b6656824..10ed10b24860 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -509,6 +509,7 @@ static bool is_ptr_cast_function(enum bpf_func_id func_id)
 		func_id == BPF_FUNC_skc_to_tcp_sock ||
 		func_id == BPF_FUNC_skc_to_tcp6_sock ||
 		func_id == BPF_FUNC_skc_to_udp6_sock ||
+		func_id == BPF_FUNC_skc_to_mptcp_sock ||
 		func_id == BPF_FUNC_skc_to_tcp_timewait_sock ||
 		func_id == BPF_FUNC_skc_to_tcp_request_sock;
 }
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 7141ca8a1c2d..10b157a6d73e 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1705,6 +1705,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_skc_to_udp6_sock_proto;
 	case BPF_FUNC_skc_to_unix_sock:
 		return &bpf_skc_to_unix_sock_proto;
+	case BPF_FUNC_skc_to_mptcp_sock:
+		return &bpf_skc_to_mptcp_sock_proto;
 	case BPF_FUNC_sk_storage_get:
 		return &bpf_sk_storage_get_tracing_proto;
 	case BPF_FUNC_sk_storage_delete:
diff --git a/net/core/filter.c b/net/core/filter.c
index fe0da529d00f..5af58eb48587 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -78,6 +78,7 @@
 #include <linux/btf_ids.h>
 #include <net/tls.h>
 #include <net/xdp.h>
+#include <net/mptcp.h>
 
 static const struct bpf_func_proto *
 bpf_sk_base_func_proto(enum bpf_func_id func_id);
@@ -11281,6 +11282,20 @@ const struct bpf_func_proto bpf_skc_to_unix_sock_proto = {
 	.ret_btf_id		= &btf_sock_ids[BTF_SOCK_TYPE_UNIX],
 };
 
+BPF_CALL_1(bpf_skc_to_mptcp_sock, struct sock *, sk)
+{
+	BTF_TYPE_EMIT(struct mptcp_sock);
+	return (unsigned long)bpf_mptcp_sock_from_subflow(sk);
+}
+
+const struct bpf_func_proto bpf_skc_to_mptcp_sock_proto = {
+	.func		= bpf_skc_to_mptcp_sock,
+	.gpl_only	= false,
+	.ret_type	= RET_PTR_TO_BTF_ID_OR_NULL,
+	.arg1_type	= ARG_PTR_TO_SOCK_COMMON,
+	.ret_btf_id	= &btf_sock_ids[BTF_SOCK_TYPE_MPTCP],
+};
+
 BPF_CALL_1(bpf_sock_from_file, struct file *, file)
 {
 	return (unsigned long)sock_from_file(file);
@@ -11323,6 +11338,9 @@ bpf_sk_base_func_proto(enum bpf_func_id func_id)
 	case BPF_FUNC_skc_to_unix_sock:
 		func = &bpf_skc_to_unix_sock_proto;
 		break;
+	case BPF_FUNC_skc_to_mptcp_sock:
+		func = &bpf_skc_to_mptcp_sock_proto;
+		break;
 	case BPF_FUNC_ktime_get_coarse_ns:
 		return &bpf_ktime_get_coarse_ns_proto;
 	default:
diff --git a/net/mptcp/Makefile b/net/mptcp/Makefile
index e54daceac58b..99dddf08ca73 100644
--- a/net/mptcp/Makefile
+++ b/net/mptcp/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_INET_MPTCP_DIAG) += mptcp_diag.o
 mptcp_crypto_test-objs := crypto_test.o
 mptcp_token_test-objs := token_test.o
 obj-$(CONFIG_MPTCP_KUNIT_TEST) += mptcp_crypto_test.o mptcp_token_test.o
+
+obj-$(CONFIG_BPF_SYSCALL) += bpf.o
diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
new file mode 100644
index 000000000000..535602ba2582
--- /dev/null
+++ b/net/mptcp/bpf.c
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Multipath TCP
+ *
+ * Copyright (c) 2020, Tessares SA.
+ * Copyright (c) 2022, SUSE.
+ *
+ * Author: Nicolas Rybowski <nicolas.rybowski@tessares.net>
+ */
+
+#define pr_fmt(fmt) "MPTCP: " fmt
+
+#include <linux/bpf.h>
+#include "protocol.h"
+
+struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
+{
+	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
+		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
+
+	return NULL;
+}
+EXPORT_SYMBOL(bpf_mptcp_sock_from_subflow);
diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
index 096625242475..d5452f7eb996 100755
--- a/scripts/bpf_doc.py
+++ b/scripts/bpf_doc.py
@@ -633,6 +633,7 @@ class PrinterHelpers(Printer):
             'struct socket',
             'struct file',
             'struct bpf_timer',
+            'struct mptcp_sock',
     ]
     known_types = {
             '...',
@@ -682,6 +683,7 @@ class PrinterHelpers(Printer):
             'struct socket',
             'struct file',
             'struct bpf_timer',
+            'struct mptcp_sock',
     }
     mapped_types = {
             'u8': '__u8',
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 0210f85131b3..56688bee20d9 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5172,6 +5172,12 @@ union bpf_attr {
  * 	Return
  * 		Map value associated to *key* on *cpu*, or **NULL** if no entry
  * 		was found or *cpu* is invalid.
+ *
+ * struct mptcp_sock *bpf_skc_to_mptcp_sock(void *sk)
+ *	Description
+ *		Dynamically cast a *sk* pointer to a *mptcp_sock* pointer.
+ *	Return
+ *		*sk* if casting is valid, or **NULL** otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -5370,6 +5376,7 @@ union bpf_attr {
 	FN(ima_file_hash),		\
 	FN(kptr_xchg),			\
 	FN(map_lookup_percpu_elem),     \
+	FN(skc_to_mptcp_sock),		\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCH bpf-next v4 2/7] selftests/bpf: Enable CONFIG_IKCONFIG_PROC in config
  2022-05-13 22:48 [PATCH bpf-next v4 0/7] bpf: mptcp: Support for mptcp_sock Mat Martineau
  2022-05-13 22:48 ` [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto Mat Martineau
@ 2022-05-13 22:48 ` Mat Martineau
  2022-05-13 22:48 ` [PATCH bpf-next v4 3/7] selftests/bpf: add MPTCP test base Mat Martineau
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 25+ messages in thread
From: Mat Martineau @ 2022-05-13 22:48 UTC (permalink / raw)
  To: netdev, bpf; +Cc: Geliang Tang, ast, daniel, andrii, mptcp, Mat Martineau

From: Geliang Tang <geliang.tang@suse.com>

CONFIG_IKCONFIG_PROC is required by BPF selftests, otherwise we get
errors like this:

 libbpf: failed to open system Kconfig
 libbpf: failed to load object 'kprobe_multi'
 libbpf: failed to load BPF skeleton 'kprobe_multi': -22

It's because /proc/config.gz is opened in bpf_object__read_kconfig_file()
in tools/lib/bpf/libbpf.c:

        file = gzopen("/proc/config.gz", "r");

So this patch enables CONFIG_IKCONFIG and CONFIG_IKCONFIG_PROC in
tools/testing/selftests/bpf/config.

Suggested-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 tools/testing/selftests/bpf/config | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 08c6e5a66d87..6840d4625e01 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -54,3 +54,5 @@ CONFIG_NF_DEFRAG_IPV6=y
 CONFIG_NF_CONNTRACK=y
 CONFIG_USERFAULTFD=y
 CONFIG_FPROBE=y
+CONFIG_IKCONFIG=y
+CONFIG_IKCONFIG_PROC=y
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCH bpf-next v4 3/7] selftests/bpf: add MPTCP test base
  2022-05-13 22:48 [PATCH bpf-next v4 0/7] bpf: mptcp: Support for mptcp_sock Mat Martineau
  2022-05-13 22:48 ` [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto Mat Martineau
  2022-05-13 22:48 ` [PATCH bpf-next v4 2/7] selftests/bpf: Enable CONFIG_IKCONFIG_PROC in config Mat Martineau
@ 2022-05-13 22:48 ` Mat Martineau
  2022-05-16 22:43   ` Andrii Nakryiko
  2022-05-17  1:18   ` Martin KaFai Lau
  2022-05-13 22:48 ` [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock Mat Martineau
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 25+ messages in thread
From: Mat Martineau @ 2022-05-13 22:48 UTC (permalink / raw)
  To: netdev, bpf
  Cc: Nicolas Rybowski, ast, daniel, andrii, mptcp, Matthieu Baerts,
	Geliang Tang, Mat Martineau

From: Nicolas Rybowski <nicolas.rybowski@tessares.net>

This patch adds a base for MPTCP specific tests.

It is currently limited to the is_mptcp field in case of plain TCP
connection because there is no easy way to get the subflow sk from a msk
in userspace. This implies that we cannot lookup the sk_storage attached
to the subflow sk in the sockops program.

v4:
 - add copyright 2022 (Andrii)
 - use ASSERT_* instead of CHECK_FAIL (Andrii)
 - drop SEC("version") (Andrii)
 - use is_mptcp in tcp_sock, instead of bpf_tcp_sock (Martin & Andrii)

Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Co-developed-by: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Nicolas Rybowski <nicolas.rybowski@tessares.net>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 MAINTAINERS                                   |   1 +
 tools/testing/selftests/bpf/bpf_tcp_helpers.h |   1 +
 tools/testing/selftests/bpf/config            |   1 +
 tools/testing/selftests/bpf/network_helpers.c |  43 ++++--
 tools/testing/selftests/bpf/network_helpers.h |   4 +
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 136 ++++++++++++++++++
 .../testing/selftests/bpf/progs/mptcp_sock.c  |  53 +++++++
 7 files changed, 231 insertions(+), 8 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/mptcp.c
 create mode 100644 tools/testing/selftests/bpf/progs/mptcp_sock.c

diff --git a/MAINTAINERS b/MAINTAINERS
index cc5559a7fb5c..359afc617b92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13780,6 +13780,7 @@ F:	include/net/mptcp.h
 F:	include/trace/events/mptcp.h
 F:	include/uapi/linux/mptcp.h
 F:	net/mptcp/
+F:	tools/testing/selftests/bpf/*/*mptcp*.c
 F:	tools/testing/selftests/net/mptcp/
 
 NETWORKING [TCP]
diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index b1ede6f0b821..22e0c8849a17 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -81,6 +81,7 @@ struct tcp_sock {
 	__u32	lsndtime;
 	__u32	prior_cwnd;
 	__u64	tcp_mstamp;	/* most recent packet received/sent */
+	bool	is_mptcp;
 } __attribute__((preserve_access_index));
 
 static __always_inline struct inet_connection_sock *inet_csk(const struct sock *sk)
diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 6840d4625e01..3b3edc0fc8a6 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -56,3 +56,4 @@ CONFIG_USERFAULTFD=y
 CONFIG_FPROBE=y
 CONFIG_IKCONFIG=y
 CONFIG_IKCONFIG_PROC=y
+CONFIG_MPTCP=y
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 2bb1f9b3841d..c9a2e39e34fc 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -21,6 +21,10 @@
 #include "network_helpers.h"
 #include "test_progs.h"
 
+#ifndef IPPROTO_MPTCP
+#define IPPROTO_MPTCP 262
+#endif
+
 #define clean_errno() (errno == 0 ? "None" : strerror(errno))
 #define log_err(MSG, ...) ({						\
 			int __save = errno;				\
@@ -73,13 +77,13 @@ int settimeo(int fd, int timeout_ms)
 
 #define save_errno_close(fd) ({ int __save = errno; close(fd); errno = __save; })
 
-static int __start_server(int type, const struct sockaddr *addr,
+static int __start_server(int type, int protocol, const struct sockaddr *addr,
 			  socklen_t addrlen, int timeout_ms, bool reuseport)
 {
 	int on = 1;
 	int fd;
 
-	fd = socket(addr->sa_family, type, 0);
+	fd = socket(addr->sa_family, type, protocol);
 	if (fd < 0) {
 		log_err("Failed to create server socket");
 		return -1;
@@ -113,8 +117,8 @@ static int __start_server(int type, const struct sockaddr *addr,
 	return -1;
 }
 
-int start_server(int family, int type, const char *addr_str, __u16 port,
-		 int timeout_ms)
+static int start_server_proto(int family, int type, int protocol,
+			      const char *addr_str, __u16 port, int timeout_ms)
 {
 	struct sockaddr_storage addr;
 	socklen_t addrlen;
@@ -122,10 +126,23 @@ int start_server(int family, int type, const char *addr_str, __u16 port,
 	if (make_sockaddr(family, addr_str, port, &addr, &addrlen))
 		return -1;
 
-	return __start_server(type, (struct sockaddr *)&addr,
+	return __start_server(type, protocol, (struct sockaddr *)&addr,
 			      addrlen, timeout_ms, false);
 }
 
+int start_server(int family, int type, const char *addr_str, __u16 port,
+		 int timeout_ms)
+{
+	return start_server_proto(family, type, 0, addr_str, port, timeout_ms);
+}
+
+int start_mptcp_server(int family, const char *addr_str, __u16 port,
+		       int timeout_ms)
+{
+	return start_server_proto(family, SOCK_STREAM, IPPROTO_MPTCP, addr_str,
+				  port, timeout_ms);
+}
+
 int *start_reuseport_server(int family, int type, const char *addr_str,
 			    __u16 port, int timeout_ms, unsigned int nr_listens)
 {
@@ -144,7 +161,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
 	if (!fds)
 		return NULL;
 
-	fds[0] = __start_server(type, (struct sockaddr *)&addr, addrlen,
+	fds[0] = __start_server(type, 0, (struct sockaddr *)&addr, addrlen,
 				timeout_ms, true);
 	if (fds[0] == -1)
 		goto close_fds;
@@ -154,7 +171,7 @@ int *start_reuseport_server(int family, int type, const char *addr_str,
 		goto close_fds;
 
 	for (; nr_fds < nr_listens; nr_fds++) {
-		fds[nr_fds] = __start_server(type, (struct sockaddr *)&addr,
+		fds[nr_fds] = __start_server(type, 0, (struct sockaddr *)&addr,
 					     addrlen, timeout_ms, true);
 		if (fds[nr_fds] == -1)
 			goto close_fds;
@@ -265,7 +282,7 @@ int connect_to_fd_opts(int server_fd, const struct network_helper_opts *opts)
 	}
 
 	addr_in = (struct sockaddr_in *)&addr;
-	fd = socket(addr_in->sin_family, type, 0);
+	fd = socket(addr_in->sin_family, type, opts->protocol);
 	if (fd < 0) {
 		log_err("Failed to create client socket");
 		return -1;
@@ -298,6 +315,16 @@ int connect_to_fd(int server_fd, int timeout_ms)
 	return connect_to_fd_opts(server_fd, &opts);
 }
 
+int connect_to_mptcp_fd(int server_fd, int timeout_ms)
+{
+	struct network_helper_opts opts = {
+		.timeout_ms = timeout_ms,
+		.protocol = IPPROTO_MPTCP,
+	};
+
+	return connect_to_fd_opts(server_fd, &opts);
+}
+
 int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms)
 {
 	struct sockaddr_storage addr;
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
index a4b3b2f9877b..e0feb115b2ae 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -21,6 +21,7 @@ struct network_helper_opts {
 	const char *cc;
 	int timeout_ms;
 	bool must_fail;
+	int protocol;
 };
 
 /* ipv4 test vector */
@@ -42,11 +43,14 @@ extern struct ipv6_packet pkt_v6;
 int settimeo(int fd, int timeout_ms);
 int start_server(int family, int type, const char *addr, __u16 port,
 		 int timeout_ms);
+int start_mptcp_server(int family, const char *addr, __u16 port,
+		       int timeout_ms);
 int *start_reuseport_server(int family, int type, const char *addr_str,
 			    __u16 port, int timeout_ms,
 			    unsigned int nr_listens);
 void free_fds(int *fds, unsigned int nr_close_fds);
 int connect_to_fd(int server_fd, int timeout_ms);
+int connect_to_mptcp_fd(int server_fd, int timeout_ms);
 int connect_to_fd_opts(int server_fd, const struct network_helper_opts *opts);
 int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms);
 int fastopen_connect(int server_fd, const char *data, unsigned int data_len,
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
new file mode 100644
index 000000000000..cb0389ca8690
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020, Tessares SA. */
+/* Copyright (c) 2022, SUSE. */
+
+#include <test_progs.h>
+#include "cgroup_helpers.h"
+#include "network_helpers.h"
+
+struct mptcp_storage {
+	__u32 invoked;
+	__u32 is_mptcp;
+};
+
+static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp)
+{
+	int err, cfd = client_fd;
+	struct mptcp_storage val;
+
+	if (is_mptcp == 1)
+		return 0;
+
+	err = bpf_map_lookup_elem(map_fd, &cfd, &val);
+	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
+		return err;
+
+	if (val.invoked != 1) {
+		log_err("%s: unexpected invoked count %d != 1",
+			msg, val.invoked);
+		err++;
+	}
+
+	if (val.is_mptcp != 0) {
+		log_err("%s: unexpected bpf_tcp_sock.is_mptcp %d != 0",
+			msg, val.is_mptcp);
+		err++;
+	}
+
+	return err;
+}
+
+static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
+{
+	int client_fd, prog_fd, map_fd, err;
+	struct bpf_program *prog;
+	struct bpf_object *obj;
+	struct bpf_map *map;
+
+	obj = bpf_object__open("./mptcp_sock.o");
+	if (libbpf_get_error(obj))
+		return -EIO;
+
+	err = bpf_object__load(obj);
+	if (!ASSERT_OK(err, "bpf_object__load"))
+		goto out;
+
+	prog = bpf_object__find_program_by_name(obj, "_sockops");
+	if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name")) {
+		err = -EIO;
+		goto out;
+	}
+
+	prog_fd = bpf_program__fd(prog);
+	if (!ASSERT_GE(prog_fd, 0, "bpf_program__fd")) {
+		err = -EIO;
+		goto out;
+	}
+
+	map = bpf_object__find_map_by_name(obj, "socket_storage_map");
+	if (!ASSERT_OK_PTR(map, "bpf_object__find_map_by_name")) {
+		err = -EIO;
+		goto out;
+	}
+
+	map_fd = bpf_map__fd(map);
+	if (!ASSERT_GE(map_fd, 0, "bpf_map__fd")) {
+		err = -EIO;
+		goto out;
+	}
+
+	err = bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_SOCK_OPS, 0);
+	if (!ASSERT_OK(err, "bpf_prog_attach"))
+		goto out;
+
+	client_fd = is_mptcp ? connect_to_mptcp_fd(server_fd, 0) :
+			       connect_to_fd(server_fd, 0);
+	if (!ASSERT_GE(client_fd, 0, "connect to fd")) {
+		err = -EIO;
+		goto out;
+	}
+
+	err += is_mptcp ? verify_sk(map_fd, client_fd, "MPTCP subflow socket", 1) :
+			  verify_sk(map_fd, client_fd, "plain TCP socket", 0);
+
+	close(client_fd);
+
+out:
+	bpf_object__close(obj);
+	return err;
+}
+
+void test_base(void)
+{
+	int server_fd, cgroup_fd;
+
+	cgroup_fd = test__join_cgroup("/mptcp");
+	if (CHECK_FAIL(cgroup_fd < 0))
+		return;
+
+	/* without MPTCP */
+	server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
+	if (CHECK_FAIL(server_fd < 0))
+		goto with_mptcp;
+
+	CHECK_FAIL(run_test(cgroup_fd, server_fd, false));
+
+	close(server_fd);
+
+with_mptcp:
+	/* with MPTCP */
+	server_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
+	if (CHECK_FAIL(server_fd < 0))
+		goto close_cgroup_fd;
+
+	CHECK_FAIL(run_test(cgroup_fd, server_fd, true));
+
+	close(server_fd);
+
+close_cgroup_fd:
+	close(cgroup_fd);
+}
+
+void test_mptcp(void)
+{
+	if (test__start_subtest("base"))
+		test_base();
+}
diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
new file mode 100644
index 000000000000..bc09dba0b078
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020, Tessares SA. */
+/* Copyright (c) 2022, SUSE. */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_tcp_helpers.h"
+
+char _license[] SEC("license") = "GPL";
+
+struct mptcp_storage {
+	__u32 invoked;
+	__u32 is_mptcp;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SK_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, int);
+	__type(value, struct mptcp_storage);
+} socket_storage_map SEC(".maps");
+
+SEC("sockops")
+int _sockops(struct bpf_sock_ops *ctx)
+{
+	struct mptcp_storage *storage;
+	int op = (int)ctx->op;
+	struct tcp_sock *tsk;
+	struct bpf_sock *sk;
+	bool is_mptcp;
+
+	if (op != BPF_SOCK_OPS_TCP_CONNECT_CB)
+		return 1;
+
+	sk = ctx->sk;
+	if (!sk)
+		return 1;
+
+	tsk = bpf_skc_to_tcp_sock(sk);
+	if (!tsk)
+		return 1;
+
+	is_mptcp = bpf_core_field_exists(tsk->is_mptcp) ? tsk->is_mptcp : 0;
+	storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
+				     BPF_SK_STORAGE_GET_F_CREATE);
+	if (!storage)
+		return 1;
+
+	storage->invoked++;
+	storage->is_mptcp = is_mptcp;
+
+	return 1;
+}
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock
  2022-05-13 22:48 [PATCH bpf-next v4 0/7] bpf: mptcp: Support for mptcp_sock Mat Martineau
                   ` (2 preceding siblings ...)
  2022-05-13 22:48 ` [PATCH bpf-next v4 3/7] selftests/bpf: add MPTCP test base Mat Martineau
@ 2022-05-13 22:48 ` Mat Martineau
  2022-05-13 23:53   ` Geliang Tang
                     ` (2 more replies)
  2022-05-13 22:48 ` [PATCH bpf-next v4 5/7] selftests/bpf: verify token of struct mptcp_sock Mat Martineau
                   ` (2 subsequent siblings)
  6 siblings, 3 replies; 25+ messages in thread
From: Mat Martineau @ 2022-05-13 22:48 UTC (permalink / raw)
  To: netdev, bpf
  Cc: Geliang Tang, ast, daniel, andrii, mptcp, Matthieu Baerts, Mat Martineau

From: Geliang Tang <geliang.tang@suse.com>

This patch extends the MPTCP test base, to test the new helper
bpf_skc_to_mptcp_sock().

Define struct mptcp_sock in bpf_tcp_helpers.h, use bpf_skc_to_mptcp_sock
to get the msk socket in progs/mptcp_sock.c and store the infos in
socket_storage_map.

Get the infos from socket_storage_map in prog_tests/mptcp.c. Add a new
function verify_msk() to verify the infos of MPTCP socket, and rename
verify_sk() to verify_tsk() to verify TCP socket only.

v2: Add CONFIG_MPTCP check for clearer error messages
v4:
 - use ASSERT_* instead of CHECK_FAIL (Andrii)
 - drop bpf_mptcp_helpers.h (Andrii)

Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 tools/testing/selftests/bpf/bpf_tcp_helpers.h |  5 +++
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 45 ++++++++++++++-----
 .../testing/selftests/bpf/progs/mptcp_sock.c  | 23 ++++++++--
 3 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index 22e0c8849a17..90fecafc493d 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -226,4 +226,9 @@ static __always_inline bool tcp_cc_eq(const char *a, const char *b)
 extern __u32 tcp_slow_start(struct tcp_sock *tp, __u32 acked) __ksym;
 extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym;
 
+struct mptcp_sock {
+	struct inet_connection_sock	sk;
+
+} __attribute__((preserve_access_index));
+
 #endif
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index cb0389ca8690..02e7fd8918e6 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -11,14 +11,12 @@ struct mptcp_storage {
 	__u32 is_mptcp;
 };
 
-static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp)
+static int verify_tsk(int map_fd, int client_fd)
 {
+	char *msg = "plain TCP socket";
 	int err, cfd = client_fd;
 	struct mptcp_storage val;
 
-	if (is_mptcp == 1)
-		return 0;
-
 	err = bpf_map_lookup_elem(map_fd, &cfd, &val);
 	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
 		return err;
@@ -38,6 +36,31 @@ static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp)
 	return err;
 }
 
+static int verify_msk(int map_fd, int client_fd)
+{
+	char *msg = "MPTCP subflow socket";
+	int err, cfd = client_fd;
+	struct mptcp_storage val;
+
+	err = bpf_map_lookup_elem(map_fd, &cfd, &val);
+	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
+		return err;
+
+	if (val.invoked != 1) {
+		log_err("%s: unexpected invoked count %d != 1",
+			msg, val.invoked);
+		err++;
+	}
+
+	if (val.is_mptcp != 1) {
+		log_err("%s: unexpected bpf_tcp_sock.is_mptcp %d != 1",
+			msg, val.is_mptcp);
+		err++;
+	}
+
+	return err;
+}
+
 static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
 {
 	int client_fd, prog_fd, map_fd, err;
@@ -88,8 +111,8 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
 		goto out;
 	}
 
-	err += is_mptcp ? verify_sk(map_fd, client_fd, "MPTCP subflow socket", 1) :
-			  verify_sk(map_fd, client_fd, "plain TCP socket", 0);
+	err += is_mptcp ? verify_msk(map_fd, client_fd) :
+			  verify_tsk(map_fd, client_fd);
 
 	close(client_fd);
 
@@ -103,25 +126,25 @@ void test_base(void)
 	int server_fd, cgroup_fd;
 
 	cgroup_fd = test__join_cgroup("/mptcp");
-	if (CHECK_FAIL(cgroup_fd < 0))
+	if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup"))
 		return;
 
 	/* without MPTCP */
 	server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
-	if (CHECK_FAIL(server_fd < 0))
+	if (!ASSERT_GE(server_fd, 0, "start_server"))
 		goto with_mptcp;
 
-	CHECK_FAIL(run_test(cgroup_fd, server_fd, false));
+	ASSERT_OK(run_test(cgroup_fd, server_fd, false), "run_test tcp");
 
 	close(server_fd);
 
 with_mptcp:
 	/* with MPTCP */
 	server_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
-	if (CHECK_FAIL(server_fd < 0))
+	if (!ASSERT_GE(server_fd, 0, "start_mptcp_server"))
 		goto close_cgroup_fd;
 
-	CHECK_FAIL(run_test(cgroup_fd, server_fd, true));
+	ASSERT_OK(run_test(cgroup_fd, server_fd, true), "run_test mptcp");
 
 	close(server_fd);
 
diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
index bc09dba0b078..3feb7ff578e2 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -7,6 +7,7 @@
 #include "bpf_tcp_helpers.h"
 
 char _license[] SEC("license") = "GPL";
+extern bool CONFIG_MPTCP __kconfig;
 
 struct mptcp_storage {
 	__u32 invoked;
@@ -24,6 +25,7 @@ SEC("sockops")
 int _sockops(struct bpf_sock_ops *ctx)
 {
 	struct mptcp_storage *storage;
+	struct mptcp_sock *msk;
 	int op = (int)ctx->op;
 	struct tcp_sock *tsk;
 	struct bpf_sock *sk;
@@ -41,11 +43,24 @@ int _sockops(struct bpf_sock_ops *ctx)
 		return 1;
 
 	is_mptcp = bpf_core_field_exists(tsk->is_mptcp) ? tsk->is_mptcp : 0;
-	storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
-				     BPF_SK_STORAGE_GET_F_CREATE);
-	if (!storage)
-		return 1;
+	if (!is_mptcp) {
+		storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
+					     BPF_SK_STORAGE_GET_F_CREATE);
+		if (!storage)
+			return 1;
+	} else {
+		if (!CONFIG_MPTCP)
+			return 1;
+
+		msk = bpf_skc_to_mptcp_sock(sk);
+		if (!msk)
+			return 1;
 
+		storage = bpf_sk_storage_get(&socket_storage_map, msk, 0,
+					     BPF_SK_STORAGE_GET_F_CREATE);
+		if (!storage)
+			return 1;
+	}
 	storage->invoked++;
 	storage->is_mptcp = is_mptcp;
 
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCH bpf-next v4 5/7] selftests/bpf: verify token of struct mptcp_sock
  2022-05-13 22:48 [PATCH bpf-next v4 0/7] bpf: mptcp: Support for mptcp_sock Mat Martineau
                   ` (3 preceding siblings ...)
  2022-05-13 22:48 ` [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock Mat Martineau
@ 2022-05-13 22:48 ` Mat Martineau
  2022-05-16 22:45   ` Andrii Nakryiko
  2022-05-17  1:32   ` Martin KaFai Lau
  2022-05-13 22:48 ` [PATCH bpf-next v4 6/7] selftests/bpf: verify ca_name " Mat Martineau
  2022-05-13 22:48 ` [PATCH bpf-next v4 7/7] selftests/bpf: verify first " Mat Martineau
  6 siblings, 2 replies; 25+ messages in thread
From: Mat Martineau @ 2022-05-13 22:48 UTC (permalink / raw)
  To: netdev, bpf
  Cc: Geliang Tang, ast, daniel, andrii, mptcp, Matthieu Baerts, Mat Martineau

From: Geliang Tang <geliang.tang@suse.com>

This patch verifies the struct member token of struct mptcp_sock. Add a
new function get_msk_token() to parse the msk token from the output of
the command 'ip mptcp monitor', and verify it in verify_msk().

v4:
 - use ASSERT_* instead of CHECK_FAIL (Andrii)
 - skip the test if 'ip mptcp monitor' is not supported (Mat)

Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 tools/testing/selftests/bpf/bpf_tcp_helpers.h |  1 +
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 64 +++++++++++++++++++
 .../testing/selftests/bpf/progs/mptcp_sock.c  |  5 ++
 3 files changed, 70 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index 90fecafc493d..422491872619 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -229,6 +229,7 @@ extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym;
 struct mptcp_sock {
 	struct inet_connection_sock	sk;
 
+	__u32		token;
 } __attribute__((preserve_access_index));
 
 #endif
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 02e7fd8918e6..ac98aa314123 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -9,8 +9,11 @@
 struct mptcp_storage {
 	__u32 invoked;
 	__u32 is_mptcp;
+	__u32 token;
 };
 
+static char monitor_log_path[64];
+
 static int verify_tsk(int map_fd, int client_fd)
 {
 	char *msg = "plain TCP socket";
@@ -36,11 +39,52 @@ static int verify_tsk(int map_fd, int client_fd)
 	return err;
 }
 
+/*
+ * Parse the token from the output of 'ip mptcp monitor':
+ *
+ * [       CREATED] token=3ca933d3 remid=0 locid=0 saddr4=127.0.0.1 ...
+ * [       CREATED] token=2ab57040 remid=0 locid=0 saddr4=127.0.0.1 ...
+ */
+static __u32 get_msk_token(void)
+{
+	char *prefix = "[       CREATED] token=";
+	char buf[BUFSIZ] = {};
+	__u32 token = 0;
+	ssize_t len;
+	int fd;
+
+	sync();
+
+	fd = open(monitor_log_path, O_RDONLY);
+	if (!ASSERT_GE(fd, 0, "Failed to open monitor_log_path"))
+		return token;
+
+	len = read(fd, buf, sizeof(buf));
+	if (!ASSERT_GT(len, 0, "Failed to read monitor_log_path"))
+		goto err;
+
+	if (strncmp(buf, prefix, strlen(prefix))) {
+		log_err("Invalid prefix %s", buf);
+		goto err;
+	}
+
+	token = strtol(buf + strlen(prefix), NULL, 16);
+
+err:
+	close(fd);
+	return token;
+}
+
 static int verify_msk(int map_fd, int client_fd)
 {
 	char *msg = "MPTCP subflow socket";
 	int err, cfd = client_fd;
 	struct mptcp_storage val;
+	__u32 token;
+
+	token = get_msk_token();
+	if (!ASSERT_GT(token, 0, "Unexpected token"))
+		return -1;
 
 	err = bpf_map_lookup_elem(map_fd, &cfd, &val);
 	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
@@ -58,6 +102,12 @@ static int verify_msk(int map_fd, int client_fd)
 		err++;
 	}
 
+	if (val.token != token) {
+		log_err("Unexpected mptcp_sock.token %x != %x",
+			val.token, token);
+		err++;
+	}
+
 	return err;
 }
 
@@ -123,6 +173,7 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
 
 void test_base(void)
 {
+	char cmd[256], tmp_dir[] = "/tmp/XXXXXX";
 	int server_fd, cgroup_fd;
 
 	cgroup_fd = test__join_cgroup("/mptcp");
@@ -140,6 +191,17 @@ void test_base(void)
 
 with_mptcp:
 	/* with MPTCP */
+	if (system("ip mptcp help 2>&1 | grep -q monitor")) {
+		test__skip();
+		goto close_cgroup_fd;
+	}
+	if (!ASSERT_OK_PTR(mkdtemp(tmp_dir), "mkdtemp"))
+		goto close_cgroup_fd;
+	snprintf(monitor_log_path, sizeof(monitor_log_path),
+		 "%s/ip_mptcp_monitor", tmp_dir);
+	snprintf(cmd, sizeof(cmd), "ip mptcp monitor > %s &", monitor_log_path);
+	if (!ASSERT_OK(system(cmd), "ip mptcp monitor"))
+		goto close_cgroup_fd;
 	server_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
 	if (!ASSERT_GE(server_fd, 0, "start_mptcp_server"))
 		goto close_cgroup_fd;
@@ -147,6 +209,8 @@ void test_base(void)
 	ASSERT_OK(run_test(cgroup_fd, server_fd, true), "run_test mptcp");
 
 	close(server_fd);
+	snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp_dir);
+	system(cmd);
 
 close_cgroup_fd:
 	close(cgroup_fd);
diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
index 3feb7ff578e2..4890130826c6 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -12,6 +12,7 @@ extern bool CONFIG_MPTCP __kconfig;
 struct mptcp_storage {
 	__u32 invoked;
 	__u32 is_mptcp;
+	__u32 token;
 };
 
 struct {
@@ -48,6 +49,8 @@ int _sockops(struct bpf_sock_ops *ctx)
 					     BPF_SK_STORAGE_GET_F_CREATE);
 		if (!storage)
 			return 1;
+
+		storage->token = 0;
 	} else {
 		if (!CONFIG_MPTCP)
 			return 1;
@@ -60,6 +63,8 @@ int _sockops(struct bpf_sock_ops *ctx)
 					     BPF_SK_STORAGE_GET_F_CREATE);
 		if (!storage)
 			return 1;
+
+		storage->token = msk->token;
 	}
 	storage->invoked++;
 	storage->is_mptcp = is_mptcp;
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCH bpf-next v4 6/7] selftests/bpf: verify ca_name of struct mptcp_sock
  2022-05-13 22:48 [PATCH bpf-next v4 0/7] bpf: mptcp: Support for mptcp_sock Mat Martineau
                   ` (4 preceding siblings ...)
  2022-05-13 22:48 ` [PATCH bpf-next v4 5/7] selftests/bpf: verify token of struct mptcp_sock Mat Martineau
@ 2022-05-13 22:48 ` Mat Martineau
  2022-05-13 22:48 ` [PATCH bpf-next v4 7/7] selftests/bpf: verify first " Mat Martineau
  6 siblings, 0 replies; 25+ messages in thread
From: Mat Martineau @ 2022-05-13 22:48 UTC (permalink / raw)
  To: netdev, bpf
  Cc: Geliang Tang, ast, daniel, andrii, mptcp, Matthieu Baerts, Mat Martineau

From: Geliang Tang <geliang.tang@suse.com>

This patch verifies another member of struct mptcp_sock, ca_name. Add a
new function get_msk_ca_name() to read the sysctl tcp_congestion_control
and verify it in verify_msk().

v3: Access the sysctl through the filesystem to avoid compatibility
    issues with the busybox sysctl command.
v4: use ASSERT_* instead of CHECK_FAIL (Andrii)

Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 tools/testing/selftests/bpf/bpf_tcp_helpers.h |  5 +++
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 34 +++++++++++++++++++
 .../testing/selftests/bpf/progs/mptcp_sock.c  |  4 +++
 3 files changed, 43 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index 422491872619..c38c66d5c1e6 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -16,6 +16,10 @@ BPF_PROG(name, args)
 #define SOL_TCP 6
 #endif
 
+#ifndef TCP_CA_NAME_MAX
+#define TCP_CA_NAME_MAX	16
+#endif
+
 #define tcp_jiffies32 ((__u32)bpf_jiffies64())
 
 struct sock_common {
@@ -230,6 +234,7 @@ struct mptcp_sock {
 	struct inet_connection_sock	sk;
 
 	__u32		token;
+	char		ca_name[TCP_CA_NAME_MAX];
 } __attribute__((preserve_access_index));
 
 #endif
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index ac98aa314123..2ff7f18ea0ce 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -6,10 +6,15 @@
 #include "cgroup_helpers.h"
 #include "network_helpers.h"
 
+#ifndef TCP_CA_NAME_MAX
+#define TCP_CA_NAME_MAX	16
+#endif
+
 struct mptcp_storage {
 	__u32 invoked;
 	__u32 is_mptcp;
 	__u32 token;
+	char ca_name[TCP_CA_NAME_MAX];
 };
 
 static char monitor_log_path[64];
@@ -75,17 +80,40 @@ static __u32 get_msk_token(void)
 	return token;
 }
 
+void get_msk_ca_name(char ca_name[])
+{
+	size_t len;
+	int fd;
+
+	fd = open("/proc/sys/net/ipv4/tcp_congestion_control", O_RDONLY);
+	if (!ASSERT_GE(fd, 0, "Failed to open tcp_congestion_control"))
+		return;
+
+	len = read(fd, ca_name, TCP_CA_NAME_MAX);
+	if (!ASSERT_GT(len, 0, "Failed to read ca_name"))
+		goto err;
+
+	if (len > 0 && ca_name[len - 1] == '\n')
+		ca_name[len - 1] = '\0';
+
+err:
+	close(fd);
+}
+
 static int verify_msk(int map_fd, int client_fd)
 {
 	char *msg = "MPTCP subflow socket";
 	int err, cfd = client_fd;
 	struct mptcp_storage val;
+	char ca_name[TCP_CA_NAME_MAX];
 	__u32 token;
 
 	token = get_msk_token();
 	if (!ASSERT_GT(token, 0, "Unexpected token"))
 		return -1;
 
+	get_msk_ca_name(ca_name);
+
 	err = bpf_map_lookup_elem(map_fd, &cfd, &val);
 	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
 		return err;
@@ -108,6 +136,12 @@ static int verify_msk(int map_fd, int client_fd)
 		err++;
 	}
 
+	if (strncmp(val.ca_name, ca_name, TCP_CA_NAME_MAX)) {
+		log_err("Unexpected mptcp_sock.ca_name %s != %s",
+			val.ca_name, ca_name);
+		err++;
+	}
+
 	return err;
 }
 
diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
index 4890130826c6..c36f2f6bd2f1 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -2,6 +2,7 @@
 /* Copyright (c) 2020, Tessares SA. */
 /* Copyright (c) 2022, SUSE. */
 
+#include <string.h>
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include "bpf_tcp_helpers.h"
@@ -13,6 +14,7 @@ struct mptcp_storage {
 	__u32 invoked;
 	__u32 is_mptcp;
 	__u32 token;
+	char ca_name[TCP_CA_NAME_MAX];
 };
 
 struct {
@@ -51,6 +53,7 @@ int _sockops(struct bpf_sock_ops *ctx)
 			return 1;
 
 		storage->token = 0;
+		bzero(storage->ca_name, TCP_CA_NAME_MAX);
 	} else {
 		if (!CONFIG_MPTCP)
 			return 1;
@@ -65,6 +68,7 @@ int _sockops(struct bpf_sock_ops *ctx)
 			return 1;
 
 		storage->token = msk->token;
+		memcpy(storage->ca_name, msk->ca_name, TCP_CA_NAME_MAX);
 	}
 	storage->invoked++;
 	storage->is_mptcp = is_mptcp;
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCH bpf-next v4 7/7] selftests/bpf: verify first of struct mptcp_sock
  2022-05-13 22:48 [PATCH bpf-next v4 0/7] bpf: mptcp: Support for mptcp_sock Mat Martineau
                   ` (5 preceding siblings ...)
  2022-05-13 22:48 ` [PATCH bpf-next v4 6/7] selftests/bpf: verify ca_name " Mat Martineau
@ 2022-05-13 22:48 ` Mat Martineau
  6 siblings, 0 replies; 25+ messages in thread
From: Mat Martineau @ 2022-05-13 22:48 UTC (permalink / raw)
  To: netdev, bpf
  Cc: Geliang Tang, ast, daniel, andrii, mptcp, Matthieu Baerts, Mat Martineau

From: Geliang Tang <geliang.tang@suse.com>

This patch verifies the 'first' struct member of struct mptcp_sock, which
points to the first subflow of msk. Save 'sk' in mptcp_storage, and verify
it with 'first' in verify_msk().

Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 tools/testing/selftests/bpf/bpf_tcp_helpers.h  | 1 +
 tools/testing/selftests/bpf/prog_tests/mptcp.c | 8 ++++++++
 tools/testing/selftests/bpf/progs/mptcp_sock.c | 5 +++++
 3 files changed, 14 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index c38c66d5c1e6..82a7c9de95f9 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -234,6 +234,7 @@ struct mptcp_sock {
 	struct inet_connection_sock	sk;
 
 	__u32		token;
+	struct sock	*first;
 	char		ca_name[TCP_CA_NAME_MAX];
 } __attribute__((preserve_access_index));
 
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 2ff7f18ea0ce..51a3e17acb9e 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -13,7 +13,9 @@
 struct mptcp_storage {
 	__u32 invoked;
 	__u32 is_mptcp;
+	struct sock *sk;
 	__u32 token;
+	struct sock *first;
 	char ca_name[TCP_CA_NAME_MAX];
 };
 
@@ -136,6 +138,12 @@ static int verify_msk(int map_fd, int client_fd)
 		err++;
 	}
 
+	if (val.first != val.sk) {
+		log_err("Unexpected mptcp_sock.first %p != %p",
+			val.first, val.sk);
+		err++;
+	}
+
 	if (strncmp(val.ca_name, ca_name, TCP_CA_NAME_MAX)) {
 		log_err("Unexpected mptcp_sock.ca_name %s != %s",
 			val.ca_name, ca_name);
diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
index c36f2f6bd2f1..ab135edf3ae3 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -13,7 +13,9 @@ extern bool CONFIG_MPTCP __kconfig;
 struct mptcp_storage {
 	__u32 invoked;
 	__u32 is_mptcp;
+	struct sock *sk;
 	__u32 token;
+	struct sock *first;
 	char ca_name[TCP_CA_NAME_MAX];
 };
 
@@ -54,6 +56,7 @@ int _sockops(struct bpf_sock_ops *ctx)
 
 		storage->token = 0;
 		bzero(storage->ca_name, TCP_CA_NAME_MAX);
+		storage->first = NULL;
 	} else {
 		if (!CONFIG_MPTCP)
 			return 1;
@@ -69,9 +72,11 @@ int _sockops(struct bpf_sock_ops *ctx)
 
 		storage->token = msk->token;
 		memcpy(storage->ca_name, msk->ca_name, TCP_CA_NAME_MAX);
+		storage->first = msk->first;
 	}
 	storage->invoked++;
 	storage->is_mptcp = is_mptcp;
+	storage->sk = (struct sock *)sk;
 
 	return 1;
 }
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock
  2022-05-13 22:48 ` [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock Mat Martineau
@ 2022-05-13 23:53   ` Geliang Tang
  2022-05-16 22:47   ` Andrii Nakryiko
  2022-05-17  1:20   ` Martin KaFai Lau
  2 siblings, 0 replies; 25+ messages in thread
From: Geliang Tang @ 2022-05-13 23:53 UTC (permalink / raw)
  To: Mat Martineau
  Cc: Networking, bpf, Geliang Tang, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, MPTCP Upstream,
	Matthieu Baerts

Mat Martineau <mathew.j.martineau@linux.intel.com> 于2022年5月14日周六 06:48写道：
>
> From: Geliang Tang <geliang.tang@suse.com>
>
> This patch extends the MPTCP test base, to test the new helper
> bpf_skc_to_mptcp_sock().
>
> Define struct mptcp_sock in bpf_tcp_helpers.h, use bpf_skc_to_mptcp_sock
> to get the msk socket in progs/mptcp_sock.c and store the infos in
> socket_storage_map.
>
> Get the infos from socket_storage_map in prog_tests/mptcp.c. Add a new
> function verify_msk() to verify the infos of MPTCP socket, and rename
> verify_sk() to verify_tsk() to verify TCP socket only.
>
> v2: Add CONFIG_MPTCP check for clearer error messages
> v4:
>  - use ASSERT_* instead of CHECK_FAIL (Andrii)
>  - drop bpf_mptcp_helpers.h (Andrii)
>
> Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
>  tools/testing/selftests/bpf/bpf_tcp_helpers.h |  5 +++
>  .../testing/selftests/bpf/prog_tests/mptcp.c  | 45 ++++++++++++++-----
>  .../testing/selftests/bpf/progs/mptcp_sock.c  | 23 ++++++++--
>  3 files changed, 58 insertions(+), 15 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> index 22e0c8849a17..90fecafc493d 100644
> --- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> @@ -226,4 +226,9 @@ static __always_inline bool tcp_cc_eq(const char *a, const char *b)
>  extern __u32 tcp_slow_start(struct tcp_sock *tp, __u32 acked) __ksym;
>  extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym;
>
> +struct mptcp_sock {
> +       struct inet_connection_sock     sk;
> +
> +} __attribute__((preserve_access_index));
> +
>  #endif
> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index cb0389ca8690..02e7fd8918e6 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -11,14 +11,12 @@ struct mptcp_storage {
>         __u32 is_mptcp;
>  };
>
> -static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp)
> +static int verify_tsk(int map_fd, int client_fd)
>  {
> +       char *msg = "plain TCP socket";
>         int err, cfd = client_fd;
>         struct mptcp_storage val;
>
> -       if (is_mptcp == 1)
> -               return 0;
> -
>         err = bpf_map_lookup_elem(map_fd, &cfd, &val);
>         if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
>                 return err;
> @@ -38,6 +36,31 @@ static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp)
>         return err;
>  }
>
> +static int verify_msk(int map_fd, int client_fd)
> +{
> +       char *msg = "MPTCP subflow socket";
> +       int err, cfd = client_fd;
> +       struct mptcp_storage val;
> +
> +       err = bpf_map_lookup_elem(map_fd, &cfd, &val);
> +       if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> +               return err;
> +
> +       if (val.invoked != 1) {
> +               log_err("%s: unexpected invoked count %d != 1",
> +                       msg, val.invoked);
> +               err++;
> +       }
> +
> +       if (val.is_mptcp != 1) {
> +               log_err("%s: unexpected bpf_tcp_sock.is_mptcp %d != 1",
> +                       msg, val.is_mptcp);
> +               err++;
> +       }
> +
> +       return err;
> +}
> +
>  static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
>  {
>         int client_fd, prog_fd, map_fd, err;
> @@ -88,8 +111,8 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
>                 goto out;
>         }
>
> -       err += is_mptcp ? verify_sk(map_fd, client_fd, "MPTCP subflow socket", 1) :
> -                         verify_sk(map_fd, client_fd, "plain TCP socket", 0);
> +       err += is_mptcp ? verify_msk(map_fd, client_fd) :
> +                         verify_tsk(map_fd, client_fd);
>
>         close(client_fd);
>


''''
> @@ -103,25 +126,25 @@ void test_base(void)
>         int server_fd, cgroup_fd;
>
>         cgroup_fd = test__join_cgroup("/mptcp");
> -       if (CHECK_FAIL(cgroup_fd < 0))
> +       if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup"))
>                 return;
>
>         /* without MPTCP */
>         server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
> -       if (CHECK_FAIL(server_fd < 0))
> +       if (!ASSERT_GE(server_fd, 0, "start_server"))
>                 goto with_mptcp;
>
> -       CHECK_FAIL(run_test(cgroup_fd, server_fd, false));
> +       ASSERT_OK(run_test(cgroup_fd, server_fd, false), "run_test tcp");
>
>         close(server_fd);
>
>  with_mptcp:
>         /* with MPTCP */
>         server_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
> -       if (CHECK_FAIL(server_fd < 0))
> +       if (!ASSERT_GE(server_fd, 0, "start_mptcp_server"))
>                 goto close_cgroup_fd;
>
> -       CHECK_FAIL(run_test(cgroup_fd, server_fd, true));
> +       ASSERT_OK(run_test(cgroup_fd, server_fd, true), "run_test mptcp");
'''

Sorry Mat, this code using ASSERT_* instead of CHECK_FAIL should be
squash into patch #3, it shouldn't in this patch. I'll send a v5 to
MPTCP ML to fix this.

Thanks,
-Geliang

>
>         close(server_fd);
>
> diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> index bc09dba0b078..3feb7ff578e2 100644
> --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
> +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> @@ -7,6 +7,7 @@
>  #include "bpf_tcp_helpers.h"
>
>  char _license[] SEC("license") = "GPL";
> +extern bool CONFIG_MPTCP __kconfig;
>
>  struct mptcp_storage {
>         __u32 invoked;
> @@ -24,6 +25,7 @@ SEC("sockops")
>  int _sockops(struct bpf_sock_ops *ctx)
>  {
>         struct mptcp_storage *storage;
> +       struct mptcp_sock *msk;
>         int op = (int)ctx->op;
>         struct tcp_sock *tsk;
>         struct bpf_sock *sk;
> @@ -41,11 +43,24 @@ int _sockops(struct bpf_sock_ops *ctx)
>                 return 1;
>
>         is_mptcp = bpf_core_field_exists(tsk->is_mptcp) ? tsk->is_mptcp : 0;
> -       storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
> -                                    BPF_SK_STORAGE_GET_F_CREATE);
> -       if (!storage)
> -               return 1;
> +       if (!is_mptcp) {
> +               storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
> +                                            BPF_SK_STORAGE_GET_F_CREATE);
> +               if (!storage)
> +                       return 1;
> +       } else {
> +               if (!CONFIG_MPTCP)
> +                       return 1;
> +
> +               msk = bpf_skc_to_mptcp_sock(sk);
> +               if (!msk)
> +                       return 1;
>
> +               storage = bpf_sk_storage_get(&socket_storage_map, msk, 0,
> +                                            BPF_SK_STORAGE_GET_F_CREATE);
> +               if (!storage)
> +                       return 1;
> +       }
>         storage->invoked++;
>         storage->is_mptcp = is_mptcp;
>
> --
> 2.36.1
>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 3/7] selftests/bpf: add MPTCP test base
  2022-05-13 22:48 ` [PATCH bpf-next v4 3/7] selftests/bpf: add MPTCP test base Mat Martineau
@ 2022-05-16 22:43   ` Andrii Nakryiko
  2022-05-17  5:30     ` Geliang Tang
  2022-05-17  1:18   ` Martin KaFai Lau
  1 sibling, 1 reply; 25+ messages in thread
From: Andrii Nakryiko @ 2022-05-16 22:43 UTC (permalink / raw)
  To: Mat Martineau
  Cc: Networking, bpf, Nicolas Rybowski, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, mptcp, Matthieu Baerts,
	Geliang Tang

On Fri, May 13, 2022 at 3:48 PM Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
>
> From: Nicolas Rybowski <nicolas.rybowski@tessares.net>
>
> This patch adds a base for MPTCP specific tests.
>
> It is currently limited to the is_mptcp field in case of plain TCP
> connection because there is no easy way to get the subflow sk from a msk
> in userspace. This implies that we cannot lookup the sk_storage attached
> to the subflow sk in the sockops program.
>
> v4:
>  - add copyright 2022 (Andrii)
>  - use ASSERT_* instead of CHECK_FAIL (Andrii)
>  - drop SEC("version") (Andrii)
>  - use is_mptcp in tcp_sock, instead of bpf_tcp_sock (Martin & Andrii)
>
> Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Co-developed-by: Geliang Tang <geliang.tang@suse.com>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> Signed-off-by: Nicolas Rybowski <nicolas.rybowski@tessares.net>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
>  MAINTAINERS                                   |   1 +
>  tools/testing/selftests/bpf/bpf_tcp_helpers.h |   1 +
>  tools/testing/selftests/bpf/config            |   1 +
>  tools/testing/selftests/bpf/network_helpers.c |  43 ++++--
>  tools/testing/selftests/bpf/network_helpers.h |   4 +
>  .../testing/selftests/bpf/prog_tests/mptcp.c  | 136 ++++++++++++++++++
>  .../testing/selftests/bpf/progs/mptcp_sock.c  |  53 +++++++
>  7 files changed, 231 insertions(+), 8 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/mptcp.c
>  create mode 100644 tools/testing/selftests/bpf/progs/mptcp_sock.c
>

Seems like bpf_core_field_exists() works fine for your use case and CI
is green. See some selftest-specific issues below, though.

[...]

> +static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
> +{
> +       int client_fd, prog_fd, map_fd, err;
> +       struct bpf_program *prog;
> +       struct bpf_object *obj;
> +       struct bpf_map *map;
> +
> +       obj = bpf_object__open("./mptcp_sock.o");
> +       if (libbpf_get_error(obj))
> +               return -EIO;
> +
> +       err = bpf_object__load(obj);
> +       if (!ASSERT_OK(err, "bpf_object__load"))
> +               goto out;
> +
> +       prog = bpf_object__find_program_by_name(obj, "_sockops");

can you please use BPF skeleton instead of doing these lookups by
name? See other tests that are including .skel.h headers for example

> +       if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name")) {
> +               err = -EIO;
> +               goto out;
> +       }
> +

[...]

> +void test_base(void)
> +{
> +       int server_fd, cgroup_fd;
> +
> +       cgroup_fd = test__join_cgroup("/mptcp");
> +       if (CHECK_FAIL(cgroup_fd < 0))
> +               return;
> +
> +       /* without MPTCP */
> +       server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
> +       if (CHECK_FAIL(server_fd < 0))
> +               goto with_mptcp;
> +
> +       CHECK_FAIL(run_test(cgroup_fd, server_fd, false));

please don't add new uses of CHECK_FAIL()

> +
> +       close(server_fd);
> +

[...]

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 5/7] selftests/bpf: verify token of struct mptcp_sock
  2022-05-13 22:48 ` [PATCH bpf-next v4 5/7] selftests/bpf: verify token of struct mptcp_sock Mat Martineau
@ 2022-05-16 22:45   ` Andrii Nakryiko
  2022-05-17  1:32   ` Martin KaFai Lau
  1 sibling, 0 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2022-05-16 22:45 UTC (permalink / raw)
  To: Mat Martineau
  Cc: Networking, bpf, Geliang Tang, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, mptcp, Matthieu Baerts

On Fri, May 13, 2022 at 3:48 PM Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
>
> From: Geliang Tang <geliang.tang@suse.com>
>
> This patch verifies the struct member token of struct mptcp_sock. Add a
> new function get_msk_token() to parse the msk token from the output of
> the command 'ip mptcp monitor', and verify it in verify_msk().
>
> v4:
>  - use ASSERT_* instead of CHECK_FAIL (Andrii)
>  - skip the test if 'ip mptcp monitor' is not supported (Mat)
>
> Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
>  tools/testing/selftests/bpf/bpf_tcp_helpers.h |  1 +
>  .../testing/selftests/bpf/prog_tests/mptcp.c  | 64 +++++++++++++++++++
>  .../testing/selftests/bpf/progs/mptcp_sock.c  |  5 ++
>  3 files changed, 70 insertions(+)
>

[...]

> +       fd = open(monitor_log_path, O_RDONLY);
> +       if (!ASSERT_GE(fd, 0, "Failed to open monitor_log_path"))
> +               return token;
> +
> +       len = read(fd, buf, sizeof(buf));
> +       if (!ASSERT_GT(len, 0, "Failed to read monitor_log_path"))
> +               goto err;
> +
> +       if (strncmp(buf, prefix, strlen(prefix))) {

ASSERT_STRNEQ ?

> +               log_err("Invalid prefix %s", buf);
> +               goto err;
> +       }
> +
> +       token = strtol(buf + strlen(prefix), NULL, 16);
> +
> +err:
> +       close(fd);
> +       return token;
> +}
> +
>  static int verify_msk(int map_fd, int client_fd)
>  {
>         char *msg = "MPTCP subflow socket";
>         int err, cfd = client_fd;
>         struct mptcp_storage val;
> +       __u32 token;
> +
> +       token = get_msk_token();
> +       if (!ASSERT_GT(token, 0, "Unexpected token"))
> +               return -1;
>
>         err = bpf_map_lookup_elem(map_fd, &cfd, &val);
>         if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> @@ -58,6 +102,12 @@ static int verify_msk(int map_fd, int client_fd)
>                 err++;
>         }
>
> +       if (val.token != token) {


ASSERT_NEQ

> +               log_err("Unexpected mptcp_sock.token %x != %x",
> +                       val.token, token);
> +               err++;
> +       }
> +
>         return err;
>  }
>

[...]

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock
  2022-05-13 22:48 ` [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock Mat Martineau
  2022-05-13 23:53   ` Geliang Tang
@ 2022-05-16 22:47   ` Andrii Nakryiko
  2022-05-17  1:20   ` Martin KaFai Lau
  2 siblings, 0 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2022-05-16 22:47 UTC (permalink / raw)
  To: Mat Martineau
  Cc: Networking, bpf, Geliang Tang, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, mptcp, Matthieu Baerts

On Fri, May 13, 2022 at 3:48 PM Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
>
> From: Geliang Tang <geliang.tang@suse.com>
>
> This patch extends the MPTCP test base, to test the new helper
> bpf_skc_to_mptcp_sock().
>
> Define struct mptcp_sock in bpf_tcp_helpers.h, use bpf_skc_to_mptcp_sock
> to get the msk socket in progs/mptcp_sock.c and store the infos in
> socket_storage_map.
>
> Get the infos from socket_storage_map in prog_tests/mptcp.c. Add a new
> function verify_msk() to verify the infos of MPTCP socket, and rename
> verify_sk() to verify_tsk() to verify TCP socket only.
>
> v2: Add CONFIG_MPTCP check for clearer error messages
> v4:
>  - use ASSERT_* instead of CHECK_FAIL (Andrii)
>  - drop bpf_mptcp_helpers.h (Andrii)
>
> Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
>  tools/testing/selftests/bpf/bpf_tcp_helpers.h |  5 +++
>  .../testing/selftests/bpf/prog_tests/mptcp.c  | 45 ++++++++++++++-----
>  .../testing/selftests/bpf/progs/mptcp_sock.c  | 23 ++++++++--
>  3 files changed, 58 insertions(+), 15 deletions(-)
>

[...]

> +static int verify_msk(int map_fd, int client_fd)
> +{
> +       char *msg = "MPTCP subflow socket";
> +       int err, cfd = client_fd;
> +       struct mptcp_storage val;
> +
> +       err = bpf_map_lookup_elem(map_fd, &cfd, &val);
> +       if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> +               return err;
> +
> +       if (val.invoked != 1) {
> +               log_err("%s: unexpected invoked count %d != 1",
> +                       msg, val.invoked);
> +               err++;
> +       }
> +
> +       if (val.is_mptcp != 1) {
> +               log_err("%s: unexpected bpf_tcp_sock.is_mptcp %d != 1",
> +                       msg, val.is_mptcp);
> +               err++;
> +       }

any reason to not use ASSERT_NEQ ?


> +
> +       return err;
> +}
> +
>  static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
>  {
>         int client_fd, prog_fd, map_fd, err;

[...]

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto
  2022-05-13 22:48 ` [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto Mat Martineau
@ 2022-05-17  1:07   ` Martin KaFai Lau
  2022-05-17  5:26     ` Geliang Tang
  0 siblings, 1 reply; 25+ messages in thread
From: Martin KaFai Lau @ 2022-05-17  1:07 UTC (permalink / raw)
  To: Geliang Tang, Mat Martineau
  Cc: netdev, bpf, ast, daniel, andrii, mptcp, Nicolas Rybowski,
	Matthieu Baerts

On Fri, May 13, 2022 at 03:48:21PM -0700, Mat Martineau wrote:
[ ... ]

> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 8b1afd6f5cc4..2ba09de955c7 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -284,4 +284,10 @@ static inline int mptcpv6_init(void) { return 0; }
>  static inline void mptcpv6_handle_mapped(struct sock *sk, bool mapped) { }
>  #endif
>  
> +#if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_SYSCALL)
> +struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk);
Can this be inline ?

> +#else
> +static inline struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk) { return NULL; }
> +#endif
> +
>  #endif /* __NET_MPTCP_H */

[ ... ]

> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> new file mode 100644
> index 000000000000..535602ba2582
> --- /dev/null
> +++ b/net/mptcp/bpf.c
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Multipath TCP
> + *
> + * Copyright (c) 2020, Tessares SA.
> + * Copyright (c) 2022, SUSE.
> + *
> + * Author: Nicolas Rybowski <nicolas.rybowski@tessares.net>
> + */
> +
> +#define pr_fmt(fmt) "MPTCP: " fmt
> +
> +#include <linux/bpf.h>
> +#include "protocol.h"
> +
> +struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
> +{
> +	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
> +		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL(bpf_mptcp_sock_from_subflow);
Is EXPORT_SYMBOL needed ?

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 3/7] selftests/bpf: add MPTCP test base
  2022-05-13 22:48 ` [PATCH bpf-next v4 3/7] selftests/bpf: add MPTCP test base Mat Martineau
  2022-05-16 22:43   ` Andrii Nakryiko
@ 2022-05-17  1:18   ` Martin KaFai Lau
  2022-05-17  5:28     ` Geliang Tang
  1 sibling, 1 reply; 25+ messages in thread
From: Martin KaFai Lau @ 2022-05-17  1:18 UTC (permalink / raw)
  To: Mat Martineau
  Cc: netdev, bpf, Nicolas Rybowski, ast, daniel, andrii, mptcp,
	Matthieu Baerts, Geliang Tang

On Fri, May 13, 2022 at 03:48:23PM -0700, Mat Martineau wrote:
[ ... ]

> @@ -265,7 +282,7 @@ int connect_to_fd_opts(int server_fd, const struct network_helper_opts *opts)
>  	}
>  
>  	addr_in = (struct sockaddr_in *)&addr;
> -	fd = socket(addr_in->sin_family, type, 0);
> +	fd = socket(addr_in->sin_family, type, opts->protocol);
ops->protocol is the same as the server_fd's protocol ?

Can that be learned from getsockopt(server_fd, SOL_SOCKET, SO_PROTOCOL, ....) ?
Then the ops->protocol additions and related changes are not needed.

connect_to_fd_opts() has already obtained the SO_TYPE in similar way.

>  	if (fd < 0) {
>  		log_err("Failed to create client socket");
>  		return -1;
> @@ -298,6 +315,16 @@ int connect_to_fd(int server_fd, int timeout_ms)
>  	return connect_to_fd_opts(server_fd, &opts);
>  }
>  
> +int connect_to_mptcp_fd(int server_fd, int timeout_ms)
> +{
> +	struct network_helper_opts opts = {
> +		.timeout_ms = timeout_ms,
> +		.protocol = IPPROTO_MPTCP,
> +	};
> +
> +	return connect_to_fd_opts(server_fd, &opts);
> +}
> +
>  int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms)
>  {
>  	struct sockaddr_storage addr;
> diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
> index a4b3b2f9877b..e0feb115b2ae 100644
> --- a/tools/testing/selftests/bpf/network_helpers.h
> +++ b/tools/testing/selftests/bpf/network_helpers.h
> @@ -21,6 +21,7 @@ struct network_helper_opts {
>  	const char *cc;
>  	int timeout_ms;
>  	bool must_fail;
> +	int protocol;
>  };
>  
>  /* ipv4 test vector */
> @@ -42,11 +43,14 @@ extern struct ipv6_packet pkt_v6;
>  int settimeo(int fd, int timeout_ms);
>  int start_server(int family, int type, const char *addr, __u16 port,
>  		 int timeout_ms);
> +int start_mptcp_server(int family, const char *addr, __u16 port,
> +		       int timeout_ms);
>  int *start_reuseport_server(int family, int type, const char *addr_str,
>  			    __u16 port, int timeout_ms,
>  			    unsigned int nr_listens);
>  void free_fds(int *fds, unsigned int nr_close_fds);
>  int connect_to_fd(int server_fd, int timeout_ms);
> +int connect_to_mptcp_fd(int server_fd, int timeout_ms);
>  int connect_to_fd_opts(int server_fd, const struct network_helper_opts *opts);
>  int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms);
>  int fastopen_connect(int server_fd, const char *data, unsigned int data_len,

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock
  2022-05-13 22:48 ` [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock Mat Martineau
  2022-05-13 23:53   ` Geliang Tang
  2022-05-16 22:47   ` Andrii Nakryiko
@ 2022-05-17  1:20   ` Martin KaFai Lau
  2022-05-17  5:29     ` Geliang Tang
  2 siblings, 1 reply; 25+ messages in thread
From: Martin KaFai Lau @ 2022-05-17  1:20 UTC (permalink / raw)
  To: Geliang Tang, Mat Martineau
  Cc: netdev, bpf, ast, daniel, andrii, mptcp, Matthieu Baerts

On Fri, May 13, 2022 at 03:48:24PM -0700, Mat Martineau wrote:
[ ... ]
> diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> index bc09dba0b078..3feb7ff578e2 100644
> --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
> +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> @@ -7,6 +7,7 @@
>  #include "bpf_tcp_helpers.h"
>  
>  char _license[] SEC("license") = "GPL";
> +extern bool CONFIG_MPTCP __kconfig;
>  
>  struct mptcp_storage {
>  	__u32 invoked;
> @@ -24,6 +25,7 @@ SEC("sockops")
>  int _sockops(struct bpf_sock_ops *ctx)
>  {
>  	struct mptcp_storage *storage;
> +	struct mptcp_sock *msk;
>  	int op = (int)ctx->op;
>  	struct tcp_sock *tsk;
>  	struct bpf_sock *sk;
> @@ -41,11 +43,24 @@ int _sockops(struct bpf_sock_ops *ctx)
>  		return 1;
>  
>  	is_mptcp = bpf_core_field_exists(tsk->is_mptcp) ? tsk->is_mptcp : 0;
> -	storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
> -				     BPF_SK_STORAGE_GET_F_CREATE);
> -	if (!storage)
> -		return 1;
> +	if (!is_mptcp) {
> +		storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
> +					     BPF_SK_STORAGE_GET_F_CREATE);
> +		if (!storage)
> +			return 1;
> +	} else {
> +		if (!CONFIG_MPTCP)
hmm... how is it possible ?  The above just tested "!is_mptcp".

> +			return 1;
> +
> +		msk = bpf_skc_to_mptcp_sock(sk);
> +		if (!msk)
> +			return 1;
>  
> +		storage = bpf_sk_storage_get(&socket_storage_map, msk, 0,
> +					     BPF_SK_STORAGE_GET_F_CREATE);
> +		if (!storage)
> +			return 1;
> +	}
>  	storage->invoked++;
>  	storage->is_mptcp = is_mptcp;
>  
> -- 
> 2.36.1
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 5/7] selftests/bpf: verify token of struct mptcp_sock
  2022-05-13 22:48 ` [PATCH bpf-next v4 5/7] selftests/bpf: verify token of struct mptcp_sock Mat Martineau
  2022-05-16 22:45   ` Andrii Nakryiko
@ 2022-05-17  1:32   ` Martin KaFai Lau
  2022-05-17  5:19     ` Geliang Tang
  1 sibling, 1 reply; 25+ messages in thread
From: Martin KaFai Lau @ 2022-05-17  1:32 UTC (permalink / raw)
  To: Mat Martineau, Geliang Tang
  Cc: netdev, bpf, ast, daniel, andrii, mptcp, Matthieu Baerts

On Fri, May 13, 2022 at 03:48:25PM -0700, Mat Martineau wrote:
[ ... ]

> +/*
> + * Parse the token from the output of 'ip mptcp monitor':
> + *
> + * [       CREATED] token=3ca933d3 remid=0 locid=0 saddr4=127.0.0.1 ...
> + * [       CREATED] token=2ab57040 remid=0 locid=0 saddr4=127.0.0.1 ...
How stable is the string format ?

If it happens to have some unrelated mptcp connection going on, will the test
break ?

> + */
> +static __u32 get_msk_token(void)
> +{
> +	char *prefix = "[       CREATED] token=";
> +	char buf[BUFSIZ] = {};
> +	__u32 token = 0;
> +	ssize_t len;
> +	int fd;
> +
> +	sync();
> +
> +	fd = open(monitor_log_path, O_RDONLY);
> +	if (!ASSERT_GE(fd, 0, "Failed to open monitor_log_path"))
> +		return token;
> +
> +	len = read(fd, buf, sizeof(buf));
> +	if (!ASSERT_GT(len, 0, "Failed to read monitor_log_path"))
> +		goto err;
> +
> +	if (strncmp(buf, prefix, strlen(prefix))) {
> +		log_err("Invalid prefix %s", buf);
> +		goto err;
> +	}
> +
> +	token = strtol(buf + strlen(prefix), NULL, 16);
> +
> +err:
> +	close(fd);
> +	return token;
> +}
> +

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 5/7] selftests/bpf: verify token of struct mptcp_sock
  2022-05-17  1:32   ` Martin KaFai Lau
@ 2022-05-17  5:19     ` Geliang Tang
  2022-05-17 21:30       ` Martin KaFai Lau
  0 siblings, 1 reply; 25+ messages in thread
From: Geliang Tang @ 2022-05-17  5:19 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Mat Martineau, Geliang Tang, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, MPTCP Upstream,
	Matthieu Baerts

Martin KaFai Lau <kafai@fb.com> 于2022年5月17日周二 09:32写道：
>
> On Fri, May 13, 2022 at 03:48:25PM -0700, Mat Martineau wrote:
> [ ... ]
>
> > +/*
> > + * Parse the token from the output of 'ip mptcp monitor':
> > + *
> > + * [       CREATED] token=3ca933d3 remid=0 locid=0 saddr4=127.0.0.1 ...
> > + * [       CREATED] token=2ab57040 remid=0 locid=0 saddr4=127.0.0.1 ...
> How stable is the string format ?
>
> If it happens to have some unrelated mptcp connection going on, will the test
> break ?

Hi Martin,

Yes, it will break in that case. How can I fix this? Should I run the
test in a special net namespace?

'ip mptcp monitor' can easily run in a special net namespace:

ip -net ns1 mptcp monitor

But I don't know how to run start_server() and connect_to_fd() in a
special namespace. Could you please give me some suggestions about
this?

Thanks,
-Geliang

>
> > + */
> > +static __u32 get_msk_token(void)
> > +{
> > +     char *prefix = "[       CREATED] token=";
> > +     char buf[BUFSIZ] = {};
> > +     __u32 token = 0;
> > +     ssize_t len;
> > +     int fd;
> > +
> > +     sync();
> > +
> > +     fd = open(monitor_log_path, O_RDONLY);
> > +     if (!ASSERT_GE(fd, 0, "Failed to open monitor_log_path"))
> > +             return token;
> > +
> > +     len = read(fd, buf, sizeof(buf));
> > +     if (!ASSERT_GT(len, 0, "Failed to read monitor_log_path"))
> > +             goto err;
> > +
> > +     if (strncmp(buf, prefix, strlen(prefix))) {
> > +             log_err("Invalid prefix %s", buf);
> > +             goto err;
> > +     }
> > +
> > +     token = strtol(buf + strlen(prefix), NULL, 16);
> > +
> > +err:
> > +     close(fd);
> > +     return token;
> > +}
> > +
>

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto
  2022-05-17  1:07   ` Martin KaFai Lau
@ 2022-05-17  5:26     ` Geliang Tang
  2022-05-17 17:00       ` Alexei Starovoitov
  0 siblings, 1 reply; 25+ messages in thread
From: Geliang Tang @ 2022-05-17  5:26 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Geliang Tang, Mat Martineau, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, MPTCP Upstream,
	Nicolas Rybowski, Matthieu Baerts

Martin KaFai Lau <kafai@fb.com> 于2022年5月17日周二 09:07写道：
>
> On Fri, May 13, 2022 at 03:48:21PM -0700, Mat Martineau wrote:
> [ ... ]
>
> > diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> > index 8b1afd6f5cc4..2ba09de955c7 100644
> > --- a/include/net/mptcp.h
> > +++ b/include/net/mptcp.h
> > @@ -284,4 +284,10 @@ static inline int mptcpv6_init(void) { return 0; }
> >  static inline void mptcpv6_handle_mapped(struct sock *sk, bool mapped) { }
> >  #endif
> >
> > +#if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_SYSCALL)
> > +struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk);
> Can this be inline ?

This function can't be inline since it uses struct mptcp_subflow_context.

mptcp_subflow_context is defined in net/mptcp/protocol.h, and we don't
want to export it to user space in net/mptcp/protocol.h.

>
> > +#else
> > +static inline struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk) { return NULL; }
> > +#endif
> > +
> >  #endif /* __NET_MPTCP_H */
>
> [ ... ]
>
> > diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> > new file mode 100644
> > index 000000000000..535602ba2582
> > --- /dev/null
> > +++ b/net/mptcp/bpf.c
> > @@ -0,0 +1,22 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Multipath TCP
> > + *
> > + * Copyright (c) 2020, Tessares SA.
> > + * Copyright (c) 2022, SUSE.
> > + *
> > + * Author: Nicolas Rybowski <nicolas.rybowski@tessares.net>
> > + */
> > +
> > +#define pr_fmt(fmt) "MPTCP: " fmt
> > +
> > +#include <linux/bpf.h>
> > +#include "protocol.h"
> > +
> > +struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
> > +{
> > +     if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
> > +             return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
> > +
> > +     return NULL;
> > +}
> > +EXPORT_SYMBOL(bpf_mptcp_sock_from_subflow);
> Is EXPORT_SYMBOL needed ?

Will drop in v5.

Thanks,
-Geliang

>

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 3/7] selftests/bpf: add MPTCP test base
  2022-05-17  1:18   ` Martin KaFai Lau
@ 2022-05-17  5:28     ` Geliang Tang
  0 siblings, 0 replies; 25+ messages in thread
From: Geliang Tang @ 2022-05-17  5:28 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Mat Martineau, Networking, bpf, Nicolas Rybowski,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	MPTCP Upstream, Matthieu Baerts, Geliang Tang

Martin KaFai Lau <kafai@fb.com> 于2022年5月17日周二 09:18写道：
>
> On Fri, May 13, 2022 at 03:48:23PM -0700, Mat Martineau wrote:
> [ ... ]
>
> > @@ -265,7 +282,7 @@ int connect_to_fd_opts(int server_fd, const struct network_helper_opts *opts)
> >       }
> >
> >       addr_in = (struct sockaddr_in *)&addr;
> > -     fd = socket(addr_in->sin_family, type, 0);
> > +     fd = socket(addr_in->sin_family, type, opts->protocol);
> ops->protocol is the same as the server_fd's protocol ?
>
> Can that be learned from getsockopt(server_fd, SOL_SOCKET, SO_PROTOCOL, ....) ?
> Then the ops->protocol additions and related changes are not needed.

Yes, I will update this in v5.

>
> connect_to_fd_opts() has already obtained the SO_TYPE in similar way.
>
> >       if (fd < 0) {
> >               log_err("Failed to create client socket");
> >               return -1;
> > @@ -298,6 +315,16 @@ int connect_to_fd(int server_fd, int timeout_ms)
> >       return connect_to_fd_opts(server_fd, &opts);
> >  }
> >
> > +int connect_to_mptcp_fd(int server_fd, int timeout_ms)
> > +{
> > +     struct network_helper_opts opts = {
> > +             .timeout_ms = timeout_ms,
> > +             .protocol = IPPROTO_MPTCP,
> > +     };
> > +
> > +     return connect_to_fd_opts(server_fd, &opts);
> > +}
> > +
> >  int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms)
> >  {
> >       struct sockaddr_storage addr;
> > diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
> > index a4b3b2f9877b..e0feb115b2ae 100644
> > --- a/tools/testing/selftests/bpf/network_helpers.h
> > +++ b/tools/testing/selftests/bpf/network_helpers.h
> > @@ -21,6 +21,7 @@ struct network_helper_opts {
> >       const char *cc;
> >       int timeout_ms;
> >       bool must_fail;
> > +     int protocol;
> >  };
> >
> >  /* ipv4 test vector */
> > @@ -42,11 +43,14 @@ extern struct ipv6_packet pkt_v6;
> >  int settimeo(int fd, int timeout_ms);
> >  int start_server(int family, int type, const char *addr, __u16 port,
> >                int timeout_ms);
> > +int start_mptcp_server(int family, const char *addr, __u16 port,
> > +                    int timeout_ms);
> >  int *start_reuseport_server(int family, int type, const char *addr_str,
> >                           __u16 port, int timeout_ms,
> >                           unsigned int nr_listens);
> >  void free_fds(int *fds, unsigned int nr_close_fds);
> >  int connect_to_fd(int server_fd, int timeout_ms);
> > +int connect_to_mptcp_fd(int server_fd, int timeout_ms);
> >  int connect_to_fd_opts(int server_fd, const struct network_helper_opts *opts);
> >  int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms);
> >  int fastopen_connect(int server_fd, const char *data, unsigned int data_len,
>

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock
  2022-05-17  1:20   ` Martin KaFai Lau
@ 2022-05-17  5:29     ` Geliang Tang
  0 siblings, 0 replies; 25+ messages in thread
From: Geliang Tang @ 2022-05-17  5:29 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Geliang Tang, Mat Martineau, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, MPTCP Upstream,
	Matthieu Baerts

Martin KaFai Lau <kafai@fb.com> 于2022年5月17日周二 09:21写道：
>
> On Fri, May 13, 2022 at 03:48:24PM -0700, Mat Martineau wrote:
> [ ... ]
> > diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> > index bc09dba0b078..3feb7ff578e2 100644
> > --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
> > +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> > @@ -7,6 +7,7 @@
> >  #include "bpf_tcp_helpers.h"
> >
> >  char _license[] SEC("license") = "GPL";
> > +extern bool CONFIG_MPTCP __kconfig;
> >
> >  struct mptcp_storage {
> >       __u32 invoked;
> > @@ -24,6 +25,7 @@ SEC("sockops")
> >  int _sockops(struct bpf_sock_ops *ctx)
> >  {
> >       struct mptcp_storage *storage;
> > +     struct mptcp_sock *msk;
> >       int op = (int)ctx->op;
> >       struct tcp_sock *tsk;
> >       struct bpf_sock *sk;
> > @@ -41,11 +43,24 @@ int _sockops(struct bpf_sock_ops *ctx)
> >               return 1;
> >
> >       is_mptcp = bpf_core_field_exists(tsk->is_mptcp) ? tsk->is_mptcp : 0;
> > -     storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
> > -                                  BPF_SK_STORAGE_GET_F_CREATE);
> > -     if (!storage)
> > -             return 1;
> > +     if (!is_mptcp) {
> > +             storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
> > +                                          BPF_SK_STORAGE_GET_F_CREATE);
> > +             if (!storage)
> > +                     return 1;
> > +     } else {
> > +             if (!CONFIG_MPTCP)
> hmm... how is it possible ?  The above just tested "!is_mptcp".

Will drop this in v5, thanks.

>
> > +                     return 1;
> > +
> > +             msk = bpf_skc_to_mptcp_sock(sk);
> > +             if (!msk)
> > +                     return 1;
> >
> > +             storage = bpf_sk_storage_get(&socket_storage_map, msk, 0,
> > +                                          BPF_SK_STORAGE_GET_F_CREATE);
> > +             if (!storage)
> > +                     return 1;
> > +     }
> >       storage->invoked++;
> >       storage->is_mptcp = is_mptcp;
> >
> > --
> > 2.36.1
> >
>

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 3/7] selftests/bpf: add MPTCP test base
  2022-05-16 22:43   ` Andrii Nakryiko
@ 2022-05-17  5:30     ` Geliang Tang
  0 siblings, 0 replies; 25+ messages in thread
From: Geliang Tang @ 2022-05-17  5:30 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Mat Martineau, Networking, bpf, Nicolas Rybowski,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	MPTCP Upstream, Matthieu Baerts, Geliang Tang

Andrii Nakryiko <andrii.nakryiko@gmail.com> 于2022年5月17日周二 06:43写道：
>
> On Fri, May 13, 2022 at 3:48 PM Mat Martineau
> <mathew.j.martineau@linux.intel.com> wrote:
> >
> > From: Nicolas Rybowski <nicolas.rybowski@tessares.net>
> >
> > This patch adds a base for MPTCP specific tests.
> >
> > It is currently limited to the is_mptcp field in case of plain TCP
> > connection because there is no easy way to get the subflow sk from a msk
> > in userspace. This implies that we cannot lookup the sk_storage attached
> > to the subflow sk in the sockops program.
> >
> > v4:
> >  - add copyright 2022 (Andrii)
> >  - use ASSERT_* instead of CHECK_FAIL (Andrii)
> >  - drop SEC("version") (Andrii)
> >  - use is_mptcp in tcp_sock, instead of bpf_tcp_sock (Martin & Andrii)
> >
> > Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> > Co-developed-by: Geliang Tang <geliang.tang@suse.com>
> > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > Signed-off-by: Nicolas Rybowski <nicolas.rybowski@tessares.net>
> > Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> > ---
> >  MAINTAINERS                                   |   1 +
> >  tools/testing/selftests/bpf/bpf_tcp_helpers.h |   1 +
> >  tools/testing/selftests/bpf/config            |   1 +
> >  tools/testing/selftests/bpf/network_helpers.c |  43 ++++--
> >  tools/testing/selftests/bpf/network_helpers.h |   4 +
> >  .../testing/selftests/bpf/prog_tests/mptcp.c  | 136 ++++++++++++++++++
> >  .../testing/selftests/bpf/progs/mptcp_sock.c  |  53 +++++++
> >  7 files changed, 231 insertions(+), 8 deletions(-)
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/mptcp.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/mptcp_sock.c
> >
>
> Seems like bpf_core_field_exists() works fine for your use case and CI
> is green. See some selftest-specific issues below, though.
>
> [...]
>
> > +static int run_test(int cgroup_fd, int server_fd, bool is_mptcp)
> > +{
> > +       int client_fd, prog_fd, map_fd, err;
> > +       struct bpf_program *prog;
> > +       struct bpf_object *obj;
> > +       struct bpf_map *map;
> > +
> > +       obj = bpf_object__open("./mptcp_sock.o");
> > +       if (libbpf_get_error(obj))
> > +               return -EIO;
> > +
> > +       err = bpf_object__load(obj);
> > +       if (!ASSERT_OK(err, "bpf_object__load"))
> > +               goto out;
> > +
> > +       prog = bpf_object__find_program_by_name(obj, "_sockops");
>
> can you please use BPF skeleton instead of doing these lookups by
> name? See other tests that are including .skel.h headers for example

Sure, I will update this in v5.

>
> > +       if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name")) {
> > +               err = -EIO;
> > +               goto out;
> > +       }
> > +
>
> [...]
>
> > +void test_base(void)
> > +{
> > +       int server_fd, cgroup_fd;
> > +
> > +       cgroup_fd = test__join_cgroup("/mptcp");
> > +       if (CHECK_FAIL(cgroup_fd < 0))
> > +               return;
> > +
> > +       /* without MPTCP */
> > +       server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
> > +       if (CHECK_FAIL(server_fd < 0))
> > +               goto with_mptcp;
> > +
> > +       CHECK_FAIL(run_test(cgroup_fd, server_fd, false));
>
> please don't add new uses of CHECK_FAIL()
>
> > +
> > +       close(server_fd);
> > +
>
> [...]
>

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto
  2022-05-17  5:26     ` Geliang Tang
@ 2022-05-17 17:00       ` Alexei Starovoitov
  2022-05-19  0:38         ` Mat Martineau
  0 siblings, 1 reply; 25+ messages in thread
From: Alexei Starovoitov @ 2022-05-17 17:00 UTC (permalink / raw)
  To: Geliang Tang
  Cc: Martin KaFai Lau, Geliang Tang, Mat Martineau, Networking, bpf,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	MPTCP Upstream, Nicolas Rybowski, Matthieu Baerts

On Mon, May 16, 2022 at 10:26 PM Geliang Tang <geliangtang@gmail.com> wrote:
>
> Martin KaFai Lau <kafai@fb.com> 于2022年5月17日周二 09:07写道：
> >
> > On Fri, May 13, 2022 at 03:48:21PM -0700, Mat Martineau wrote:
> > [ ... ]
> >
> > > diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> > > index 8b1afd6f5cc4..2ba09de955c7 100644
> > > --- a/include/net/mptcp.h
> > > +++ b/include/net/mptcp.h
> > > @@ -284,4 +284,10 @@ static inline int mptcpv6_init(void) { return 0; }
> > >  static inline void mptcpv6_handle_mapped(struct sock *sk, bool mapped) { }
> > >  #endif
> > >
> > > +#if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_SYSCALL)
> > > +struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk);
> > Can this be inline ?
>
> This function can't be inline since it uses struct mptcp_subflow_context.
>
> mptcp_subflow_context is defined in net/mptcp/protocol.h, and we don't
> want to export it to user space in net/mptcp/protocol.h.

The above function can be made static inline in a header file.
That doesn't automatically expose it to user space.

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 5/7] selftests/bpf: verify token of struct mptcp_sock
  2022-05-17  5:19     ` Geliang Tang
@ 2022-05-17 21:30       ` Martin KaFai Lau
  0 siblings, 0 replies; 25+ messages in thread
From: Martin KaFai Lau @ 2022-05-17 21:30 UTC (permalink / raw)
  To: Geliang Tang
  Cc: Mat Martineau, Geliang Tang, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, MPTCP Upstream,
	Matthieu Baerts

On Tue, May 17, 2022 at 01:19:38PM +0800, Geliang Tang wrote:
> Martin KaFai Lau <kafai@fb.com> 于2022年5月17日周二 09:32写道：
> >
> > On Fri, May 13, 2022 at 03:48:25PM -0700, Mat Martineau wrote:
> > [ ... ]
> >
> > > +/*
> > > + * Parse the token from the output of 'ip mptcp monitor':
> > > + *
> > > + * [       CREATED] token=3ca933d3 remid=0 locid=0 saddr4=127.0.0.1 ...
> > > + * [       CREATED] token=2ab57040 remid=0 locid=0 saddr4=127.0.0.1 ...
> > How stable is the string format ?
> >
> > If it happens to have some unrelated mptcp connection going on, will the test
> > break ?
> 
> Hi Martin,
> 
> Yes, it will break in that case. How can I fix this? Should I run the
> test in a special net namespace?
> 
> 'ip mptcp monitor' can easily run in a special net namespace:
> 
> ip -net ns1 mptcp monitor
> 
> But I don't know how to run start_server() and connect_to_fd() in a
> special namespace. Could you please give me some suggestions about
> this?
You can take a look at the open_netns() usages under prog_tests/.
For example, tc_redirect.c.

I would also avoid string matching from "ip mptcp monitor" if possible
considering the command may not have support (test skip) and the
string format may change also.

One option, you can consider to directly trace some of the mptcp
functions by using bpf fentry/fexit prog to obtain the token and
save it in a global bpf variable.
Search 'SEC("fentry' or 'SEC("fexit' for some examples.  Then
the iproute2 support testing and the test skip logic can go away.

Suggesting them here because the test will have better chance
to catch issue if the test is not skipped or failed because
of string format not match.  I won't insist on not using
"ip mptcp monitor" though.

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto
  2022-05-17 17:00       ` Alexei Starovoitov
@ 2022-05-19  0:38         ` Mat Martineau
  2022-05-19 17:58           ` Martin KaFai Lau
  0 siblings, 1 reply; 25+ messages in thread
From: Mat Martineau @ 2022-05-19  0:38 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Geliang Tang, Martin KaFai Lau, Geliang Tang, Networking, bpf,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	MPTCP Upstream, Nicolas Rybowski, Matthieu Baerts

[-- Attachment #1: Type: text/plain, Size: 1577 bytes --]

On Tue, 17 May 2022, Alexei Starovoitov wrote:

> On Mon, May 16, 2022 at 10:26 PM Geliang Tang <geliangtang@gmail.com> wrote:
>>
>> Martin KaFai Lau <kafai@fb.com> 于2022年5月17日周二 09:07写道：
>>>
>>> On Fri, May 13, 2022 at 03:48:21PM -0700, Mat Martineau wrote:
>>> [ ... ]
>>>
>>>> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
>>>> index 8b1afd6f5cc4..2ba09de955c7 100644
>>>> --- a/include/net/mptcp.h
>>>> +++ b/include/net/mptcp.h
>>>> @@ -284,4 +284,10 @@ static inline int mptcpv6_init(void) { return 0; }
>>>>  static inline void mptcpv6_handle_mapped(struct sock *sk, bool mapped) { }
>>>>  #endif
>>>>
>>>> +#if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_SYSCALL)
>>>> +struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk);
>>> Can this be inline ?
>>
>> This function can't be inline since it uses struct mptcp_subflow_context.
>>
>> mptcp_subflow_context is defined in net/mptcp/protocol.h, and we don't
>> want to export it to user space in net/mptcp/protocol.h.
>
> The above function can be made static inline in a header file.
> That doesn't automatically expose it to user space.
>

True, it's not a question of userspace exposure. But making this one 
function inline involves a bunch of churn in the (non-BPF) mptcp headers 
that I'd rather avoid. The definitions in protocol.h are there because 
they aren't relevant outside of the mptcp subsystem code.

Does making this one function inline benefit BPF, specifically, in a 
meaningful way? If not, I'd like to leave it as-is.

--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 25+ messages in thread
* Re: [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto
  2022-05-19  0:38         ` Mat Martineau
@ 2022-05-19 17:58           ` Martin KaFai Lau
  0 siblings, 0 replies; 25+ messages in thread
From: Martin KaFai Lau @ 2022-05-19 17:58 UTC (permalink / raw)
  To: Mat Martineau
  Cc: Alexei Starovoitov, Geliang Tang, Geliang Tang, Networking, bpf,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	MPTCP Upstream, Nicolas Rybowski, Matthieu Baerts

On Wed, May 18, 2022 at 05:38:43PM -0700, Mat Martineau wrote:
> On Tue, 17 May 2022, Alexei Starovoitov wrote:
> 
> > On Mon, May 16, 2022 at 10:26 PM Geliang Tang <geliangtang@gmail.com> wrote:
> > > 
> > > Martin KaFai Lau <kafai@fb.com> 于2022年5月17日周二 09:07写道：
> > > > 
> > > > On Fri, May 13, 2022 at 03:48:21PM -0700, Mat Martineau wrote:
> > > > [ ... ]
> > > > 
> > > > > diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> > > > > index 8b1afd6f5cc4..2ba09de955c7 100644
> > > > > --- a/include/net/mptcp.h
> > > > > +++ b/include/net/mptcp.h
> > > > > @@ -284,4 +284,10 @@ static inline int mptcpv6_init(void) { return 0; }
> > > > >  static inline void mptcpv6_handle_mapped(struct sock *sk, bool mapped) { }
> > > > >  #endif
> > > > > 
> > > > > +#if defined(CONFIG_MPTCP) && defined(CONFIG_BPF_SYSCALL)
> > > > > +struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk);
> > > > Can this be inline ?
> > > 
> > > This function can't be inline since it uses struct mptcp_subflow_context.
> > > 
> > > mptcp_subflow_context is defined in net/mptcp/protocol.h, and we don't
> > > want to export it to user space in net/mptcp/protocol.h.
> > 
> > The above function can be made static inline in a header file.
> > That doesn't automatically expose it to user space.
> > 
> 
> True, it's not a question of userspace exposure. But making this one
> function inline involves a bunch of churn in the (non-BPF) mptcp headers
> that I'd rather avoid. The definitions in protocol.h are there because they
> aren't relevant outside of the mptcp subsystem code.
> 
> Does making this one function inline benefit BPF, specifically, in a
> meaningful way?
I believe this is similar to the already inlined mptcp_subflow_ctx().

> If not, I'd like to leave it as-is.
Sure, it is fine to leave it if the churn is too much.

Was suggesting because all other bpf_skc_to_* cast helpers
don't have this call-out.  Just in case if those cast helpers will be
inlined by verifier in the future, this won't be the only one that
got left behind.

^ permalink raw reply	[flat|nested] 25+ messages in thread
end of thread, other threads:[~2022-05-19 17:58 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13 22:48 [PATCH bpf-next v4 0/7] bpf: mptcp: Support for mptcp_sock Mat Martineau
2022-05-13 22:48 ` [PATCH bpf-next v4 1/7] bpf: add bpf_skc_to_mptcp_sock_proto Mat Martineau
2022-05-17  1:07   ` Martin KaFai Lau
2022-05-17  5:26     ` Geliang Tang
2022-05-17 17:00       ` Alexei Starovoitov
2022-05-19  0:38         ` Mat Martineau
2022-05-19 17:58           ` Martin KaFai Lau
2022-05-13 22:48 ` [PATCH bpf-next v4 2/7] selftests/bpf: Enable CONFIG_IKCONFIG_PROC in config Mat Martineau
2022-05-13 22:48 ` [PATCH bpf-next v4 3/7] selftests/bpf: add MPTCP test base Mat Martineau
2022-05-16 22:43   ` Andrii Nakryiko
2022-05-17  5:30     ` Geliang Tang
2022-05-17  1:18   ` Martin KaFai Lau
2022-05-17  5:28     ` Geliang Tang
2022-05-13 22:48 ` [PATCH bpf-next v4 4/7] selftests/bpf: test bpf_skc_to_mptcp_sock Mat Martineau
2022-05-13 23:53   ` Geliang Tang
2022-05-16 22:47   ` Andrii Nakryiko
2022-05-17  1:20   ` Martin KaFai Lau
2022-05-17  5:29     ` Geliang Tang
2022-05-13 22:48 ` [PATCH bpf-next v4 5/7] selftests/bpf: verify token of struct mptcp_sock Mat Martineau
2022-05-16 22:45   ` Andrii Nakryiko
2022-05-17  1:32   ` Martin KaFai Lau
2022-05-17  5:19     ` Geliang Tang
2022-05-17 21:30       ` Martin KaFai Lau
2022-05-13 22:48 ` [PATCH bpf-next v4 6/7] selftests/bpf: verify ca_name " Mat Martineau
2022-05-13 22:48 ` [PATCH bpf-next v4 7/7] selftests/bpf: verify first " Mat Martineau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.