[mptcp-next v2 0/7] Add 'dump_subflow' test in selftests

[mptcp-next v2 0/7] Add 'dump_subflow' test in selftests

[Gang Yan]

The 'subflow_get_info_size' is called only in 'inet_sk_attr_size', and
the only calltrace is that 'tcp_diag_dump_one'->'inet_diag_dump_one_icsk'
->'inet_sk_attr_size'.

So It's necessary to add 'get_subflow_info' in mptcp_diag.c to cover this
function. Given a specific TCP connection, it is possible to judge if
it is a MPTCP subflow by 'INET_ULP_INFO_MPTCP' and print 'subflow_info'.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/524

------
Changelog
  v2:
    - Fix the CI compiling error.


Gang Yan (7):
  selftests: mptcp: add struct params in mptcp_diag
  selftests: mptcp: refactor send_query parameters for code clarity
  selftests: mptcp: refine the 'iovlen' handling logic
  selftests: mptcp: refactor NLMSG handling with 'proto'
  selftests: mptcp: add a helper to get subflow_info
  selftests: mptcp: add a helper to print subflow_info
  selftests: mptcp: add chk_sublfow in diag.sh

 tools/testing/selftests/net/mptcp/diag.sh     |  55 +++++
 .../testing/selftests/net/mptcp/mptcp_diag.c  | 223 +++++++++++++++---
 2 files changed, 248 insertions(+), 30 deletions(-)

-- 
2.25.1

[mptcp-next v2 1/7] selftests: mptcp: add struct params in mptcp_diag

[Gang Yan]

This patch adds a struct named 'params' to save 'target_token' and other
future parameters. This structure facilitates future function expansions.

Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_diag.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
index 37d5015ad08c..ea7cb1128044 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
@@ -19,6 +19,10 @@
 #define IPPROTO_MPTCP 262
 #endif
 
+struct params {
+	__u32 target_token;
+};
+
 struct mptcp_info {
 	__u8	mptcpi_subflows;
 	__u8	mptcpi_add_addr_signal;
@@ -237,7 +241,7 @@ static void get_mptcpinfo(__u32 token)
 	close(fd);
 }
 
-static void parse_opts(int argc, char **argv, __u32 *target_token)
+static void parse_opts(int argc, char **argv, struct params *p)
 {
 	int c;
 
@@ -250,7 +254,7 @@ static void parse_opts(int argc, char **argv, __u32 *target_token)
 			die_usage(0);
 			break;
 		case 't':
-			sscanf(optarg, "%x", target_token);
+			sscanf(optarg, "%x", &p->target_token);
 			break;
 		default:
 			die_usage(1);
@@ -261,10 +265,12 @@ static void parse_opts(int argc, char **argv, __u32 *target_token)
 
 int main(int argc, char *argv[])
 {
-	__u32 target_token;
+	struct params p = { 0 };
+
+	parse_opts(argc, argv, &p);
 
-	parse_opts(argc, argv, &target_token);
-	get_mptcpinfo(target_token);
+	if (p.target_token)
+		get_mptcpinfo(p.target_token);
 
 	return 0;
 }
-- 
2.25.1

[mptcp-next v2 2/7] selftests: mptcp: refactor send_query parameters for code clarity

[Gang Yan]

This patch use 'inet_diag_req_v2' instead of 'token' as parameters of
send_query, and construct the req in 'get_mptcpinfo'. This modification
can enhance the enhancing clarity of the code, and prepare for the
dump_subflow_info.

Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 .../testing/selftests/net/mptcp/mptcp_diag.c  | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
index ea7cb1128044..8a5b7745c58c 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
@@ -62,7 +62,7 @@ static void die_usage(int r)
 	exit(r);
 }
 
-static void send_query(int fd, __u32 token)
+static void send_query(int fd, struct inet_diag_req_v2 *r)
 {
 	struct sockaddr_nl nladdr = {
 		.nl_family = AF_NETLINK
@@ -76,19 +76,13 @@ static void send_query(int fd, __u32 token)
 			.nlmsg_type = SOCK_DIAG_BY_FAMILY,
 			.nlmsg_flags = NLM_F_REQUEST
 		},
-		.r = {
-			.sdiag_family = AF_INET,
-			/* Real proto is set via INET_DIAG_REQ_PROTOCOL */
-			.sdiag_protocol = IPPROTO_TCP,
-			.id.idiag_cookie[0] = token,
-		}
+		.r = *r
 	};
 	struct rtattr rta_proto;
 	struct iovec iov[6];
 	int iovlen = 1;
 	__u32 proto;
 
-	req.r.idiag_ext |= (1 << (INET_DIAG_INFO - 1));
 	proto = IPPROTO_MPTCP;
 	rta_proto.rta_type = INET_DIAG_REQ_PROTOCOL;
 	rta_proto.rta_len = RTA_LENGTH(sizeof(proto));
@@ -229,13 +223,20 @@ static void recv_nlmsg(int fd)
 
 static void get_mptcpinfo(__u32 token)
 {
+	struct inet_diag_req_v2 r = {
+		.sdiag_family           = AF_INET,
+		/* Real proto is set via INET_DIAG_REQ_PROTOCOL */
+		.sdiag_protocol         = IPPROTO_TCP,
+		.id.idiag_cookie[0]     = token,
+	};
 	int fd;
 
 	fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_SOCK_DIAG);
 	if (fd < 0)
 		die_perror("Netlink socket");
 
-	send_query(fd, token);
+	r.idiag_ext |= (1 << (INET_DIAG_INFO - 1));
+	send_query(fd, &r);
 	recv_nlmsg(fd);
 
 	close(fd);
-- 
2.25.1

[mptcp-next v2 3/7] selftests: mptcp: refine the 'iovlen' handling logic

[Gang Yan]

