bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp
@ 2022-05-02 21:12 Mat Martineau
  2022-05-02 21:12 ` [PATCH bpf-next v3 1/8] bpf: expose is_mptcp flag to bpf_tcp_sock Mat Martineau
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Mat Martineau @ 2022-05-02 21:12 UTC (permalink / raw)
  To: netdev, bpf; +Cc: Mat Martineau, ast, daniel, andrii, mptcp

This patch set adds BPF access to the is_mptcp flag in tcp_sock and
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.


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 (2):
  bpf: expose is_mptcp flag to bpf_tcp_sock
  selftests: bpf: add MPTCP test base

 MAINTAINERS                                   |   2 +
 include/linux/bpf.h                           |   1 +
 include/linux/btf_ids.h                       |   3 +-
 include/net/mptcp.h                           |   6 +
 include/uapi/linux/bpf.h                      |   8 +
 kernel/bpf/verifier.c                         |   1 +
 kernel/trace/bpf_trace.c                      |   2 +
 net/core/filter.c                             |  27 +-
 net/mptcp/Makefile                            |   2 +
 net/mptcp/bpf.c                               |  22 ++
 scripts/bpf_doc.py                            |   2 +
 tools/include/uapi/linux/bpf.h                |   8 +
 .../testing/selftests/bpf/bpf_mptcp_helpers.h |  17 ++
 tools/testing/selftests/bpf/bpf_tcp_helpers.h |   4 +
 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  | 272 ++++++++++++++++++
 .../testing/selftests/bpf/progs/mptcp_sock.c  |  80 ++++++
 19 files changed, 497 insertions(+), 10 deletions(-)
 create mode 100644 net/mptcp/bpf.c
 create mode 100644 tools/testing/selftests/bpf/bpf_mptcp_helpers.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/mptcp.c
 create mode 100644 tools/testing/selftests/bpf/progs/mptcp_sock.c


base-commit: 20b87e7c29dffcfa3f96f2e99daec84fd46cabdb
-- 
2.36.0


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH bpf-next v3 1/8] bpf: expose is_mptcp flag to bpf_tcp_sock
  2022-05-02 21:12 [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Mat Martineau
@ 2022-05-02 21:12 ` Mat Martineau
  2022-05-11  0:48   ` Martin KaFai Lau
  2022-05-02 21:12 ` [PATCH bpf-next v3 2/8] bpf: add bpf_skc_to_mptcp_sock_proto Mat Martineau
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Mat Martineau @ 2022-05-02 21:12 UTC (permalink / raw)
  To: netdev, bpf
  Cc: Nicolas Rybowski, ast, daniel, andrii, mptcp, Matthieu Baerts,
	Mat Martineau

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

is_mptcp is a field from struct tcp_sock used to indicate that the
current tcp_sock is part of the MPTCP protocol.

In this protocol, a first socket (mptcp_sock) is created with
sk_protocol set to IPPROTO_MPTCP (=262) for control purpose but it
isn't directly on the wire. This is the role of the subflow (kernel)
sockets which are classical tcp_sock with sk_protocol set to
IPPROTO_TCP. The only way to differentiate such sockets from plain TCP
sockets is the is_mptcp field from tcp_sock.

Such an exposure in BPF is thus required to be able to differentiate
plain TCP sockets from MPTCP subflow sockets in BPF_PROG_TYPE_SOCK_OPS
programs.

The choice has been made to silently pass the case when CONFIG_MPTCP is
unset by defaulting is_mptcp to 0 in order to make BPF independent of
the MPTCP configuration. Another solution is to make the verifier fail
in 'bpf_tcp_sock_is_valid_ctx_access' but this will add an additional
'#ifdef CONFIG_MPTCP' in the BPF code and a same injected BPF program
will not run if MPTCP is not set.

An example use-case is provided in
https://github.com/multipath-tcp/mptcp_net-next/tree/scripts/bpf/examples

Suggested-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Nicolas Rybowski <nicolas.rybowski@tessares.net>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 include/uapi/linux/bpf.h       | 1 +
 net/core/filter.c              | 9 ++++++++-
 tools/include/uapi/linux/bpf.h | 1 +
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 444fe6f1cf35..7043f3641534 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5706,6 +5706,7 @@ struct bpf_tcp_sock {
 	__u32 delivered;	/* Total data packets delivered incl. rexmits */
 	__u32 delivered_ce;	/* Like the above but only ECE marked packets */
 	__u32 icsk_retransmits;	/* Number of unrecovered [RTO] timeouts */
+	__u32 is_mptcp;		/* Is MPTCP subflow? */
 };
 
 struct bpf_sock_tuple {
diff --git a/net/core/filter.c b/net/core/filter.c
index b741b9f7e6a9..b474e5bd1458 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6754,7 +6754,7 @@ bool bpf_tcp_sock_is_valid_access(int off, int size, enum bpf_access_type type,
 				  struct bpf_insn_access_aux *info)
 {
 	if (off < 0 || off >= offsetofend(struct bpf_tcp_sock,
-					  icsk_retransmits))
+					  is_mptcp))
 		return false;
 
 	if (off % size != 0)
