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

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 11/09/2025 11:04, Geliang Tang wrote:
> Hi Matt,
> 
> On Thu, 2025-09-11 at 10:35 +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 11/09/2025 10:16, Geliang Tang wrote:
>>> 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..d4981b76693bbddca741694
>>>> 37a5
>>>> 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:
>>
>> I'm not sure to understand why it is strange: the output is read by
>> scripts, the order should not matter, right?
> 
> I mean the original display order of "./pm_nl_ctl events" is:
> 
> type:1,token:2327950928,server_side:1,family:10,saddr6:::ffff:10.0.1.1,
> daddr6:::ffff:10.0.1.2,sport:10000,dport:40004,loc_id:0,rem_id:0
> 
> With this patch, the order is changed:
> 
> type:1,token:1072969578,family:10,saddr6:::ffff:10.0.1.1,daddr6:::ffff:
> 10.0.1.2,sport:10000,dport:42484,loc_id:0,rem_id:0,server_side:1
> 
> If an application still uses code like this to parse server_side, an
> error will occur.
> 
> sscanf(str, "type:%u,token:%u,server_side:%u",
>        &type, &token, &server_side);
> 
> So it makes sense to keep the original order. But up to you.

I hope no applications are doing that: this pm_nl_ctl tool lists the
arguments in the order it receives it, but this order is set by the
kernel and can change. In other words, this order is not fixed in stone.

Plus this tool is for the selftests, I don't think other people are
using it. 'ip mptcp' is probably used instead. And even they should not
expect the order to be fixed.

So I don't think we need to increase the complexity here if it is not
needed.

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