In the original version, the handling logic for the 'iovlen' appeared
somewhat redundant. Therefore, 'iovlen++' is used to simpify this
portion of the code and improve its readaility.

Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_diag.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
index 8a5b7745c58c..2bf13bce59a5 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
@@ -91,10 +91,9 @@ static void send_query(int fd, struct inet_diag_req_v2 *r)
 		.iov_base = &req,
 		.iov_len = sizeof(req)
 	};
-	iov[iovlen] = (struct iovec){ &rta_proto, sizeof(rta_proto)};
-	iov[iovlen + 1] = (struct iovec){ &proto, sizeof(proto)};
+	iov[iovlen++] = (struct iovec){ &rta_proto, sizeof(rta_proto)};
+	iov[iovlen++] = (struct iovec){ &proto, sizeof(proto)};
 	req.nlh.nlmsg_len += RTA_LENGTH(sizeof(proto));
-	iovlen += 2;
 	struct msghdr msg = {
 		.msg_name = &nladdr,
 		.msg_namelen = sizeof(nladdr),
-- 
2.25.1

[mptcp-next v2 4/7] selftests: mptcp: refactor NLMSG handling with 'proto'

[Gang Yan]

This patch introduces the '__u32 proto' variable to the 'send_query' and
'recv_nlmsg' functions for further extending function.

In the 'send_query' function, the inclusion of this variable makes the
structure clearer and more readable.

In the 'recv_nlmsg' function, the '__u32 proto' variable ensures that
the 'diag_info' field remains unmodified when processing IPPROTO_TCP data,
thereby preventing unintended transformation into 'mptcp_info' format.

Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 .../testing/selftests/net/mptcp/mptcp_diag.c  | 33 ++++++++++---------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
index 2bf13bce59a5..2116b40bc927 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
@@ -62,7 +62,7 @@ static void die_usage(int r)
 	exit(r);
 }
 
-static void send_query(int fd, struct inet_diag_req_v2 *r)
+static void send_query(int fd, struct inet_diag_req_v2 *r, __u32 proto)
 {
 	struct sockaddr_nl nladdr = {
 		.nl_family = AF_NETLINK
@@ -81,19 +81,21 @@ static void send_query(int fd, struct inet_diag_req_v2 *r)
 	struct rtattr rta_proto;
 	struct iovec iov[6];
 	int iovlen = 1;
-	__u32 proto;
-
-	proto = IPPROTO_MPTCP;
-	rta_proto.rta_type = INET_DIAG_REQ_PROTOCOL;
-	rta_proto.rta_len = RTA_LENGTH(sizeof(proto));
 
 	iov[0] = (struct iovec) {
 		.iov_base = &req,
 		.iov_len = sizeof(req)
 	};
-	iov[iovlen++] = (struct iovec){ &rta_proto, sizeof(rta_proto)};
-	iov[iovlen++] = (struct iovec){ &proto, sizeof(proto)};
-	req.nlh.nlmsg_len += RTA_LENGTH(sizeof(proto));
+
+	if (proto == IPPROTO_MPTCP) {
+		rta_proto.rta_type = INET_DIAG_REQ_PROTOCOL;
+		rta_proto.rta_len = RTA_LENGTH(sizeof(proto));
+
+		iov[iovlen++] = (struct iovec){ &rta_proto, sizeof(rta_proto)};
+		iov[iovlen++] = (struct iovec){ &proto, sizeof(proto)};
+		req.nlh.nlmsg_len += RTA_LENGTH(sizeof(proto));
+	}
+
 	struct msghdr msg = {
 		.msg_name = &nladdr,
 		.msg_namelen = sizeof(nladdr),
@@ -157,7 +159,7 @@ static void print_info_msg(struct mptcp_info *info)
 	printf("bytes_acked:      %llu\n", info->mptcpi_bytes_acked);
 }
 
-static void parse_nlmsg(struct nlmsghdr *nlh)
+static void parse_nlmsg(struct nlmsghdr *nlh, __u32 proto)
 {
 	struct inet_diag_msg *r = NLMSG_DATA(nlh);
 	struct rtattr *tb[INET_DIAG_MAX + 1];
@@ -166,7 +168,7 @@ static void parse_nlmsg(struct nlmsghdr *nlh)
 			   nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)),
 			   NLA_F_NESTED);
 
-	if (tb[INET_DIAG_INFO]) {
+	if (proto == IPPROTO_MPTCP && tb[INET_DIAG_INFO]) {
 		int len = RTA_PAYLOAD(tb[INET_DIAG_INFO]);
 		struct mptcp_info *info;
 
@@ -182,7 +184,7 @@ static void parse_nlmsg(struct nlmsghdr *nlh)
 	}
 }
 
-static void recv_nlmsg(int fd)
+static void recv_nlmsg(int fd, __u32 proto)
 {
 	char rcv_buff[8192];
 	struct nlmsghdr *nlh = (struct nlmsghdr *)rcv_buff;
@@ -215,7 +217,7 @@ static void recv_nlmsg(int fd)
 			       -(err->error), strerror(-(err->error)));
 			break;
 		}
-		parse_nlmsg(nlh);
+		parse_nlmsg(nlh, proto);
 		nlh = NLMSG_NEXT(nlh, len);
 	}
 }
@@ -228,6 +230,7 @@ static void get_mptcpinfo(__u32 token)
 		.sdiag_protocol         = IPPROTO_TCP,
 		.id.idiag_cookie[0]     = token,
 	};
+	__u32 proto = IPPROTO_MPTCP;
 	int fd;
 
 	fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_SOCK_DIAG);
@@ -235,8 +238,8 @@ static void get_mptcpinfo(__u32 token)
 		die_perror("Netlink socket");
 
 	r.idiag_ext |= (1 << (INET_DIAG_INFO - 1));