@@ -6888,6 +6888,13 @@ u32 bpf_tcp_sock_convert_ctx_access(enum bpf_access_type type,
 	case offsetof(struct bpf_tcp_sock, icsk_retransmits):
 		BPF_INET_SOCK_GET_COMMON(icsk_retransmits);
 		break;
+	case offsetof(struct bpf_tcp_sock, is_mptcp):
+#ifdef CONFIG_MPTCP
+		BPF_TCP_SOCK_GET_COMMON(is_mptcp);
+#else
+		*insn++ = BPF_MOV32_IMM(si->dst_reg, 0);
+#endif
+		break;
 	}
 
 	return insn - insn_buf;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 444fe6f1cf35..7043f3641534 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5706,6 +5706,7 @@ struct bpf_tcp_sock {
 	__u32 delivered;	/* Total data packets delivered incl. rexmits */
 	__u32 delivered_ce;	/* Like the above but only ECE marked packets */
 	__u32 icsk_retransmits;	/* Number of unrecovered [RTO] timeouts */
+	__u32 is_mptcp;		/* Is MPTCP subflow? */
 };
 
 struct bpf_sock_tuple {
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH bpf-next v3 2/8] bpf: add bpf_skc_to_mptcp_sock_proto
  2022-05-02 21:12 [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Mat Martineau
  2022-05-02 21:12 ` [PATCH bpf-next v3 1/8] bpf: expose is_mptcp flag to bpf_tcp_sock Mat Martineau
@ 2022-05-02 21:12 ` Mat Martineau
  2022-05-02 21:12 ` [PATCH bpf-next v3 3/8] selftests: bpf: Enable CONFIG_IKCONFIG_PROC in config Mat Martineau
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 24+ messages in thread
From: Mat Martineau @ 2022-05-02 21:12 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 be94833d390a..f53e39065a6e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2204,6 +2204,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 7043f3641534..56a66778dc28 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5154,6 +5154,12 @@ union bpf_attr {
  *		if not NULL, is a reference which must be released using its
  *		corresponding release function, or moved into a BPF map before
  *		program exit.
+ *
+ * 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),			\
@@ -5351,6 +5357,7 @@ union bpf_attr {
 	FN(skb_set_tstamp),		\
 	FN(ima_file_hash),		\
 	FN(kptr_xchg),			\
+	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 813f6ee80419..3d8790e81c48 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 f15b826f9899..8451fc83d031 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1688,6 +1688,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 b474e5bd1458..b39e4af9a0ec 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);
@@ -11279,6 +11280,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);
@@ -11321,6 +11336,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 7043f3641534..56a66778dc28 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5154,6 +5154,12 @@ union bpf_attr {
  *		if not NULL, is a reference which must be released using its
  *		corresponding release function, or moved into a BPF map before
  *		program exit.
+ *
+ * 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),			\
@@ -5351,6 +5357,7 @@ union bpf_attr {
 	FN(skb_set_tstamp),		\
 	FN(ima_file_hash),		\
 	FN(kptr_xchg),			\
+	FN(skc_to_mptcp_sock),		\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH bpf-next v3 3/8] selftests: bpf: Enable CONFIG_IKCONFIG_PROC in config
  2022-05-02 21:12 [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Mat Martineau
  2022-05-02 21:12 ` [PATCH bpf-next v3 1/8] bpf: expose is_mptcp flag to bpf_tcp_sock Mat Martineau
  2022-05-02 21:12 ` [PATCH bpf-next v3 2/8] bpf: add bpf_skc_to_mptcp_sock_proto Mat Martineau
@ 2022-05-02 21:12 ` Mat Martineau
  2022-05-06 22:25   ` Andrii Nakryiko
  2022-05-02 21:12 ` [PATCH bpf-next v3 4/8] selftests: bpf: add MPTCP test base Mat Martineau
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Mat Martineau @ 2022-05-02 21:12 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>
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 763db63a3890..8d7faff33c54 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -53,3 +53,5 @@ CONFIG_NF_DEFRAG_IPV4=y
 CONFIG_NF_DEFRAG_IPV6=y
 CONFIG_NF_CONNTRACK=y
 CONFIG_USERFAULTFD=y
+CONFIG_IKCONFIG=y
+CONFIG_IKCONFIG_PROC=y
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH bpf-next v3 4/8] selftests: bpf: add MPTCP test base
  2022-05-02 21:12 [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Mat Martineau
                   ` (2 preceding siblings ...)
  2022-05-02 21:12 ` [PATCH bpf-next v3 3/8] selftests: bpf: Enable CONFIG_IKCONFIG_PROC in config Mat Martineau
@ 2022-05-02 21:12 ` Mat Martineau
  2022-05-06 22:24   ` Andrii Nakryiko
  2022-05-02 21:12 ` [PATCH bpf-next v3 5/8] selftests: bpf: test bpf_skc_to_mptcp_sock Mat Martineau
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Mat Martineau @ 2022-05-02 21:12 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.

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/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  |  50 +++++++
 6 files changed, 227 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/config b/tools/testing/selftests/bpf/config
index 8d7faff33c54..a25e15d55918 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -55,3 +55,4 @@ CONFIG_NF_CONNTRACK=y
 CONFIG_USERFAULTFD=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..cd548bb2828f
--- /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. */
+
+#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 = 0, cfd = client_fd;
+	struct mptcp_storage val;
+
+	if (is_mptcp == 1)
+		return 0;
+
+	if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) {
+		perror("Failed to read socket storage");
+		return -1;
+	}
+
+	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 (CHECK_FAIL(err))
+		goto out;
+
+	prog = bpf_object__find_program_by_name(obj, "_sockops");
+	if (CHECK_FAIL(!prog)) {
+		err = -EIO;
+		goto out;
+	}
+
+	prog_fd = bpf_program__fd(prog);
+	if (CHECK_FAIL(prog_fd < 0)) {
+		err = -EIO;
+		goto out;
+	}
+
+	map = bpf_object__find_map_by_name(obj, "socket_storage_map");
+	if (CHECK_FAIL(!map)) {
+		err = -EIO;
+		goto out;
+	}
+
+	map_fd = bpf_map__fd(map);
+	if (CHECK_FAIL(map_fd < 0)) {
+		err = -EIO;
+		goto out;
+	}
+
+	err = bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_SOCK_OPS, 0);
+	if (CHECK_FAIL(err))
+		goto out;
+
+	client_fd = is_mptcp ? connect_to_mptcp_fd(server_fd, 0) :
+			       connect_to_fd(server_fd, 0);
+	if (client_fd < 0) {
+		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..0d65fb889d03
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020, Tessares SA. */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
+
+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;
+	struct bpf_tcp_sock *tcp_sk;
+	int op = (int)ctx->op;
+	struct bpf_sock *sk;
+
+	if (op != BPF_SOCK_OPS_TCP_CONNECT_CB)
+		return 1;
+
+	sk = ctx->sk;
+	if (!sk)
+		return 1;
+
+	tcp_sk = bpf_tcp_sock(sk);
+	if (!tcp_sk)
+		return 1;
+
+	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 = tcp_sk->is_mptcp;
+
+	return 1;
+}
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH bpf-next v3 5/8] selftests: bpf: test bpf_skc_to_mptcp_sock
  2022-05-02 21:12 [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Mat Martineau
                   ` (3 preceding siblings ...)
  2022-05-02 21:12 ` [PATCH bpf-next v3 4/8] selftests: bpf: add MPTCP test base Mat Martineau
@ 2022-05-02 21:12 ` Mat Martineau
  2022-05-06 22:26   ` Andrii Nakryiko
  2022-05-02 21:12 ` [PATCH bpf-next v3 6/8] selftests: bpf: verify token of struct mptcp_sock Mat Martineau
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Mat Martineau @ 2022-05-02 21:12 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

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>
---
 MAINTAINERS                                   |  1 +
 .../testing/selftests/bpf/bpf_mptcp_helpers.h | 14 ++++++++
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 36 +++++++++++++++----
 .../testing/selftests/bpf/progs/mptcp_sock.c  | 24 ++++++++++---
 4 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/bpf_mptcp_helpers.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 359afc617b92..d48d3cb6abbc 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/bpf_mptcp_helpers.h
 F:	tools/testing/selftests/bpf/*/*mptcp*.c
 F:	tools/testing/selftests/net/mptcp/
 
diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
new file mode 100644
index 000000000000..18da4cc65e89
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2022, SUSE. */
+
+#ifndef __BPF_MPTCP_HELPERS_H
+#define __BPF_MPTCP_HELPERS_H
+
+#include "bpf_tcp_helpers.h"
+
+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 cd548bb2828f..4b40bbdaf91f 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -10,14 +10,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 = 0, cfd = client_fd;
 	struct mptcp_storage val;
 
-	if (is_mptcp == 1)
-		return 0;
-
 	if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) {
 		perror("Failed to read socket storage");
 		return -1;
@@ -38,6 +36,32 @@ 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 = 0, cfd = client_fd;
+	struct mptcp_storage val;
+
+	if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) {
+		perror("Failed to read socket storage");
+		return -1;
+	}
+
+	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 +112,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);
 
diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c
index 0d65fb889d03..7b6a25e37de8 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -3,9 +3,11 @@
 
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
+#include "bpf_mptcp_helpers.h"
 
 char _license[] SEC("license") = "GPL";
 __u32 _version SEC("version") = 1;
