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

[Matthieu Baerts <matttbe@kernel.org>]

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.