-	send_query(fd, &r);
-	recv_nlmsg(fd);
+	send_query(fd, &r, proto);
+	recv_nlmsg(fd, proto);
 
 	close(fd);
 }
-- 
2.25.1

[mptcp-next v2 5/7] selftests: mptcp: add a helper to get subflow_info

[Gang Yan]

This patch adds 'get_subflow_info' in 'mptcp_diag', which can check whether
a TCP connection is an MPTCP subflow based on the "INET_ULP_INFO_MPTCP"
with tcp_diag method.

Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 .../testing/selftests/net/mptcp/mptcp_diag.c  | 86 ++++++++++++++++++-
 1 file changed, 84 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
index 2116b40bc927..28b1704d0c78 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
@@ -8,6 +8,7 @@
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <linux/tcp.h>
+#include <arpa/inet.h>
 
 #include <unistd.h>
 #include <stdlib.h>
@@ -19,8 +20,13 @@
 #define IPPROTO_MPTCP 262
 #endif
 
+#define parse_rtattr_nested(tb, max, rta) \
+	(parse_rtattr_flags((tb), (max), RTA_DATA(rta), RTA_PAYLOAD(rta), \
+			    NLA_F_NESTED))
+
 struct params {
 	__u32 target_token;
+	char subflow_addr[1024];
 };
 
 struct mptcp_info {
@@ -50,6 +56,24 @@ struct mptcp_info {
 	__u32	mptcpi_last_ack_recv;
 };
 
+enum {
+	MPTCP_SUBFLOW_ATTR_UNSPEC,
+	MPTCP_SUBFLOW_ATTR_TOKEN_REM,
+	MPTCP_SUBFLOW_ATTR_TOKEN_LOC,
+	MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ,
+	MPTCP_SUBFLOW_ATTR_MAP_SEQ,
+	MPTCP_SUBFLOW_ATTR_MAP_SFSEQ,
+	MPTCP_SUBFLOW_ATTR_SSN_OFFSET,
+	MPTCP_SUBFLOW_ATTR_MAP_DATALEN,
+	MPTCP_SUBFLOW_ATTR_FLAGS,
+	MPTCP_SUBFLOW_ATTR_ID_REM,
+	MPTCP_SUBFLOW_ATTR_ID_LOC,
+	MPTCP_SUBFLOW_ATTR_PAD,
+
+	__MPTCP_SUBFLOW_ATTR_MAX
+};
+#define MPTCP_SUBFLOW_ATTR_MAX (__MPTCP_SUBFLOW_ATTR_MAX - 1)
+
 static void die_perror(const char *msg)
 {
 	perror(msg);
@@ -58,7 +82,9 @@ static void die_perror(const char *msg)
 
 static void die_usage(int r)
 {
-	fprintf(stderr, "Usage: mptcp_diag -t\n");
+	fprintf(stderr, "Usage:\n"
+			"mptcp_diag -t <token>\n"
+			"mptcp_diag -s \"<saddr> <sport> <daddr> <dport>\"\n");
 	exit(r);
 }
 
@@ -182,6 +208,21 @@ static void parse_nlmsg(struct nlmsghdr *nlh, __u32 proto)
 		}
 		print_info_msg(info);
 	}
+	if (proto == IPPROTO_TCP && tb[INET_DIAG_ULP_INFO]) {
+		struct rtattr *ulpinfo[INET_ULP_INFO_MAX + 1] = { 0 };
+
+		parse_rtattr_nested(ulpinfo, INET_ULP_INFO_MAX,
+				    tb[INET_DIAG_ULP_INFO]);
+
+		if (ulpinfo[INET_ULP_INFO_MPTCP]) {
+			struct rtattr *sfinfo[MPTCP_SUBFLOW_ATTR_MAX + 1] = { 0 };
+
+			parse_rtattr_nested(sfinfo, MPTCP_SUBFLOW_ATTR_MAX,
+					    ulpinfo[INET_ULP_INFO_MPTCP]);
+		} else {
+			printf("It's a normal TCP!\n");
+		}
+	}
 }
 
 static void recv_nlmsg(int fd, __u32 proto)
@@ -244,6 +285,41 @@ static void get_mptcpinfo(__u32 token)
 	close(fd);
 }
 
