Re: [PATCH mptcp-next 4/4] selftests: mptcp: pm: get server-side flag

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

On Tue, 2025-09-09 at 18:30 +0200, Matthieu Baerts (NGI0) wrote:
> server-side info linked to the MPTCP connect/established events can
> now
> come from the flags, in addition to the dedicated attribute.
> 
> The attribute is now deprecated -- in favour of the new flag, and
> will
> be removed later on.
> 
> Print this info only once.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> index
> 93fea3442216c8fef43731a99c1d5710f234b150..d4981b76693bbddca74169437a5
> 40ad6294cf1d5 100644
> --- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> +++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> @@ -2,6 +2,7 @@
>  
>  #include <errno.h>
>  #include <error.h>
> +#include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -113,6 +114,8 @@ static int capture_events(int fd, int
> event_group)
>  		error(1, errno, "could not join the "
> MPTCP_PM_EV_GRP_NAME " mcast group");
>  
>  	do {
> +		bool server_side = false;
> +
>  		FD_ZERO(&rfds);
>  		FD_SET(fd, &rfds);
>  		res_len = NLMSG_ALIGN(sizeof(struct nlmsghdr)) +
> @@ -187,18 +190,22 @@ static int capture_events(int fd, int
> event_group)
>  				else if (attrs->rta_type ==
> MPTCP_ATTR_ERROR)
>  					fprintf(stderr, ",error:%u",
> *(__u8 *)RTA_DATA(attrs));
>  				else if (attrs->rta_type ==
> MPTCP_ATTR_SERVER_SIDE)
> -					fprintf(stderr,
> ",server_side:%u", *(__u8 *)RTA_DATA(attrs));

It is a little strange that server_side is displayed at the end. It
changes the original display order.  It is better to display it in the
original position, but check whether the server_side flag is false:

  else if (attrs->rta_type == MPTCP_ATTR_SERVER_SIDE &&
           !server_side) {
          fprintf(stderr, ",server_side:%u", *(__u8 *)RTA_DATA(attrs));
          server_side = true;
  } else if (attrs->rta_type == MPTCP_ATTR_FLAGS) {

> +					server_side = !!*(__u8
> *)RTA_DATA(attrs);
>  				else if (attrs->rta_type ==
> MPTCP_ATTR_FLAGS) {
>  					__u16 flags = *(__u16
> *)RTA_DATA(attrs);
>  
>  					/* only print when present,
> easier */
>  					if (flags &
> MPTCP_PM_EV_FLAG_DENY_JOIN_ID0)
>  						fprintf(stderr,
> ",deny_join_id0:1");
> +					if (flags &
> MPTCP_PM_EV_FLAG_SERVER_SIDE)
> +						server_side = true;

Here the server_side flag also needs to be checked:

  if (flags & MPTCP_PM_EV_FLAG_SERVER_SIDE &&
      !server_side) {
          fprintf(stderr, ",server_side:1");
          server_side = true;
  }

>  				}
>  
>  				attrs = RTA_NEXT(attrs, msg_len);
>  			}
>  		}
> +		if (server_side)
> +			fprintf(stderr, ",server_side:1");

Then no need to display it at the end.

WDYT?

Thanks,
-Geliang

>  		fprintf(stderr, "\n");
>  	} while (1);
>