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..d4981b76693bbddca74169437a540ad6294cf1d5 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));
+ 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;
}
attrs = RTA_NEXT(attrs, msg_len);
}
}
+ if (server_side)
+ fprintf(stderr, ",server_side:1");
fprintf(stderr, "\n");
} while (1);
--
2.51.0
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); >
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..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: I'm not sure to understand why it is strange: the output is read by scripts, the order should not matter, right? Plus the order depends on what was received by the kernel, not where they are printed here. Do you really think it is worth it changing this patch and making longer just to keep an order that we don't currently fully control here? Cheers, Matt -- Sponsored by the NGI0 Core fund.
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. Thanks, -Geliang > > Plus the order depends on what was received by the kernel, not where > they are printed here. > > Do you really think it is worth it changing this patch and making > longer > just to keep an order that we don't currently fully control here? > > Cheers, > Matt
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.
On Thu, 2025-09-11 at 11:35 +0200, Matthieu Baerts wrote: > 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..d4981b76693bbddca74 > > > > > 1694 > > > > > 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:::f > > fff: > > 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 I do use this in BPF path manager selftests under development. > 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. Sure. I have no comments then. Please add my tag for this set: Reviewed-by: Geliang Tang <geliang@kernel.org> Thanks, -Geliang > > Cheers, > Matt
Hi Geliang, On 11/09/2025 12:05, Geliang Tang wrote: > On Thu, 2025-09-11 at 11:35 +0200, Matthieu Baerts wrote: >> 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..d4981b76693bbddca74 >>>>>> 1694 >>>>>> 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:::f >>> fff: >>> 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 > > I do use this in BPF path manager selftests under development. Probably best to avoid that. Can you not split based on ',' (strtok*()) and take the values for the keys you are interested in? >> 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. > > Sure. I have no comments then. Please add my tag for this set: > > Reviewed-by: Geliang Tang <geliang@kernel.org> Thanks! (Please next time send this as a reply to the cover-letter, so the tag will be automatically propagated to each patch.) Now in our tree (feat. for net-next): New patches for t/upstream: - 419c5f213113: mptcp: pm: netlink: only add server-side attr when true - 7a32b2ee28a7: mptcp: pm: netlink: announce server-side flag - ed9ca7fac132: mptcp: pm: netlink: deprecate server-side attribute - 65cb7a9d4d5e: selftests: mptcp: pm: get server-side flag - Results: 9d5b2844a3ca..7b11ba96fa1d (export) Tests are now in progress: - export: https://github.com/multipath-tcp/mptcp_net-next/commit/d1dac2258ca371e5cfcd46487e28590da3162f9b/checks Cheers, Matt -- Sponsored by the NGI0 Core fund.
© 2016 - 2025 Red Hat, Inc.