+static void get_subflow_info(char *subflow_addr)
+{
+	struct inet_diag_req_v2 r = {
+		.sdiag_family           = AF_INET,
+		.sdiag_protocol         = IPPROTO_TCP,
+	};
+	int sport, dport;
+	char saddr[64];	// Take ipv6 into consideration
+	char daddr[64];
+	int ret;
+	int fd;
+
+	ret = sscanf(subflow_addr, "%s %d %s %d", saddr, &sport, daddr, &dport);
+	if (ret != 4)
+		die_perror("IP PORT Pairs has style problems!");
+
+	printf("%s:%d -> %s:%d\n", saddr, sport, daddr, dport);
+
+	fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_SOCK_DIAG);
+	if (fd < 0)
+		die_perror("Netlink socket");
+
+	r.id.idiag_cookie[0] = INET_DIAG_NOCOOKIE;
+	r.id.idiag_cookie[1] = INET_DIAG_NOCOOKIE;
+	r.id.idiag_sport = htons(sport);
+	r.id.idiag_dport = htons(dport);
+	r.id.idiag_if = 0;
+
+	inet_pton(AF_INET, saddr, &r.id.idiag_src);
+	inet_pton(AF_INET, daddr, &r.id.idiag_dst);
+	r.idiag_ext |= (1 << (INET_DIAG_INFO - 1));
+	send_query(fd, &r, IPPROTO_TCP);
+	recv_nlmsg(fd, IPPROTO_TCP);
+}
+
 static void parse_opts(int argc, char **argv, struct params *p)
 {
 	int c;
@@ -251,7 +327,7 @@ static void parse_opts(int argc, char **argv, struct params *p)
 	if (argc < 2)
 		die_usage(1);
 
-	while ((c = getopt(argc, argv, "ht:")) != -1) {
+	while ((c = getopt(argc, argv, "ht:s:")) != -1) {
 		switch (c) {
 		case 'h':
 			die_usage(0);
@@ -259,6 +335,9 @@ static void parse_opts(int argc, char **argv, struct params *p)
 		case 't':
 			sscanf(optarg, "%x", &p->target_token);
 			break;
+		case 's':
+			snprintf(p->subflow_addr, strlen(optarg) + 1, "%s", optarg);
+			break;
 		default:
 			die_usage(1);
 			break;
@@ -275,6 +354,9 @@ int main(int argc, char *argv[])
 	if (p.target_token)
 		get_mptcpinfo(p.target_token);
 
+	if (strlen(p.subflow_addr) != 0)
+		get_subflow_info(p.subflow_addr);
+
 	return 0;
 }
 
-- 
2.25.1

[mptcp-next v2 6/7] selftests: mptcp: add a helper to print subflow_info

[Gang Yan]

This patch adds 'print_subflow_info' in 'mptcp_diag', which can print
the subflow_filed of an MPTCP subflow. This function is for further
checking the 'subflow_info' through inet_diag method.

Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 .../testing/selftests/net/mptcp/mptcp_diag.c  | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
index 28b1704d0c78..25495c7e1c74 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
@@ -74,6 +74,18 @@ enum {
 };
 #define MPTCP_SUBFLOW_ATTR_MAX (__MPTCP_SUBFLOW_ATTR_MAX - 1)
 
+#define MPTCP_SUBFLOW_FLAG_MCAP_REM		_BITUL(0)
+#define MPTCP_SUBFLOW_FLAG_MCAP_LOC		_BITUL(1)
+#define MPTCP_SUBFLOW_FLAG_JOIN_REM		_BITUL(2)
+#define MPTCP_SUBFLOW_FLAG_JOIN_LOC		_BITUL(3)
+#define MPTCP_SUBFLOW_FLAG_BKUP_REM		_BITUL(4)
+#define MPTCP_SUBFLOW_FLAG_BKUP_LOC		_BITUL(5)
+#define MPTCP_SUBFLOW_FLAG_FULLY_ESTABLISHED	_BITUL(6)
+#define MPTCP_SUBFLOW_FLAG_CONNECTED		_BITUL(7)
+#define MPTCP_SUBFLOW_FLAG_MAPVALID		_BITUL(8)
+
+#define rta_getattr(type, value)		(*(type *)RTA_DATA(value))
+
 static void die_perror(const char *msg)
 {
 	perror(msg);
@@ -185,6 +197,65 @@ static void print_info_msg(struct mptcp_info *info)
 	printf("bytes_acked:      %llu\n", info->mptcpi_bytes_acked);
 }
 
+/*
+ * 'print_subflow_info' is from 'mptcp_subflow_info'
+ * which is a function in 'misc/ss.c' of iproute2.
+ */
+static void print_subflow_info(struct rtattr *tb[])
+{
+	u_int32_t flags = 0;
+
+	printf("It's a mptcp subflow, the subflow info:\n");
+	if (tb[MPTCP_SUBFLOW_ATTR_FLAGS]) {
+		char caps[32 + 1] = { 0 }, *cap = &caps[0];
+
+		flags = rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_FLAGS]);
+
+		if (flags & MPTCP_SUBFLOW_FLAG_MCAP_REM)
+			*cap++ = 'M';
+		if (flags & MPTCP_SUBFLOW_FLAG_MCAP_LOC)
+			*cap++ = 'm';
+		if (flags & MPTCP_SUBFLOW_FLAG_JOIN_REM)
+			*cap++ = 'J';
+		if (flags & MPTCP_SUBFLOW_FLAG_JOIN_LOC)
+			*cap++ = 'j';
+		if (flags & MPTCP_SUBFLOW_FLAG_BKUP_REM)
+			*cap++ = 'B';
+		if (flags & MPTCP_SUBFLOW_FLAG_BKUP_LOC)
+			*cap++ = 'b';
+		if (flags & MPTCP_SUBFLOW_FLAG_FULLY_ESTABLISHED)
+			*cap++ = 'e';
+		if (flags & MPTCP_SUBFLOW_FLAG_CONNECTED)
+			*cap++ = 'c';
+		if (flags & MPTCP_SUBFLOW_FLAG_MAPVALID)
+			*cap++ = 'v';
+
+		if (flags)
+			printf(" flags:%s", caps);
+	}
+	if (tb[MPTCP_SUBFLOW_ATTR_TOKEN_REM] &&
+	    tb[MPTCP_SUBFLOW_ATTR_TOKEN_LOC] &&
+	    tb[MPTCP_SUBFLOW_ATTR_ID_REM] &&
+	    tb[MPTCP_SUBFLOW_ATTR_ID_LOC])
+		printf(" token:%04x(id:%u)/%04x(id:%u)",
+		       rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_TOKEN_REM]),
+		       rta_getattr(__u8, tb[MPTCP_SUBFLOW_ATTR_ID_REM]),
+		       rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_TOKEN_LOC]),
+		       rta_getattr(__u8, tb[MPTCP_SUBFLOW_ATTR_ID_LOC]));
+	if (tb[MPTCP_SUBFLOW_ATTR_MAP_SEQ])
+		printf(" seq:%llu",
+		       rta_getattr(__u64, tb[MPTCP_SUBFLOW_ATTR_MAP_SEQ]));
+	if (tb[MPTCP_SUBFLOW_ATTR_MAP_SFSEQ])
+		printf(" sfseq:%u",
+		       rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_MAP_SFSEQ]));
+	if (tb[MPTCP_SUBFLOW_ATTR_SSN_OFFSET])
+		printf(" ssnoff:%u",
+		       rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_SSN_OFFSET]));
+	if (tb[MPTCP_SUBFLOW_ATTR_MAP_DATALEN])
+		printf(" maplen:%u",
+		       rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_MAP_DATALEN]));
+}
+
 static void parse_nlmsg(struct nlmsghdr *nlh, __u32 proto)
 {
 	struct inet_diag_msg *r = NLMSG_DATA(nlh);
@@ -219,6 +290,7 @@ static void parse_nlmsg(struct nlmsghdr *nlh, __u32 proto)
 
 			parse_rtattr_nested(sfinfo, MPTCP_SUBFLOW_ATTR_MAX,
 					    ulpinfo[INET_ULP_INFO_MPTCP]);
+			print_subflow_info(sfinfo);
 		} else {
 			printf("It's a normal TCP!\n");
 		}
