Re: [PATCH mptcp-next v3 4/5] selftests: mptcp: add helpers to get subflow_info

[Matthieu Baerts <matttbe@kernel.org>]

Hi Gang,

On 25/04/2025 08:18, 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.
> 
> The helper 'print_subflow_info' in 'mptcp_diag' can print the subflow_filed
> of an MPTCP subflow for further checking the 'subflow_info' through
> inet_diag method.
> 
> The example of the whole output should be:
> 
> '''
> 127.0.0.1:10000 -> 127.0.0.1:38984
> It's a mptcp subflow, the subflow info:
>  flags:Mec token:0000(id:0)/4278e77e(id:0) seq:9288466187236176036 \
>  sfseq:1 ssnoff:2317083055 maplen:215
> '''
> 
> 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  | 159 +++++++++++++++++-
>  1 file changed, 157 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_diag.c b/tools/testing/selftests/net/mptcp/mptcp_diag.c
> index e19b21631641..13848a25938d 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_diag.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_diag.c

(...)

> @@ -244,6 +359,39 @@ static void get_mptcpinfo(__u32 token)
>  	close(fd);
>  }
>  
> +static void get_subflow_info(char *subflow_addrs)
> +{
> +	struct inet_diag_req_v2 r = {
> +		.sdiag_family           = AF_INET,
> +		.sdiag_protocol         = IPPROTO_TCP,
> +		.id.idiag_cookie[0]	= INET_DIAG_NOCOOKIE,
> +		.id.idiag_cookie[1]	= INET_DIAG_NOCOOKIE,
> +	};
> +	char saddr[64], daddr[64];
> +	int sport, dport;
> +	int ret;
> +	int fd;
> +
> +	ret = sscanf(subflow_addrs, "%[^:]:%d %[^:]:%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_sport = htons(sport);
> +	r.id.idiag_dport = htons(dport);
> +
> +	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));

This could be declared above as well I suppose.

> +	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 +399,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 +407,10 @@ 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_addrs, strlen(optarg) + 1,

The max size (2nd argument) should be linked to the destination buffer
size, not the source one.

Note that it would make more sense to use "strncpy()" here.

> +				 "%s", optarg);
> +			break;
>  		default:
>  			die_usage(1);
>  			break;
> @@ -275,6 +427,9 @@ int main(int argc, char *argv[])
>  	if (p.target_token)
>  		get_mptcpinfo(p.target_token);
>  
> +	if (strlen(p.subflow_addrs) != 0)
> +		get_subflow_info(p.subflow_addrs);
> +
>  	return 0;
>  }
>  

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