+extern bool CONFIG_MPTCP __kconfig;
 
 struct mptcp_storage {
 	__u32 invoked;
@@ -24,6 +26,7 @@ int _sockops(struct bpf_sock_ops *ctx)
 {
 	struct mptcp_storage *storage;
 	struct bpf_tcp_sock *tcp_sk;
+	struct mptcp_sock *msk;
 	int op = (int)ctx->op;
 	struct bpf_sock *sk;
 
@@ -38,11 +41,24 @@ int _sockops(struct bpf_sock_ops *ctx)
 	if (!tcp_sk)
 		return 1;
 
-	storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
-				     BPF_SK_STORAGE_GET_F_CREATE);
-	if (!storage)
-		return 1;
+	if (!tcp_sk->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 = tcp_sk->is_mptcp;
 
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH bpf-next v3 6/8] selftests: bpf: verify token of struct mptcp_sock
  2022-05-02 21:12 [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Mat Martineau
                   ` (4 preceding siblings ...)
  2022-05-02 21:12 ` [PATCH bpf-next v3 5/8] selftests: bpf: test bpf_skc_to_mptcp_sock Mat Martineau
@ 2022-05-02 21:12 ` Mat Martineau
  2022-05-02 22:14   ` Mat Martineau
  2022-05-10 21:59   ` Mat Martineau
  2022-05-02 21:12 ` [PATCH bpf-next v3 7/8] selftests: bpf: verify ca_name " Mat Martineau
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 24+ messages in thread
From: Mat Martineau @ 2022-05-02 21:12 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().

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>
---
 .../testing/selftests/bpf/bpf_mptcp_helpers.h |  1 +
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 66 +++++++++++++++++++
 .../testing/selftests/bpf/progs/mptcp_sock.c  |  5 ++
 3 files changed, 72 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
index 18da4cc65e89..87e15810997d 100644
--- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
@@ -9,6 +9,7 @@
 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 4b40bbdaf91f..c5d96ba81e04 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -8,8 +8,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,58 @@ 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 (CHECK_FAIL(fd < 0)) {
+		log_err("Failed to open %s", monitor_log_path);
+		return token;
+	}
+
+	len = read(fd, buf, sizeof(buf));
+	if (CHECK_FAIL(len < 0)) {
+		log_err("Failed to read %s", 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 = 0, cfd = client_fd;
 	struct mptcp_storage val;
+	__u32 token;
+
+	token = get_msk_token();
+	if (token <= 0) {
+		log_err("Unexpected token %x", token);
+		return -1;
+	}
 
 	if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) {
 		perror("Failed to read socket storage");
@@ -59,6 +109,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;
 }
 
@@ -124,6 +180,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");
@@ -141,6 +198,13 @@ void test_base(void)
 
 with_mptcp:
 	/* with MPTCP */
+	if (CHECK_FAIL(!mkdtemp(tmp_dir)))
+		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 (CHECK_FAIL(system(cmd)))
+		goto close_cgroup_fd;
 	server_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
 	if (CHECK_FAIL(server_fd < 0))
 		goto close_cgroup_fd;
@@ -148,6 +212,8 @@ void test_base(void)
 	CHECK_FAIL(run_test(cgroup_fd, server_fd, true));
 
 	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 7b6a25e37de8..c58c191d8416 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 {
@@ -46,6 +47,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;
@@ -58,6 +61,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 = tcp_sk->is_mptcp;
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH bpf-next v3 7/8] selftests: bpf: verify ca_name of struct mptcp_sock
  2022-05-02 21:12 [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Mat Martineau
                   ` (5 preceding siblings ...)
  2022-05-02 21:12 ` [PATCH bpf-next v3 6/8] selftests: bpf: verify token of struct mptcp_sock Mat Martineau
@ 2022-05-02 21:12 ` Mat Martineau
  2022-05-02 21:12 ` [PATCH bpf-next v3 8/8] selftests: bpf: verify first " Mat Martineau
  2022-05-06 22:28 ` [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Andrii Nakryiko
  8 siblings, 0 replies; 24+ messages in thread
From: Mat Martineau @ 2022-05-02 21:12 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.

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>
---
 .../testing/selftests/bpf/bpf_mptcp_helpers.h |  1 +
 tools/testing/selftests/bpf/bpf_tcp_helpers.h |  4 ++
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 38 +++++++++++++++++++
 .../testing/selftests/bpf/progs/mptcp_sock.c  |  4 ++
 4 files changed, 47 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
index 87e15810997d..463e4e061c96 100644
--- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
@@ -10,6 +10,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/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index b1ede6f0b821..89750d732cfa 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 {
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index c5d96ba81e04..f2d22507431c 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -5,10 +5,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];
@@ -79,11 +84,36 @@ 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 (CHECK_FAIL(fd < 0)) {
+		log_err("Failed to open tcp_congestion_control");
+		return;
+	}
+
+	len = read(fd, ca_name, TCP_CA_NAME_MAX);
+	if (CHECK_FAIL(len < 0)) {
+		log_err("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 = 0, cfd = client_fd;
 	struct mptcp_storage val;
+	char ca_name[TCP_CA_NAME_MAX];
 	__u32 token;
 
 	token = get_msk_token();
@@ -92,6 +122,8 @@ static int verify_msk(int map_fd, int client_fd)
 		return -1;
 	}
 
+	get_msk_ca_name(ca_name);
+
 	if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) {
 		perror("Failed to read socket storage");
 		return -1;
@@ -115,6 +147,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 c58c191d8416..226571673800 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_sock.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2020, Tessares SA. */
 
+#include <string.h>
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include "bpf_mptcp_helpers.h"
@@ -13,6 +14,7 @@ struct mptcp_storage {
 	__u32 invoked;
 	__u32 is_mptcp;
 	__u32 token;
+	char ca_name[TCP_CA_NAME_MAX];
 };
 
 struct {
@@ -49,6 +51,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;
@@ -63,6 +66,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 = tcp_sk->is_mptcp;
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH bpf-next v3 8/8] selftests: bpf: verify first of struct mptcp_sock
  2022-05-02 21:12 [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Mat Martineau
                   ` (6 preceding siblings ...)
  2022-05-02 21:12 ` [PATCH bpf-next v3 7/8] selftests: bpf: verify ca_name " Mat Martineau
@ 2022-05-02 21:12 ` Mat Martineau
  2022-05-06 22:28 ` [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Andrii Nakryiko
  8 siblings, 0 replies; 24+ messages in thread
From: Mat Martineau @ 2022-05-02 21:12 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_mptcp_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_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
index 463e4e061c96..b5a43b108982 100644
--- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
@@ -10,6 +10,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 f2d22507431c..ed5773c26045 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -12,7 +12,9 @@
 struct mptcp_storage {
 	__u32 invoked;
 	__u32 is_mptcp;
+	struct sock *sk;
 	__u32 token;
+	struct sock *first;
 	char ca_name[TCP_CA_NAME_MAX];
 };
 
@@ -147,6 +149,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 226571673800..b1e7f3b4330a 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];
 };
 
@@ -52,6 +54,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;
@@ -67,9 +70,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 = tcp_sk->is_mptcp;
+	storage->sk = (struct sock *)sk;
 
 	return 1;
 }
-- 
2.36.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 6/8] selftests: bpf: verify token of struct mptcp_sock
  2022-05-02 21:12 ` [PATCH bpf-next v3 6/8] selftests: bpf: verify token of struct mptcp_sock Mat Martineau
@ 2022-05-02 22:14   ` Mat Martineau
  2022-05-06 22:27     ` Andrii Nakryiko
  2022-05-10 21:59   ` Mat Martineau
  1 sibling, 1 reply; 24+ messages in thread
From: Mat Martineau @ 2022-05-02 22:14 UTC (permalink / raw)
  To: netdev, bpf, daniel, andrii; +Cc: Geliang Tang, ast, mptcp, Matthieu Baerts

On Mon, 2 May 2022, Mat Martineau 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().
>

Daniel, Andrii,

The z15 CI build failed on this commit, not due to any endianness issue 
but because it appears the z15 CI VM has an older iproute2 version (5.5.0) 
than the x86_64 VM (where the build succeeded).

Do you need us (MPTCP) to change anything?

Thanks!

> 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>
> ---
> .../testing/selftests/bpf/bpf_mptcp_helpers.h |  1 +
> .../testing/selftests/bpf/prog_tests/mptcp.c  | 66 +++++++++++++++++++
> .../testing/selftests/bpf/progs/mptcp_sock.c  |  5 ++
> 3 files changed, 72 insertions(+)

--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 4/8] selftests: bpf: add MPTCP test base
  2022-05-02 21:12 ` [PATCH bpf-next v3 4/8] selftests: bpf: add MPTCP test base Mat Martineau
@ 2022-05-06 22:24   ` Andrii Nakryiko
  0 siblings, 0 replies; 24+ messages in thread
From: Andrii Nakryiko @ 2022-05-06 22:24 UTC (permalink / raw)
  To: Mat Martineau
  Cc: Networking, bpf, Nicolas Rybowski, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, mptcp, Matthieu Baerts,
	Geliang Tang

On Mon, May 2, 2022 at 2:12 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.
>
> 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/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  |  50 +++++++
>  6 files changed, 227 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
>

[...]

>  /* 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..cd548bb2828f
> --- /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. */

2022?

> +
> +#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 = 0, cfd = client_fd;
> +       struct mptcp_storage val;
> +
> +       if (is_mptcp == 1)
> +               return 0;
> +
> +       if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) {

don't use CHECK and especially CHECK_FAIL, please use ASSERT_xxx()
macros instead

> +               perror("Failed to read socket storage");
> +               return -1;
> +       }
> +

[...]

> 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..0d65fb889d03
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c
> @@ -0,0 +1,50 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2020, Tessares SA. */
> +
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;

version is not needed, please drop

> +
> +struct mptcp_storage {
> +       __u32 invoked;
> +       __u32 is_mptcp;
> +};

[...]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 3/8] selftests: bpf: Enable CONFIG_IKCONFIG_PROC in config
  2022-05-02 21:12 ` [PATCH bpf-next v3 3/8] selftests: bpf: Enable CONFIG_IKCONFIG_PROC in config Mat Martineau
@ 2022-05-06 22:25   ` Andrii Nakryiko
  0 siblings, 0 replies; 24+ messages in thread
From: Andrii Nakryiko @ 2022-05-06 22:25 UTC (permalink / raw)
  To: Mat Martineau
  Cc: Networking, bpf, Geliang Tang, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, mptcp

On Mon, May 2, 2022 at 2:12 PM Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
>
> 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>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---

Acked-by: Andrii Nakryiko <andrii@kernel.org>

>  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 763db63a3890..8d7faff33c54 100644
> --- a/tools/testing/selftests/bpf/config
> +++ b/tools/testing/selftests/bpf/config
> @@ -53,3 +53,5 @@ CONFIG_NF_DEFRAG_IPV4=y
>  CONFIG_NF_DEFRAG_IPV6=y
>  CONFIG_NF_CONNTRACK=y
>  CONFIG_USERFAULTFD=y
> +CONFIG_IKCONFIG=y
> +CONFIG_IKCONFIG_PROC=y
> --
> 2.36.0
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 5/8] selftests: bpf: test bpf_skc_to_mptcp_sock
  2022-05-02 21:12 ` [PATCH bpf-next v3 5/8] selftests: bpf: test bpf_skc_to_mptcp_sock Mat Martineau
@ 2022-05-06 22:26   ` Andrii Nakryiko
  2022-05-09  9:00     ` Matthieu Baerts
  0 siblings, 1 reply; 24+ messages in thread
From: Andrii Nakryiko @ 2022-05-06 22:26 UTC (permalink / raw)
  To: Mat Martineau
  Cc: Networking, bpf, Geliang Tang, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, mptcp, Matthieu Baerts

On Mon, May 2, 2022 at 2:12 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
>
> 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>
> ---
>  MAINTAINERS                                   |  1 +
>  .../testing/selftests/bpf/bpf_mptcp_helpers.h | 14 ++++++++
>  .../testing/selftests/bpf/prog_tests/mptcp.c  | 36 +++++++++++++++----
>  .../testing/selftests/bpf/progs/mptcp_sock.c  | 24 ++++++++++---
>  4 files changed, 65 insertions(+), 10 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/bpf_mptcp_helpers.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 359afc617b92..d48d3cb6abbc 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/bpf_mptcp_helpers.h
>  F:     tools/testing/selftests/bpf/*/*mptcp*.c
>  F:     tools/testing/selftests/net/mptcp/
>
> diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> new file mode 100644
> index 000000000000..18da4cc65e89
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2022, SUSE. */
> +
> +#ifndef __BPF_MPTCP_HELPERS_H
> +#define __BPF_MPTCP_HELPERS_H
> +
> +#include "bpf_tcp_helpers.h"
> +
> +struct mptcp_sock {
> +       struct inet_connection_sock     sk;
> +
> +} __attribute__((preserve_access_index));

why can't all this live in bpf_tcp_helpers.h? why do we need extra header?

> +
> +#endif
> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index cd548bb2828f..4b40bbdaf91f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -10,14 +10,12 @@ struct mptcp_storage {
>         __u32 is_mptcp;
>  };
>

[...]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 6/8] selftests: bpf: verify token of struct mptcp_sock
  2022-05-02 22:14   ` Mat Martineau
@ 2022-05-06 22:27     ` Andrii Nakryiko
  0 siblings, 0 replies; 24+ messages in thread
From: Andrii Nakryiko @ 2022-05-06 22:27 UTC (permalink / raw)
  To: Mat Martineau, Ilya Leoshkevich
  Cc: Networking, bpf, Daniel Borkmann, Andrii Nakryiko, Geliang Tang,
	Alexei Starovoitov, mptcp, Matthieu Baerts

On Mon, May 2, 2022 at 3:14 PM Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
>
> On Mon, 2 May 2022, Mat Martineau 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().
> >
>
> Daniel, Andrii,
>
> The z15 CI build failed on this commit, not due to any endianness issue
> but because it appears the z15 CI VM has an older iproute2 version (5.5.0)
> than the x86_64 VM (where the build succeeded).
>
> Do you need us (MPTCP) to change anything?
>

I'll defer to Ilya (cc'ed).

> Thanks!
>
> > 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>
> > ---
> > .../testing/selftests/bpf/bpf_mptcp_helpers.h |  1 +
> > .../testing/selftests/bpf/prog_tests/mptcp.c  | 66 +++++++++++++++++++
> > .../testing/selftests/bpf/progs/mptcp_sock.c  |  5 ++
> > 3 files changed, 72 insertions(+)
>
> --
> Mat Martineau
> Intel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp
  2022-05-02 21:12 [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Mat Martineau
                   ` (7 preceding siblings ...)
  2022-05-02 21:12 ` [PATCH bpf-next v3 8/8] selftests: bpf: verify first " Mat Martineau
@ 2022-05-06 22:28 ` Andrii Nakryiko
  2022-05-06 23:06   ` Mat Martineau
  8 siblings, 1 reply; 24+ messages in thread
From: Andrii Nakryiko @ 2022-05-06 22:28 UTC (permalink / raw)
  To: Mat Martineau
  Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, mptcp

On Mon, May 2, 2022 at 2:12 PM Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
>
> This patch set adds BPF access to the is_mptcp flag in tcp_sock and
> 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.
>
>
> 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
>

It would be nice to use more consistent with the majority of other
commits "selftests/bpf: " prefix. Thank you.

> Nicolas Rybowski (2):
>   bpf: expose is_mptcp flag to bpf_tcp_sock
>   selftests: bpf: add MPTCP test base
>
>  MAINTAINERS                                   |   2 +
>  include/linux/bpf.h                           |   1 +
>  include/linux/btf_ids.h                       |   3 +-
>  include/net/mptcp.h                           |   6 +
>  include/uapi/linux/bpf.h                      |   8 +
>  kernel/bpf/verifier.c                         |   1 +
>  kernel/trace/bpf_trace.c                      |   2 +
>  net/core/filter.c                             |  27 +-
>  net/mptcp/Makefile                            |   2 +
>  net/mptcp/bpf.c                               |  22 ++
>  scripts/bpf_doc.py                            |   2 +
>  tools/include/uapi/linux/bpf.h                |   8 +
>  .../testing/selftests/bpf/bpf_mptcp_helpers.h |  17 ++
>  tools/testing/selftests/bpf/bpf_tcp_helpers.h |   4 +
>  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  | 272 ++++++++++++++++++
>  .../testing/selftests/bpf/progs/mptcp_sock.c  |  80 ++++++
>  19 files changed, 497 insertions(+), 10 deletions(-)
>  create mode 100644 net/mptcp/bpf.c
>  create mode 100644 tools/testing/selftests/bpf/bpf_mptcp_helpers.h
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/mptcp.c
>  create mode 100644 tools/testing/selftests/bpf/progs/mptcp_sock.c
>
>
> base-commit: 20b87e7c29dffcfa3f96f2e99daec84fd46cabdb
> --
> 2.36.0
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp
  2022-05-06 22:28 ` [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Andrii Nakryiko
@ 2022-05-06 23:06   ` Mat Martineau
  0 siblings, 0 replies; 24+ messages in thread
From: Mat Martineau @ 2022-05-06 23:06 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, mptcp

On Fri, 6 May 2022, Andrii Nakryiko wrote:

> On Mon, May 2, 2022 at 2:12 PM Mat Martineau
> <mathew.j.martineau@linux.intel.com> wrote:
>>
>> This patch set adds BPF access to the is_mptcp flag in tcp_sock and
>> 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.
>>
>>
>> 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
>>
>
> It would be nice to use more consistent with the majority of other
> commits "selftests/bpf: " prefix. Thank you.

Sure, we'll update those prefixes and address your comments in the 
individual patches. Thanks for the review.

- Mat

>
>> Nicolas Rybowski (2):
>>   bpf: expose is_mptcp flag to bpf_tcp_sock
>>   selftests: bpf: add MPTCP test base
>>
>>  MAINTAINERS                                   |   2 +
>>  include/linux/bpf.h                           |   1 +
>>  include/linux/btf_ids.h                       |   3 +-
>>  include/net/mptcp.h                           |   6 +
>>  include/uapi/linux/bpf.h                      |   8 +
>>  kernel/bpf/verifier.c                         |   1 +
>>  kernel/trace/bpf_trace.c                      |   2 +
>>  net/core/filter.c                             |  27 +-
>>  net/mptcp/Makefile                            |   2 +
>>  net/mptcp/bpf.c                               |  22 ++
>>  scripts/bpf_doc.py                            |   2 +
>>  tools/include/uapi/linux/bpf.h                |   8 +
>>  .../testing/selftests/bpf/bpf_mptcp_helpers.h |  17 ++
>>  tools/testing/selftests/bpf/bpf_tcp_helpers.h |   4 +
>>  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  | 272 ++++++++++++++++++
>>  .../testing/selftests/bpf/progs/mptcp_sock.c  |  80 ++++++
>>  19 files changed, 497 insertions(+), 10 deletions(-)
>>  create mode 100644 net/mptcp/bpf.c
>>  create mode 100644 tools/testing/selftests/bpf/bpf_mptcp_helpers.h
>>  create mode 100644 tools/testing/selftests/bpf/prog_tests/mptcp.c
>>  create mode 100644 tools/testing/selftests/bpf/progs/mptcp_sock.c
>>
>>
>> base-commit: 20b87e7c29dffcfa3f96f2e99daec84fd46cabdb
>> --
>> 2.36.0
>>
>

--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 5/8] selftests: bpf: test bpf_skc_to_mptcp_sock
  2022-05-06 22:26   ` Andrii Nakryiko
@ 2022-05-09  9:00     ` Matthieu Baerts
  2022-05-09 21:00       ` Andrii Nakryiko
  0 siblings, 1 reply; 24+ messages in thread
From: Matthieu Baerts @ 2022-05-09  9:00 UTC (permalink / raw)
  To: Andrii Nakryiko, Mat Martineau
  Cc: Networking, bpf, Geliang Tang, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, mptcp

Hi Andrii,

Thank you for the review!

On 07/05/2022 00:26, Andrii Nakryiko wrote:
> On Mon, May 2, 2022 at 2:12 PM Mat Martineau
> <mathew.j.martineau@linux.intel.com> wrote:

(...)

>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 359afc617b92..d48d3cb6abbc 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/bpf_mptcp_helpers.h
>>  F:     tools/testing/selftests/bpf/*/*mptcp*.c
>>  F:     tools/testing/selftests/net/mptcp/
>>
>> diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
>> new file mode 100644
>> index 000000000000..18da4cc65e89
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
>> @@ -0,0 +1,14 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright (c) 2022, SUSE. */
>> +
>> +#ifndef __BPF_MPTCP_HELPERS_H
>> +#define __BPF_MPTCP_HELPERS_H
>> +
>> +#include "bpf_tcp_helpers.h"
>> +
>> +struct mptcp_sock {
>> +       struct inet_connection_sock     sk;
>> +
>> +} __attribute__((preserve_access_index));
> 
> why can't all this live in bpf_tcp_helpers.h? why do we need extra header?

The main reason is related to the maintenance: to have MPTCP ML being
cc'd for all patches modifying this file.

Do you prefer if all these specific MPTCP structures and macros and
mixed with TCP ones?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 5/8] selftests: bpf: test bpf_skc_to_mptcp_sock
  2022-05-09  9:00     ` Matthieu Baerts
@ 2022-05-09 21:00       ` Andrii Nakryiko
  2022-05-10 13:48         ` Matthieu Baerts
  0 siblings, 1 reply; 24+ messages in thread
From: Andrii Nakryiko @ 2022-05-09 21:00 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: Mat Martineau, Networking, bpf, Geliang Tang, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, mptcp

On Mon, May 9, 2022 at 2:00 AM Matthieu Baerts
<matthieu.baerts@tessares.net> wrote:
>
> Hi Andrii,
>
> Thank you for the review!
>
> On 07/05/2022 00:26, Andrii Nakryiko wrote:
> > On Mon, May 2, 2022 at 2:12 PM Mat Martineau
> > <mathew.j.martineau@linux.intel.com> wrote:
>
> (...)
>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 359afc617b92..d48d3cb6abbc 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/bpf_mptcp_helpers.h
> >>  F:     tools/testing/selftests/bpf/*/*mptcp*.c
> >>  F:     tools/testing/selftests/net/mptcp/
> >>
> >> diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> >> new file mode 100644
> >> index 000000000000..18da4cc65e89
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> >> @@ -0,0 +1,14 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +/* Copyright (c) 2022, SUSE. */
> >> +
> >> +#ifndef __BPF_MPTCP_HELPERS_H
> >> +#define __BPF_MPTCP_HELPERS_H
> >> +
> >> +#include "bpf_tcp_helpers.h"
> >> +
> >> +struct mptcp_sock {
> >> +       struct inet_connection_sock     sk;
> >> +
> >> +} __attribute__((preserve_access_index));
> >
> > why can't all this live in bpf_tcp_helpers.h? why do we need extra header?
>
> The main reason is related to the maintenance: to have MPTCP ML being
> cc'd for all patches modifying this file.
>
> Do you prefer if all these specific MPTCP structures and macros and
> mixed with TCP ones?
>

These definitions don't even have to be 1:1 w/ whatever is kernel
defining in terms of having all the fields, or their order, etc. So I
think it won't require active maintenance and thus can be merged into
bpf_tcp_helpers.h to keep it in one place.


> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 5/8] selftests: bpf: test bpf_skc_to_mptcp_sock
  2022-05-09 21:00       ` Andrii Nakryiko
@ 2022-05-10 13:48         ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2022-05-10 13:48 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Mat Martineau, Networking, bpf, Geliang Tang, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, mptcp

Hi Andrii,

On 09/05/2022 23:00, Andrii Nakryiko wrote:
> On Mon, May 9, 2022 at 2:00 AM Matthieu Baerts
> <matthieu.baerts@tessares.net> wrote:
>>
>> Hi Andrii,
>>
>> Thank you for the review!
>>
>> On 07/05/2022 00:26, Andrii Nakryiko wrote:
>>> On Mon, May 2, 2022 at 2:12 PM Mat Martineau
>>> <mathew.j.martineau@linux.intel.com> wrote:
>>
>> (...)
>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 359afc617b92..d48d3cb6abbc 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/bpf_mptcp_helpers.h
>>>>  F:     tools/testing/selftests/bpf/*/*mptcp*.c
>>>>  F:     tools/testing/selftests/net/mptcp/
>>>>
>>>> diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
>>>> new file mode 100644
>>>> index 000000000000..18da4cc65e89
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
>>>> @@ -0,0 +1,14 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +/* Copyright (c) 2022, SUSE. */
>>>> +
>>>> +#ifndef __BPF_MPTCP_HELPERS_H
>>>> +#define __BPF_MPTCP_HELPERS_H
>>>> +
>>>> +#include "bpf_tcp_helpers.h"
>>>> +
>>>> +struct mptcp_sock {
>>>> +       struct inet_connection_sock     sk;
>>>> +
>>>> +} __attribute__((preserve_access_index));
>>>
>>> why can't all this live in bpf_tcp_helpers.h? why do we need extra header?
>>
>> The main reason is related to the maintenance: to have MPTCP ML being
>> cc'd for all patches modifying this file.
>>
>> Do you prefer if all these specific MPTCP structures and macros and
>> mixed with TCP ones?
>>
> 
> These definitions don't even have to be 1:1 w/ whatever is kernel
> defining in terms of having all the fields, or their order, etc. So I
> think it won't require active maintenance and thus can be merged into
> bpf_tcp_helpers.h to keep it in one place.

Thank you for your reply!

New structures and macros[1] are going to be added later but I see your
point: there is nothing requiring an active maintenance. We can move
them all to bpf_tcp_helpers.h.

[1]
https://github.com/multipath-tcp/mptcp_net-next/blob/export/20220510T054929/tools/testing/selftests/bpf/bpf_mptcp_helpers.h

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 6/8] selftests: bpf: verify token of struct mptcp_sock
  2022-05-02 21:12 ` [PATCH bpf-next v3 6/8] selftests: bpf: verify token of struct mptcp_sock Mat Martineau
  2022-05-02 22:14   ` Mat Martineau
@ 2022-05-10 21:59   ` Mat Martineau
  2022-05-10 23:58     ` Andrii Nakryiko
  1 sibling, 1 reply; 24+ messages in thread
From: Mat Martineau @ 2022-05-10 21:59 UTC (permalink / raw)
  To: netdev, bpf, Geliang Tang; +Cc: ast, daniel, andrii, mptcp, Matthieu Baerts

On Mon, 2 May 2022, Mat Martineau 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().
>
> 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>
> ---
> .../testing/selftests/bpf/bpf_mptcp_helpers.h |  1 +
> .../testing/selftests/bpf/prog_tests/mptcp.c  | 66 +++++++++++++++++++
> .../testing/selftests/bpf/progs/mptcp_sock.c  |  5 ++
> 3 files changed, 72 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> index 18da4cc65e89..87e15810997d 100644
> --- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> @@ -9,6 +9,7 @@
> 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 4b40bbdaf91f..c5d96ba81e04 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -8,8 +8,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,58 @@ 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 (CHECK_FAIL(fd < 0)) {
> +		log_err("Failed to open %s", monitor_log_path);
> +		return token;
> +	}
> +
> +	len = read(fd, buf, sizeof(buf));
> +	if (CHECK_FAIL(len < 0)) {
> +		log_err("Failed to read %s", 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 = 0, cfd = client_fd;
> 	struct mptcp_storage val;
> +	__u32 token;
> +
> +	token = get_msk_token();
> +	if (token <= 0) {
> +		log_err("Unexpected token %x", token);
> +		return -1;
> +	}
>
> 	if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) {
> 		perror("Failed to read socket storage");
> @@ -59,6 +109,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;
> }
>
> @@ -124,6 +180,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");
> @@ -141,6 +198,13 @@ void test_base(void)
>
> with_mptcp:
> 	/* with MPTCP */

Geliang, could you add a check here that skips this test (instead of 
failing) if the 'ip mptcp monitor' command is not supported?

Checking the exit status of "ip mptcp help 2>&1 | grep monitor" should 
work.

Thanks,

Mat

> +	if (CHECK_FAIL(!mkdtemp(tmp_dir)))
> +		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 (CHECK_FAIL(system(cmd)))
> +		goto close_cgroup_fd;
> 	server_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
> 	if (CHECK_FAIL(server_fd < 0))
> 		goto close_cgroup_fd;
> @@ -148,6 +212,8 @@ void test_base(void)
> 	CHECK_FAIL(run_test(cgroup_fd, server_fd, true));
>
> 	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 7b6a25e37de8..c58c191d8416 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 {
> @@ -46,6 +47,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;
> @@ -58,6 +61,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 = tcp_sk->is_mptcp;
> -- 
> 2.36.0
>
>

--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 6/8] selftests: bpf: verify token of struct mptcp_sock
  2022-05-10 21:59   ` Mat Martineau
@ 2022-05-10 23:58     ` Andrii Nakryiko
  0 siblings, 0 replies; 24+ messages in thread
From: Andrii Nakryiko @ 2022-05-10 23:58 UTC (permalink / raw)
  To: Mat Martineau
  Cc: Networking, bpf, Geliang Tang, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, mptcp, Matthieu Baerts

On Tue, May 10, 2022 at 2:59 PM Mat Martineau
<mathew.j.martineau@linux.intel.com> wrote:
>
> On Mon, 2 May 2022, Mat Martineau 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().
> >
> > 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>
> > ---
> > .../testing/selftests/bpf/bpf_mptcp_helpers.h |  1 +
> > .../testing/selftests/bpf/prog_tests/mptcp.c  | 66 +++++++++++++++++++
> > .../testing/selftests/bpf/progs/mptcp_sock.c  |  5 ++
> > 3 files changed, 72 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> > index 18da4cc65e89..87e15810997d 100644
> > --- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> > +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h
> > @@ -9,6 +9,7 @@
> > 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 4b40bbdaf91f..c5d96ba81e04 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > @@ -8,8 +8,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,58 @@ 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 (CHECK_FAIL(fd < 0)) {
> > +             log_err("Failed to open %s", monitor_log_path);
> > +             return token;
> > +     }
> > +
> > +     len = read(fd, buf, sizeof(buf));
> > +     if (CHECK_FAIL(len < 0)) {
> > +             log_err("Failed to read %s", 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 = 0, cfd = client_fd;
> >       struct mptcp_storage val;
> > +     __u32 token;
> > +
> > +     token = get_msk_token();
> > +     if (token <= 0) {
> > +             log_err("Unexpected token %x", token);
> > +             return -1;
> > +     }
> >
> >       if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) {
> >               perror("Failed to read socket storage");
> > @@ -59,6 +109,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;
> > }
> >
> > @@ -124,6 +180,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");
> > @@ -141,6 +198,13 @@ void test_base(void)
> >
> > with_mptcp:
> >       /* with MPTCP */
>
> Geliang, could you add a check here that skips this test (instead of
> failing) if the 'ip mptcp monitor' command is not supported?
>
> Checking the exit status of "ip mptcp help 2>&1 | grep monitor" should
> work.
>

Ilya actually already generated updated image, and after [0] it should
be used in CI runs. But we'll know for sure with your next MPTCP
submission.

  [0] https://github.com/libbpf/ci/pull/16

> Thanks,
>
> Mat
>
> > +     if (CHECK_FAIL(!mkdtemp(tmp_dir)))
> > +             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 (CHECK_FAIL(system(cmd)))
> > +             goto close_cgroup_fd;
> >       server_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
> >       if (CHECK_FAIL(server_fd < 0))
> >               goto close_cgroup_fd;
> > @@ -148,6 +212,8 @@ void test_base(void)
> >       CHECK_FAIL(run_test(cgroup_fd, server_fd, true));
> >
> >       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 7b6a25e37de8..c58c191d8416 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 {
> > @@ -46,6 +47,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;
> > @@ -58,6 +61,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 = tcp_sk->is_mptcp;
> > --
> > 2.36.0
> >
> >
>
> --
> Mat Martineau
> Intel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 1/8] bpf: expose is_mptcp flag to bpf_tcp_sock
  2022-05-02 21:12 ` [PATCH bpf-next v3 1/8] bpf: expose is_mptcp flag to bpf_tcp_sock Mat Martineau
@ 2022-05-11  0:48   ` Martin KaFai Lau
  2022-05-11  5:02     ` Andrii Nakryiko
  0 siblings, 1 reply; 24+ messages in thread
From: Martin KaFai Lau @ 2022-05-11  0:48 UTC (permalink / raw)
  To: Mat Martineau, andrii
  Cc: netdev, bpf, Nicolas Rybowski, ast, daniel, mptcp, Matthieu Baerts

On Mon, May 02, 2022 at 02:12:27PM -0700, Mat Martineau wrote:
> From: Nicolas Rybowski <nicolas.rybowski@tessares.net>
> 
> is_mptcp is a field from struct tcp_sock used to indicate that the
> current tcp_sock is part of the MPTCP protocol.
> 
> In this protocol, a first socket (mptcp_sock) is created with
> sk_protocol set to IPPROTO_MPTCP (=262) for control purpose but it
> isn't directly on the wire. This is the role of the subflow (kernel)
> sockets which are classical tcp_sock with sk_protocol set to
> IPPROTO_TCP. The only way to differentiate such sockets from plain TCP
> sockets is the is_mptcp field from tcp_sock.
> 
> Such an exposure in BPF is thus required to be able to differentiate
> plain TCP sockets from MPTCP subflow sockets in BPF_PROG_TYPE_SOCK_OPS
> programs.
> 
> The choice has been made to silently pass the case when CONFIG_MPTCP is
> unset by defaulting is_mptcp to 0 in order to make BPF independent of
> the MPTCP configuration. Another solution is to make the verifier fail
> in 'bpf_tcp_sock_is_valid_ctx_access' but this will add an additional
> '#ifdef CONFIG_MPTCP' in the BPF code and a same injected BPF program
> will not run if MPTCP is not set.
There is already bpf_skc_to_tcp_sock() and its returned tcp_sock pointer
can access all fields of the "struct tcp_sock" without extending
the bpf_tcp_sock.

iiuc, I believe the needs to extend bpf_tcp_sock here is to make the
same bpf sockops prog works for kernel with and without CONFIG_MPTCP
because tp->is_mptcp is not always available:

struct tcp_sock {
	/* ... */

#if IS_ENABLED(CONFIG_MPTCP)
	bool    is_mptcp;
#endif
};

Andrii, do you think bpf_core_field_exists() can be used in
the bpf prog to test if is_mptcp is available in the running kernel
such that the same bpf prog can be used in kernel with and without
CONFIG_MPTCP?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 1/8] bpf: expose is_mptcp flag to bpf_tcp_sock
  2022-05-11  0:48   ` Martin KaFai Lau
@ 2022-05-11  5:02     ` Andrii Nakryiko
  2022-05-11  6:10       ` Geliang Tang
  0 siblings, 1 reply; 24+ messages in thread
From: Andrii Nakryiko @ 2022-05-11  5:02 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Mat Martineau, Andrii Nakryiko, Networking, bpf,
	Nicolas Rybowski, Alexei Starovoitov, Daniel Borkmann, mptcp,
	Matthieu Baerts

On Tue, May 10, 2022 at 5:48 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Mon, May 02, 2022 at 02:12:27PM -0700, Mat Martineau wrote:
> > From: Nicolas Rybowski <nicolas.rybowski@tessares.net>
> >
> > is_mptcp is a field from struct tcp_sock used to indicate that the
> > current tcp_sock is part of the MPTCP protocol.
> >
> > In this protocol, a first socket (mptcp_sock) is created with
> > sk_protocol set to IPPROTO_MPTCP (=262) for control purpose but it
> > isn't directly on the wire. This is the role of the subflow (kernel)
> > sockets which are classical tcp_sock with sk_protocol set to
> > IPPROTO_TCP. The only way to differentiate such sockets from plain TCP
> > sockets is the is_mptcp field from tcp_sock.
> >
> > Such an exposure in BPF is thus required to be able to differentiate
> > plain TCP sockets from MPTCP subflow sockets in BPF_PROG_TYPE_SOCK_OPS
> > programs.
> >
> > The choice has been made to silently pass the case when CONFIG_MPTCP is
> > unset by defaulting is_mptcp to 0 in order to make BPF independent of
> > the MPTCP configuration. Another solution is to make the verifier fail
> > in 'bpf_tcp_sock_is_valid_ctx_access' but this will add an additional
> > '#ifdef CONFIG_MPTCP' in the BPF code and a same injected BPF program
> > will not run if MPTCP is not set.
> There is already bpf_skc_to_tcp_sock() and its returned tcp_sock pointer
> can access all fields of the "struct tcp_sock" without extending
> the bpf_tcp_sock.
>
> iiuc, I believe the needs to extend bpf_tcp_sock here is to make the
> same bpf sockops prog works for kernel with and without CONFIG_MPTCP
> because tp->is_mptcp is not always available:
>
> struct tcp_sock {
>         /* ... */
>
> #if IS_ENABLED(CONFIG_MPTCP)
>         bool    is_mptcp;
> #endif
> };
>
> Andrii, do you think bpf_core_field_exists() can be used in
> the bpf prog to test if is_mptcp is available in the running kernel
> such that the same bpf prog can be used in kernel with and without
> CONFIG_MPTCP?

yep, absolutely:

bool is_mptcp = bpf_core_field_exists(struct tcp_sock, is_mptcp) ?
sock->is_mptcp : false;

One can also directly check if CONFIG_MPTCP is set with the following
in BPF-side code:

extern bool CONFIG_MPTCP __kconfig;

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH bpf-next v3 1/8] bpf: expose is_mptcp flag to bpf_tcp_sock
  2022-05-11  5:02     ` Andrii Nakryiko