-- 
2.25.1

[mptcp-next v2 7/7] selftests: mptcp: add chk_sublfow in diag.sh

[Gang Yan]

This patch aims to add chk_dump_subflow in diag.sh. The subflow's
info can be obtained through "ss -tin", then use the 'mptcp_diag'
to verify the token in subflow_info.

Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/diag.sh | 55 +++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index e7a75341f0f3..20de007e44cd 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -225,6 +225,60 @@ chk_dump_one()
 	fi
 }
 
+get_endpoint_addr()
+{
+	echo $1 | cut -d ":" -f 1
+}
+
+get_endpoint_port()
+{
+	echo $1 | cut -d ":" -f 2
+}
+
+chk_dump_subflow()
+{
+	local inet_diag_token
+	local subflow_line
+	local remote_addr
+	local remote_port
+	local local_addr
+	local local_port
+	local ss_token
+	local msg
+
+	subflow_line=$(ss -tnN $ns | \
+		       grep -m1 -Eo '[0-9.]+:[0-9].+ +[0-9.]+:[0-9.]+')
+
+	if [ -n "$subflow_line" ]; then
+		read -r local_endpoint remote_endpoint <<< $subflow_line
+
+		local_addr=$(get_endpoint_addr $local_endpoint)
+		local_port=$(get_endpoint_port $local_endpoint)
+
+		remote_addr=$(get_endpoint_addr $remote_endpoint)
+		remote_port=$(get_endpoint_port $remote_endpoint)
+	fi
+
+	ss_token=$(ss -tniN $ns | \
+		   grep -m1 -Eo 'token:[^ ]+')
+
+	inet_diag_token=$(ip netns exec $ns ./mptcp_diag -s \
+			  "$local_addr $local_port $remote_addr $remote_port" | \
+			  grep -Eo 'token:[^ ]+')
+
+	msg="....chk dump_subflow"
+
+	mptcp_lib_print_title "$msg"
+	if [[ $ss_token == $inet_diag_token ]]; then
+		mptcp_lib_pr_ok
+		mptcp_lib_result_pass "${msg}"
+	else
+		mptcp_lib_pr_fail "expected $ss_token but $inet_diag_token"
+		mptcp_lib_result_fail "${msg}"
+		ret=${KSFT_FAIL}
+	fi
+}
+
 msk_info_get_value()
 {
 	local port="${1}"
@@ -316,6 +370,7 @@ chk_msk_fallback_nr 0 "....chk no fallback"
 chk_msk_inuse 2
 chk_msk_cestab 2
 chk_dump_one
+chk_dump_subflow
 flush_pids
 
 chk_msk_inuse 0 "2->0"
-- 
2.25.1

Re: [mptcp-next v2 0/7] Add 'dump_subflow' test in selftests

[MPTCP CI]

Hi Gang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/14489098858

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/1fd8c63c0c7f
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=953894


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [mptcp-next v2 0/7] Add 'dump_subflow' test in selftests

[Matthieu Baerts]

Hi Gang,

On 16/04/2025 10:59, Gang Yan wrote:
> The 'subflow_get_info_size' is called only in 'inet_sk_attr_size', and
> the only calltrace is that 'tcp_diag_dump_one'->'inet_diag_dump_one_icsk'
> ->'inet_sk_attr_size'.
> 
> So It's necessary to add 'get_subflow_info' in mptcp_diag.c to cover this
> function. Given a specific TCP connection, it is possible to judge if
> it is a MPTCP subflow by 'INET_ULP_INFO_MPTCP' and print 'subflow_info'.

Thank you for looking at this. I have few minor comments, please check
my other replies.

> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/524

Can you add this line in the last patch as well please? So the ticket
will be automatically closed when the patches will be applied.


Something else: can you make sure to keep the "PATCH" prefix from the
subjects? I guess you used:

  git send-email --subject-prefix mptcp-next

instead of:

  git send-email --subject-prefix "PATCH mptcp-next"

Do not hesitate to use b4 [1] to send patches, it is helpful not to
forget things when sending patches by email. (Just don't use 'b4 prep
--auto-to-cc' when submitting patches for MPTCP)

[1] https://b4.docs.kernel.org

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [mptcp-next v2 2/7] selftests: mptcp: refactor send_query parameters for code clarity

[Matthieu Baerts]

Hi Gang,

On 16/04/2025 10:59, Gang Yan wrote:
> This patch use 'inet_diag_req_v2' instead of 'token' as parameters of
> send_query, and construct the req in 'get_mptcpinfo'. This modification
> can enhance the enhancing clarity of the code, and prepare for the
> dump_subflow_info.
> 
> Co-developed-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
>  .../testing/selftests/net/mptcp/mptcp_diag.c  | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> index ea7cb1128044..8a5b7745c58c 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> @@ -62,7 +62,7 @@ static void die_usage(int r)
>  	exit(r);
>  }
>  
> -static void send_query(int fd, __u32 token)
> +static void send_query(int fd, struct inet_diag_req_v2 *r)
>  {
>  	struct sockaddr_nl nladdr = {
>  		.nl_family = AF_NETLINK
> @@ -76,19 +76,13 @@ static void send_query(int fd, __u32 token)
>  			.nlmsg_type = SOCK_DIAG_BY_FAMILY,
>  			.nlmsg_flags = NLM_F_REQUEST
>  		},
> -		.r = {
> -			.sdiag_family = AF_INET,
> -			/* Real proto is set via INET_DIAG_REQ_PROTOCOL */
> -			.sdiag_protocol = IPPROTO_TCP,
> -			.id.idiag_cookie[0] = token,
> -		}
> +		.r = *r
>  	};
>  	struct rtattr rta_proto;
>  	struct iovec iov[6];
>  	int iovlen = 1;
>  	__u32 proto;
>  
> -	req.r.idiag_ext |= (1 << (INET_DIAG_INFO - 1));
>  	proto = IPPROTO_MPTCP;
>  	rta_proto.rta_type = INET_DIAG_REQ_PROTOCOL;
>  	rta_proto.rta_len = RTA_LENGTH(sizeof(proto));
> @@ -229,13 +223,20 @@ static void recv_nlmsg(int fd)
>  
>  static void get_mptcpinfo(__u32 token)
>  {
> +	struct inet_diag_req_v2 r = {
> +		.sdiag_family           = AF_INET,
> +		/* Real proto is set via INET_DIAG_REQ_PROTOCOL */
> +		.sdiag_protocol         = IPPROTO_TCP,
> +		.id.idiag_cookie[0]     = token,
> +	};
>  	int fd;
>  
>  	fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_SOCK_DIAG);
>  	if (fd < 0)
>  		die_perror("Netlink socket");
>  
> -	send_query(fd, token);
> +	r.idiag_ext |= (1 << (INET_DIAG_INFO - 1));

Out of curiosity, why can you not set idiag_ext above, when 'r' is declared?

> +	send_query(fd, &r);
>  	recv_nlmsg(fd);
>  
>  	close(fd);

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [mptcp-next v2 3/7] selftests: mptcp: refine the 'iovlen' handling logic

[Matthieu Baerts]

Hi Gang,

On 16/04/2025 10:59, Gang Yan wrote:
> In the original version, the handling logic for the 'iovlen' appeared
> somewhat redundant. Therefore, 'iovlen++' is used to simpify this
> portion of the code and improve its readaility.
> 
> Co-developed-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_diag.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> index 8a5b7745c58c..2bf13bce59a5 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> @@ -91,10 +91,9 @@ static void send_query(int fd, struct inet_diag_req_v2 *r)
>  		.iov_base = &req,
>  		.iov_len = sizeof(req)
>  	};

I guess you could also declare iovlen to 0, then start the increment above:

  iov[iovlen++] = (struct iovec) { (...) };

> -	iov[iovlen] = (struct iovec){ &rta_proto, sizeof(rta_proto)};
> -	iov[iovlen + 1] = (struct iovec){ &proto, sizeof(proto)};
> +	iov[iovlen++] = (struct iovec){ &rta_proto, sizeof(rta_proto)};
> +	iov[iovlen++] = (struct iovec){ &proto, sizeof(proto)};
>  	req.nlh.nlmsg_len += RTA_LENGTH(sizeof(proto));
> -	iovlen += 2;

For such very small and not complex change like that, I think there is
no need to create a dedicate patch. This can be squashed in the next
patch with a note like:

  While at it, increment iovlen directly when an item is added.

>  	struct msghdr msg = {
>  		.msg_name = &nladdr,
>  		.msg_namelen = sizeof(nladdr),

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [mptcp-next v2 5/7] selftests: mptcp: add a helper to get subflow_info

[Matthieu Baerts]

Hello,

On 16/04/2025 10:59, Gang Yan wrote:
> This patch adds 'get_subflow_info' in 'mptcp_diag', which can check whether
> a TCP connection is an MPTCP subflow based on the "INET_ULP_INFO_MPTCP"
> with tcp_diag method.
> 
> Co-developed-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
>  .../testing/selftests/net/mptcp/mptcp_diag.c  | 86 ++++++++++++++++++-
>  1 file changed, 84 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> index 2116b40bc927..28b1704d0c78 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> @@ -8,6 +8,7 @@
>  #include <sys/socket.h>
>  #include <netinet/in.h>
>  #include <linux/tcp.h>
> +#include <arpa/inet.h>
>  
>  #include <unistd.h>
>  #include <stdlib.h>
> @@ -19,8 +20,13 @@
>  #define IPPROTO_MPTCP 262
>  #endif
>  
> +#define parse_rtattr_nested(tb, max, rta) \
> +	(parse_rtattr_flags((tb), (max), RTA_DATA(rta), RTA_PAYLOAD(rta), \
> +			    NLA_F_NESTED))
> +
>  struct params {
>  	__u32 target_token;
> +	char subflow_addr[1024];

detail: maybe add a 's': subflow_addrs.

>  };
>  
>  struct mptcp_info {

(...)

> @@ -244,6 +285,41 @@ static void get_mptcpinfo(__u32 token)
>  	close(fd);
>  }
>  
> +static void get_subflow_info(char *subflow_addr)
> +{
> +	struct inet_diag_req_v2 r = {
> +		.sdiag_family           = AF_INET,
> +		.sdiag_protocol         = IPPROTO_TCP,
> +	};
> +	int sport, dport;
> +	char saddr[64];	// Take ipv6 into consideration

Detail: the comments are usually with the "/* ... */" style. You can
also remove the comment and declare both [sd]addr on the same line I think.

> +	char daddr[64];
> +	int ret;
> +	int fd;
> +
> +	ret = sscanf(subflow_addr, "%s %d %s %d", saddr, &sport, daddr, &dport);
> +	if (ret != 4)
> +		die_perror("IP PORT Pairs has style problems!");
> +
> +	printf("%s:%d -> %s:%d\n", saddr, sport, daddr, dport);
> +
> +	fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_SOCK_DIAG);
> +	if (fd < 0)
> +		die_perror("Netlink socket");
> +
> +	r.id.idiag_cookie[0] = INET_DIAG_NOCOOKIE;
> +	r.id.idiag_cookie[1] = INET_DIAG_NOCOOKIE;

Can you not set it above, when declaring 'r'? Same for 'idiag_ext'.

> +	r.id.idiag_sport = htons(sport);
> +	r.id.idiag_dport = htons(dport);
> +	r.id.idiag_if = 0;

Do you need to explicitly set it to 0? Is it not 0 by default?

> +
> +	inet_pton(AF_INET, saddr, &r.id.idiag_src);
> +	inet_pton(AF_INET, daddr, &r.id.idiag_dst);
> +	r.idiag_ext |= (1 << (INET_DIAG_INFO - 1));
> +	send_query(fd, &r, IPPROTO_TCP);
> +	recv_nlmsg(fd, IPPROTO_TCP);
> +}
> +
>  static void parse_opts(int argc, char **argv, struct params *p)
>  {
>  	int c;

(...)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [mptcp-next v2 6/7] selftests: mptcp: add a helper to print subflow_info

[Matthieu Baerts]

Hi Gang,

On 16/04/2025 10:59, Gang Yan wrote:
> This patch adds 'print_subflow_info' in 'mptcp_diag', which can print
> the subflow_filed of an MPTCP subflow. This function is for further
> checking the 'subflow_info' through inet_diag method.

I think this can be squashed in the previous patch. They don't make
sense alone.

> 
> Co-developed-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
>  .../testing/selftests/net/mptcp/mptcp_diag.c  | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> index 28b1704d0c78..25495c7e1c74 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> @@ -74,6 +74,18 @@ enum {
>  };
>  #define MPTCP_SUBFLOW_ATTR_MAX (__MPTCP_SUBFLOW_ATTR_MAX - 1)
>  
> +#define MPTCP_SUBFLOW_FLAG_MCAP_REM		_BITUL(0)
> +#define MPTCP_SUBFLOW_FLAG_MCAP_LOC		_BITUL(1)
> +#define MPTCP_SUBFLOW_FLAG_JOIN_REM		_BITUL(2)
> +#define MPTCP_SUBFLOW_FLAG_JOIN_LOC		_BITUL(3)
> +#define MPTCP_SUBFLOW_FLAG_BKUP_REM		_BITUL(4)
> +#define MPTCP_SUBFLOW_FLAG_BKUP_LOC		_BITUL(5)
> +#define MPTCP_SUBFLOW_FLAG_FULLY_ESTABLISHED	_BITUL(6)
> +#define MPTCP_SUBFLOW_FLAG_CONNECTED		_BITUL(7)
> +#define MPTCP_SUBFLOW_FLAG_MAPVALID		_BITUL(8)
> +
> +#define rta_getattr(type, value)		(*(type *)RTA_DATA(value))
> +
>  static void die_perror(const char *msg)
>  {
>  	perror(msg);
> @@ -185,6 +197,65 @@ static void print_info_msg(struct mptcp_info *info)
>  	printf("bytes_acked:      %llu\n", info->mptcpi_bytes_acked);
>  }
>  
> +/*
> + * 'print_subflow_info' is from 'mptcp_subflow_info'
> + * which is a function in 'misc/ss.c' of iproute2.
> + */
> +static void print_subflow_info(struct rtattr *tb[])
> +{
> +	u_int32_t flags = 0;
> +
> +	printf("It's a mptcp subflow, the subflow info:\n");
> +	if (tb[MPTCP_SUBFLOW_ATTR_FLAGS]) {
> +		char caps[32 + 1] = { 0 }, *cap = &caps[0];
> +
> +		flags = rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_FLAGS]);
> +
> +		if (flags & MPTCP_SUBFLOW_FLAG_MCAP_REM)
> +			*cap++ = 'M';
> +		if (flags & MPTCP_SUBFLOW_FLAG_MCAP_LOC)
> +			*cap++ = 'm';
> +		if (flags & MPTCP_SUBFLOW_FLAG_JOIN_REM)
> +			*cap++ = 'J';
> +		if (flags & MPTCP_SUBFLOW_FLAG_JOIN_LOC)
> +			*cap++ = 'j';
> +		if (flags & MPTCP_SUBFLOW_FLAG_BKUP_REM)
> +			*cap++ = 'B';
> +		if (flags & MPTCP_SUBFLOW_FLAG_BKUP_LOC)
> +			*cap++ = 'b';
> +		if (flags & MPTCP_SUBFLOW_FLAG_FULLY_ESTABLISHED)
> +			*cap++ = 'e';
> +		if (flags & MPTCP_SUBFLOW_FLAG_CONNECTED)
> +			*cap++ = 'c';
> +		if (flags & MPTCP_SUBFLOW_FLAG_MAPVALID)
> +			*cap++ = 'v';
> +
> +		if (flags)
> +			printf(" flags:%s", caps);
> +	}
> +	if (tb[MPTCP_SUBFLOW_ATTR_TOKEN_REM] &&
> +	    tb[MPTCP_SUBFLOW_ATTR_TOKEN_LOC] &&
> +	    tb[MPTCP_SUBFLOW_ATTR_ID_REM] &&
> +	    tb[MPTCP_SUBFLOW_ATTR_ID_LOC])
> +		printf(" token:%04x(id:%u)/%04x(id:%u)",
> +		       rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_TOKEN_REM]),
> +		       rta_getattr(__u8, tb[MPTCP_SUBFLOW_ATTR_ID_REM]),
> +		       rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_TOKEN_LOC]),
> +		       rta_getattr(__u8, tb[MPTCP_SUBFLOW_ATTR_ID_LOC]));
> +	if (tb[MPTCP_SUBFLOW_ATTR_MAP_SEQ])
> +		printf(" seq:%llu",
> +		       rta_getattr(__u64, tb[MPTCP_SUBFLOW_ATTR_MAP_SEQ]));
> +	if (tb[MPTCP_SUBFLOW_ATTR_MAP_SFSEQ])
> +		printf(" sfseq:%u",
> +		       rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_MAP_SFSEQ]));
> +	if (tb[MPTCP_SUBFLOW_ATTR_SSN_OFFSET])
> +		printf(" ssnoff:%u",
> +		       rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_SSN_OFFSET]));
> +	if (tb[MPTCP_SUBFLOW_ATTR_MAP_DATALEN])
> +		printf(" maplen:%u",
> +		       rta_getattr(__u32, tb[MPTCP_SUBFLOW_ATTR_MAP_DATALEN]));