@ 2022-05-11  6:10       ` Geliang Tang
  0 siblings, 0 replies; 24+ messages in thread
From: Geliang Tang @ 2022-05-11  6:10 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Martin KaFai Lau, Mat Martineau, Andrii Nakryiko, Networking,
	bpf, Nicolas Rybowski, Alexei Starovoitov, Daniel Borkmann,
	MPTCP Upstream, Matthieu Baerts

Andrii Nakryiko <andrii.nakryiko@gmail.com> 于2022年5月11日周三 13:02写道:
>
> On Tue, May 10, 2022 at 5:48 PM Martin KaFai Lau <kafai@fb.com> wrote:
> >
> > On Mon, May 02, 2022 at 02:12:27PM -0700, Mat Martineau wrote:
> > > From: Nicolas Rybowski <nicolas.rybowski@tessares.net>
> > >
> > > is_mptcp is a field from struct tcp_sock used to indicate that the
> > > current tcp_sock is part of the MPTCP protocol.
> > >
> > > In this protocol, a first socket (mptcp_sock) is created with
> > > sk_protocol set to IPPROTO_MPTCP (=262) for control purpose but it
> > > isn't directly on the wire. This is the role of the subflow (kernel)
> > > sockets which are classical tcp_sock with sk_protocol set to
> > > IPPROTO_TCP. The only way to differentiate such sockets from plain TCP
> > > sockets is the is_mptcp field from tcp_sock.
> > >
> > > Such an exposure in BPF is thus required to be able to differentiate
> > > plain TCP sockets from MPTCP subflow sockets in BPF_PROG_TYPE_SOCK_OPS
> > > programs.
> > >
> > > The choice has been made to silently pass the case when CONFIG_MPTCP is
> > > unset by defaulting is_mptcp to 0 in order to make BPF independent of
> > > the MPTCP configuration. Another solution is to make the verifier fail
> > > in 'bpf_tcp_sock_is_valid_ctx_access' but this will add an additional
> > > '#ifdef CONFIG_MPTCP' in the BPF code and a same injected BPF program
> > > will not run if MPTCP is not set.
> > There is already bpf_skc_to_tcp_sock() and its returned tcp_sock pointer
> > can access all fields of the "struct tcp_sock" without extending
> > the bpf_tcp_sock.
> >
> > iiuc, I believe the needs to extend bpf_tcp_sock here is to make the
> > same bpf sockops prog works for kernel with and without CONFIG_MPTCP
> > because tp->is_mptcp is not always available:
> >
> > struct tcp_sock {
> >         /* ... */
> >
> > #if IS_ENABLED(CONFIG_MPTCP)
> >         bool    is_mptcp;
> > #endif
> > };
> >
> > Andrii, do you think bpf_core_field_exists() can be used in
> > the bpf prog to test if is_mptcp is available in the running kernel
> > such that the same bpf prog can be used in kernel with and without
> > CONFIG_MPTCP?
>
> yep, absolutely:
>
> bool is_mptcp = bpf_core_field_exists(struct tcp_sock, is_mptcp) ?
> sock->is_mptcp : false;
>
> One can also directly check if CONFIG_MPTCP is set with the following
> in BPF-side code:
>
> extern bool CONFIG_MPTCP __kconfig;

Thanks Martin & Andrii, will update this in v4.

-Geliang
SUSE

>

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2022-05-11  6:10 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-02 21:12 [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Mat Martineau
2022-05-02 21:12 ` [PATCH bpf-next v3 1/8] bpf: expose is_mptcp flag to bpf_tcp_sock Mat Martineau
2022-05-11  0:48   ` Martin KaFai Lau
2022-05-11  5:02     ` Andrii Nakryiko
2022-05-11  6:10       ` Geliang Tang
2022-05-02 21:12 ` [PATCH bpf-next v3 2/8] bpf: add bpf_skc_to_mptcp_sock_proto Mat Martineau
2022-05-02 21:12 ` [PATCH bpf-next v3 3/8] selftests: bpf: Enable CONFIG_IKCONFIG_PROC in config Mat Martineau
2022-05-06 22:25   ` Andrii Nakryiko
2022-05-02 21:12 ` [PATCH bpf-next v3 4/8] selftests: bpf: add MPTCP test base Mat Martineau
2022-05-06 22:24   ` Andrii Nakryiko
2022-05-02 21:12 ` [PATCH bpf-next v3 5/8] selftests: bpf: test bpf_skc_to_mptcp_sock Mat Martineau
2022-05-06 22:26   ` Andrii Nakryiko
2022-05-09  9:00     ` Matthieu Baerts
2022-05-09 21:00       ` Andrii Nakryiko
2022-05-10 13:48         ` Matthieu Baerts
2022-05-02 21:12 ` [PATCH bpf-next v3 6/8] selftests: bpf: verify token of struct mptcp_sock Mat Martineau
2022-05-02 22:14   ` Mat Martineau
2022-05-06 22:27     ` Andrii Nakryiko
2022-05-10 21:59   ` Mat Martineau
2022-05-10 23:58     ` Andrii Nakryiko
2022-05-02 21:12 ` [PATCH bpf-next v3 7/8] selftests: bpf: verify ca_name " Mat Martineau
2022-05-02 21:12 ` [PATCH bpf-next v3 8/8] selftests: bpf: verify first " Mat Martineau
2022-05-06 22:28 ` [PATCH bpf-next v3 0/8] bpf: mptcp: Support for mptcp_sock and is_mptcp Andrii Nakryiko
2022-05-06 23:06   ` Mat Martineau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).