I guess you need:

  printf("\n");

here at the end.

Also, do not hesitate to show an example of the whole output in the
commit message, that would be helpful for the reviews.

> +}
> +
>  static void parse_nlmsg(struct nlmsghdr *nlh, __u32 proto)
>  {
>  	struct inet_diag_msg *r = NLMSG_DATA(nlh);
> @@ -219,6 +290,7 @@ static void parse_nlmsg(struct nlmsghdr *nlh, __u32 proto)
>  
>  			parse_rtattr_nested(sfinfo, MPTCP_SUBFLOW_ATTR_MAX,
>  					    ulpinfo[INET_ULP_INFO_MPTCP]);
> +			print_subflow_info(sfinfo);
>  		} else {
>  			printf("It's a normal TCP!\n");
>  		}


Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [mptcp-next v2 7/7] selftests: mptcp: add chk_sublfow in diag.sh

[Matthieu Baerts]

On 16/04/2025 10:59, Gang Yan wrote:
> This patch aims to add chk_dump_subflow in diag.sh. The subflow's
> info can be obtained through "ss -tin", then use the 'mptcp_diag'
> to verify the token in subflow_info.
> 
> Co-developed-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/diag.sh | 55 +++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
> index e7a75341f0f3..20de007e44cd 100755
> --- a/tools/testing/selftests/net/mptcp/diag.sh
> +++ b/tools/testing/selftests/net/mptcp/diag.sh
> @@ -225,6 +225,60 @@ chk_dump_one()
>  	fi
>  }
>  
> +get_endpoint_addr()
> +{
> +	echo $1 | cut -d ":" -f 1
> +}
> +
> +get_endpoint_port()
> +{
> +	echo $1 | cut -d ":" -f 2
> +}
> +
> +chk_dump_subflow()
> +{
> +	local inet_diag_token
> +	local subflow_line
> +	local remote_addr
> +	local remote_port
> +	local local_addr
> +	local local_port
> +	local ss_token
> +	local msg
> +
> +	subflow_line=$(ss -tnN $ns | \
> +		       grep -m1 -Eo '[0-9.]+:[0-9].+ +[0-9.]+:[0-9.]+')
> +
> +	if [ -n "$subflow_line" ]; then
> +		read -r local_endpoint remote_endpoint <<< $subflow_line
> +
> +		local_addr=$(get_endpoint_addr $local_endpoint)
> +		local_port=$(get_endpoint_port $local_endpoint)

(you could use 'read -rd: local_addr local_port' here ; same below)

> +
> +		remote_addr=$(get_endpoint_addr $remote_endpoint)
> +		remote_port=$(get_endpoint_port $remote_endpoint)
> +	fi
> +
> +	ss_token=$(ss -tniN $ns | \
> +		   grep -m1 -Eo 'token:[^ ]+')

It might be safer to call 'ss' only once, then extract the subflow and
the ports later.

> +
> +	inet_diag_token=$(ip netns exec $ns ./mptcp_diag -s \
> +			  "$local_addr $local_port $remote_addr $remote_port" | \

Why don't you call 'mptcp_diag -s "$subflow_line"', then adapt sscanf in
mptcp_diag.c to use: "%s:%d %s:%d"?

You will save a few lines of bash here above by doing that. Or is it
more difficult when handling IPv6 addresses? I don't think you handle
IPv6 addresses anyway, right?

> +			  grep -Eo 'token:[^ ]+')
> +
> +	msg="....chk dump_subflow"
> +
> +	mptcp_lib_print_title "$msg"
> +	if [[ $ss_token == $inet_diag_token ]]; then

Please use the same style as elsewhere in the file:

  if [ -n "$ss_token" ] && [ "$ss_token" = "$inet_diag_token" ]; then

> +		mptcp_lib_pr_ok
> +		mptcp_lib_result_pass "${msg}"
> +	else
> +		mptcp_lib_pr_fail "expected $ss_token but $inet_diag_token"

Same here for the style:

  "expected $ss_token found $inet_diag_token"

> +		mptcp_lib_result_fail "${msg}"
> +		ret=${KSFT_FAIL}
> +	fi
> +}
> +
>  msk_info_get_value()
>  {
>  	local port="${1}"
> @@ -316,6 +370,7 @@ chk_msk_fallback_nr 0 "....chk no fallback"
>  chk_msk_inuse 2
>  chk_msk_cestab 2
>  chk_dump_one
> +chk_dump_subflow
>  flush_pids
>  
>  chk_msk_inuse 0 "2->0"

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.