[PATCH mptcp-next 0/4] mptcp: pm: netlink: announce server-side flag

[PATCH mptcp-next 0/4] mptcp: pm: netlink: announce server-side flag

[Matthieu Baerts (NGI0)]

Now that the 'flags' attribute is used, it seems interesting to add one
flag for 'server-side', a boolean value.

Here are a few patches related to the 'server-side' attribute:

- Patch 1: only announce this attribute on the server side.

- Patch 2: announce the 'server-side' flag when this is the case.

- Patch 3: deprecate the 'server-side' attribute.

- Patch 4: use the 'server-side' flag in the selftests.

Based-on: <20250909-mptcp-pm-user-c-flag-v2-0-a6f9542481c5@kernel.org>

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (4):
      mptcp: pm: netlink: only add server-side attr when true
      mptcp: pm: netlink: announce server-side flag
      mptcp: pm: netlink: deprecate server-side attribute
      selftests: mptcp: pm: get server-side flag

 Documentation/netlink/specs/mptcp_pm.yaml         | 5 +++--
 include/uapi/linux/mptcp.h                        | 1 +
 include/uapi/linux/mptcp_pm.h                     | 4 ++--
 net/mptcp/pm_netlink.c                            | 9 +++++++--
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c     | 9 ++++++++-
 tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
 6 files changed, 22 insertions(+), 8 deletions(-)
---
base-commit: 12e66f7051b4b535a990cd20c149d76a98ec68c3
change-id: 20250909-mptcp-pm-user-server-side-flag-214c84b90b21
prerequisite-change-id: 20250720-mptcp-pm-user-c-flag-a7d5d7a00c6f:v2
prerequisite-patch-id: cc65204dc8620f1675f8b8cbdf6dc0fecf20379e
prerequisite-patch-id: 88640c51fc66f4d630e2f99db72c18830273fc83
prerequisite-patch-id: 5773e6ceede07a2c43550774eddad1c32cde67e0
prerequisite-patch-id: f9196f35eff69835799db4312e82c795f1b15eb0
prerequisite-patch-id: 8ec26f3fe4baa5128ac2f01f7284184bbb0e651b

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

[PATCH mptcp-next 1/4] mptcp: pm: netlink: only add server-side attr when true

[Matthieu Baerts (NGI0)]

This attribute is a boolean. No need to add it to set it to 'false'.

Indeed, the default value when this attribute is not set is naturally
'false'. A few bytes can then be saved by not adding this attribute if
the connection is not on the server side.

This prepares the future deprecation of its attribute, in favour of a
new flag.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 Documentation/netlink/specs/mptcp_pm.yaml         | 4 ++--
 include/uapi/linux/mptcp_pm.h                     | 4 ++--
 net/mptcp/pm_netlink.c                            | 4 +++-
 tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
index d1b4829b580ad09baf4afd73b67abd7b4ef6883a..fc47a2931014c0304efd3215cc24485ea22e1ede 100644
--- a/Documentation/netlink/specs/mptcp_pm.yaml
+++ b/Documentation/netlink/specs/mptcp_pm.yaml
@@ -28,13 +28,13 @@ definitions:
           traffic-patterns it can take a long time until the
           MPTCP_EVENT_ESTABLISHED is sent.
           Attributes: token, family, saddr4 | saddr6, daddr4 | daddr6, sport,
-          dport, server-side, [flags].
+          dport, [server-side], [flags].
       -
         name: established
         doc: >-
           A MPTCP connection is established (can start new subflows).
           Attributes: token, family, saddr4 | saddr6, daddr4 | daddr6, sport,
-          dport, server-side, [flags].
+          dport, [server-side], [flags].
       -
         name: closed
         doc: >-
diff --git a/include/uapi/linux/mptcp_pm.h b/include/uapi/linux/mptcp_pm.h
index 7359d34da446b94be148b363079120db03ba8549..bf44a5cf5b5a1e6d789632682a9bedbf8090feb9 100644
--- a/include/uapi/linux/mptcp_pm.h
+++ b/include/uapi/linux/mptcp_pm.h
@@ -16,10 +16,10 @@
  *   good time to allocate memory and send ADD_ADDR if needed. Depending on the
  *   traffic-patterns it can take a long time until the MPTCP_EVENT_ESTABLISHED
  *   is sent. Attributes: token, family, saddr4 | saddr6, daddr4 | daddr6,
- *   sport, dport, server-side, [flags].
+ *   sport, dport, [server-side], [flags].
  * @MPTCP_EVENT_ESTABLISHED: A MPTCP connection is established (can start new
  *   subflows). Attributes: token, family, saddr4 | saddr6, daddr4 | daddr6,
- *   sport, dport, server-side, [flags].
+ *   sport, dport, [server-side], [flags].
  * @MPTCP_EVENT_CLOSED: A MPTCP connection has stopped. Attribute: token.
  * @MPTCP_EVENT_ANNOUNCED: A new address has been announced by the peer.
  *   Attributes: token, rem_id, family, daddr4 | daddr6 [, dport].
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 483ddbb9ec406a3e965376ee5a833ae295896a02..33a6bf536c020b59717472aca2d38add26255419 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -413,7 +413,9 @@ static int mptcp_event_created(struct sk_buff *skb,
 	if (err)
 		return err;
 
-	if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, READ_ONCE(msk->pm.server_side)))
+	/* only set when it is the server side */
+	if (READ_ONCE(msk->pm.server_side) &&
+	    nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
 		return -EMSGSIZE;
 
 	if (READ_ONCE(msk->pm.remote_deny_join_id0))
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 0801d57a9710406a5942af742719f694e3907558..189bfc5bf86611575381542fe59b86660f335294 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -241,7 +241,7 @@ make_connection()
 
 	print_test "Established IP${is_v6} MPTCP Connection ns2 => ns1"
 	if [ "${client_token}" != "" ] && [ "${server_token}" != "" ] &&
-	   [ "${client_serverside}" = 0 ] && [ "${server_serverside}" = 1 ] &&
+	   [ "${client_serverside:-0}" = 0 ] && [ "${server_serverside:-0}" = 1 ] &&
 	   [ "${client_nojoin:-0}" = 1 ] && [ "${server_nojoin:-0}" = 0 ]
 	then
 		test_pass

-- 
2.51.0

Re: [PATCH mptcp-next 1/4] mptcp: pm: netlink: only add server-side attr when true

[Geliang Tang]

Hi Matt,

On Tue, 2025-09-09 at 18:30 +0200, Matthieu Baerts (NGI0) wrote:
> This attribute is a boolean. No need to add it to set it to 'false'.
> 
> Indeed, the default value when this attribute is not set is naturally
> 'false'. A few bytes can then be saved by not adding this attribute
> if
> the connection is not on the server side.
> 
> This prepares the future deprecation of its attribute, in favour of a
> new flag.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  Documentation/netlink/specs/mptcp_pm.yaml         | 4 ++--
>  include/uapi/linux/mptcp_pm.h                     | 4 ++--
>  net/mptcp/pm_netlink.c                            | 4 +++-
>  tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
>  4 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
> b/Documentation/netlink/specs/mptcp_pm.yaml
> index
> d1b4829b580ad09baf4afd73b67abd7b4ef6883a..fc47a2931014c0304efd3215cc2
> 4485ea22e1ede 100644
> --- a/Documentation/netlink/specs/mptcp_pm.yaml
> +++ b/Documentation/netlink/specs/mptcp_pm.yaml
> @@ -28,13 +28,13 @@ definitions:
>            traffic-patterns it can take a long time until the
>            MPTCP_EVENT_ESTABLISHED is sent.
>            Attributes: token, family, saddr4 | saddr6, daddr4 |
> daddr6, sport,
> -          dport, server-side, [flags].
> +          dport, [server-side], [flags].
>        -
>          name: established
>          doc: >-
>            A MPTCP connection is established (can start new
> subflows).
>            Attributes: token, family, saddr4 | saddr6, daddr4 |
> daddr6, sport,
> -          dport, server-side, [flags].
> +          dport, [server-side], [flags].
>        -
>          name: closed
>          doc: >-
> diff --git a/include/uapi/linux/mptcp_pm.h
> b/include/uapi/linux/mptcp_pm.h
> index
> 7359d34da446b94be148b363079120db03ba8549..bf44a5cf5b5a1e6d789632682a9
> bedbf8090feb9 100644
> --- a/include/uapi/linux/mptcp_pm.h
> +++ b/include/uapi/linux/mptcp_pm.h
> @@ -16,10 +16,10 @@
>   *   good time to allocate memory and send ADD_ADDR if needed.
> Depending on the
>   *   traffic-patterns it can take a long time until the
> MPTCP_EVENT_ESTABLISHED
>   *   is sent. Attributes: token, family, saddr4 | saddr6, daddr4 |
> daddr6,
> - *   sport, dport, server-side, [flags].
> + *   sport, dport, [server-side], [flags].
>   * @MPTCP_EVENT_ESTABLISHED: A MPTCP connection is established (can
> start new
>   *   subflows). Attributes: token, family, saddr4 | saddr6, daddr4 |
> daddr6,
> - *   sport, dport, server-side, [flags].
> + *   sport, dport, [server-side], [flags].
>   * @MPTCP_EVENT_CLOSED: A MPTCP connection has stopped. Attribute:
> token.
>   * @MPTCP_EVENT_ANNOUNCED: A new address has been announced by the
> peer.
>   *   Attributes: token, rem_id, family, daddr4 | daddr6 [, dport].
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index
> 483ddbb9ec406a3e965376ee5a833ae295896a02..33a6bf536c020b59717472aca2d
> 38add26255419 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -413,7 +413,9 @@ static int mptcp_event_created(struct sk_buff
> *skb,
>  	if (err)
>  		return err;
>  
> -	if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, READ_ONCE(msk-
> >pm.server_side)))
> +	/* only set when it is the server side */
> +	if (READ_ONCE(msk->pm.server_side) &&
> +	    nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))

In patch 2, this will be modified to use two 'if':

	if (READ_ONCE(msk->pm.server_side)) {
		flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;

		/* only set when it is the server side */
		if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
			return -EMSGSIZE;
	}

Why don't we just use two 'if' statements here, which will make patch 2
simpler:

       if (READ_ONCE(msk->pm.server_side)) {
               /* only set when it is the server side */
               if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
                       return -EMSGSIZE;
       }

WDYT?

Thanks,
-Geliang

>  		return -EMSGSIZE;
>  
>  	if (READ_ONCE(msk->pm.remote_deny_join_id0))
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index
> 0801d57a9710406a5942af742719f694e3907558..189bfc5bf86611575381542fe59
> b86660f335294 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -241,7 +241,7 @@ make_connection()
>  
>  	print_test "Established IP${is_v6} MPTCP Connection ns2 =>
> ns1"
>  	if [ "${client_token}" != "" ] && [ "${server_token}" != ""
> ] &&
> -	   [ "${client_serverside}" = 0 ] && [
> "${server_serverside}" = 1 ] &&
> +	   [ "${client_serverside:-0}" = 0 ] && [
> "${server_serverside:-0}" = 1 ] &&
>  	   [ "${client_nojoin:-0}" = 1 ] && [ "${server_nojoin:-0}"
> = 0 ]
>  	then
>  		test_pass

Re: [PATCH mptcp-next 1/4] mptcp: pm: netlink: only add server-side attr when true

[Matthieu Baerts]

Hi Geliang,

Thank you for the review!

On 11/09/2025 10:15, Geliang Tang wrote:
> Hi Matt,
> 
> On Tue, 2025-09-09 at 18:30 +0200, Matthieu Baerts (NGI0) wrote:
>> This attribute is a boolean. No need to add it to set it to 'false'.
>>
>> Indeed, the default value when this attribute is not set is naturally
>> 'false'. A few bytes can then be saved by not adding this attribute
>> if
>> the connection is not on the server side.
>>
>> This prepares the future deprecation of its attribute, in favour of a
>> new flag.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  Documentation/netlink/specs/mptcp_pm.yaml         | 4 ++--
>>  include/uapi/linux/mptcp_pm.h                     | 4 ++--
>>  net/mptcp/pm_netlink.c                            | 4 +++-
>>  tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
>>  4 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
>> b/Documentation/netlink/specs/mptcp_pm.yaml
>> index
>> d1b4829b580ad09baf4afd73b67abd7b4ef6883a..fc47a2931014c0304efd3215cc2
>> 4485ea22e1ede 100644
>> --- a/Documentation/netlink/specs/mptcp_pm.yaml
>> +++ b/Documentation/netlink/specs/mptcp_pm.yaml
>> @@ -28,13 +28,13 @@ definitions:
>>            traffic-patterns it can take a long time until the
>>            MPTCP_EVENT_ESTABLISHED is sent.
>>            Attributes: token, family, saddr4 | saddr6, daddr4 |
>> daddr6, sport,
>> -          dport, server-side, [flags].
>> +          dport, [server-side], [flags].
>>        -
>>          name: established
>>          doc: >-
>>            A MPTCP connection is established (can start new
>> subflows).
>>            Attributes: token, family, saddr4 | saddr6, daddr4 |
>> daddr6, sport,
>> -          dport, server-side, [flags].
>> +          dport, [server-side], [flags].
>>        -
>>          name: closed
>>          doc: >-
>> diff --git a/include/uapi/linux/mptcp_pm.h
>> b/include/uapi/linux/mptcp_pm.h
>> index
>> 7359d34da446b94be148b363079120db03ba8549..bf44a5cf5b5a1e6d789632682a9
>> bedbf8090feb9 100644
>> --- a/include/uapi/linux/mptcp_pm.h
>> +++ b/include/uapi/linux/mptcp_pm.h
>> @@ -16,10 +16,10 @@
>>   *   good time to allocate memory and send ADD_ADDR if needed.
>> Depending on the
>>   *   traffic-patterns it can take a long time until the
>> MPTCP_EVENT_ESTABLISHED
>>   *   is sent. Attributes: token, family, saddr4 | saddr6, daddr4 |
>> daddr6,
>> - *   sport, dport, server-side, [flags].
>> + *   sport, dport, [server-side], [flags].
>>   * @MPTCP_EVENT_ESTABLISHED: A MPTCP connection is established (can
>> start new
>>   *   subflows). Attributes: token, family, saddr4 | saddr6, daddr4 |
>> daddr6,
>> - *   sport, dport, server-side, [flags].
>> + *   sport, dport, [server-side], [flags].
>>   * @MPTCP_EVENT_CLOSED: A MPTCP connection has stopped. Attribute:
>> token.
>>   * @MPTCP_EVENT_ANNOUNCED: A new address has been announced by the
>> peer.
>>   *   Attributes: token, rem_id, family, daddr4 | daddr6 [, dport].
>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>> index
>> 483ddbb9ec406a3e965376ee5a833ae295896a02..33a6bf536c020b59717472aca2d
>> 38add26255419 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c
>> @@ -413,7 +413,9 @@ static int mptcp_event_created(struct sk_buff
>> *skb,
>>  	if (err)
>>  		return err;
>>  
>> -	if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, READ_ONCE(msk-
>>> pm.server_side)))
>> +	/* only set when it is the server side */
>> +	if (READ_ONCE(msk->pm.server_side) &&
>> +	    nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
> 
> In patch 2, this will be modified to use two 'if':
> 
> 	if (READ_ONCE(msk->pm.server_side)) {
> 		flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
> 
> 		/* only set when it is the server side */
> 		if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
> 			return -EMSGSIZE;
> 	}
> 
> Why don't we just use two 'if' statements here, which will make patch 2
> simpler:
> 
>        if (READ_ONCE(msk->pm.server_side)) {
>                /* only set when it is the server side */
>                if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
>                        return -EMSGSIZE;
>        }
> 
> WDYT?

I'm not convinced: that might make the patch 2 simpler, but then the
patch 1 will raise questions from reviewers and developers looking at
the patches in the order they have been sent: why using 2 'if'
statements while there is no need to use 2.

If we want that, I would have to add something in the commit message. I
don't think that's worth it.

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

Re: [PATCH mptcp-next 1/4] mptcp: pm: netlink: only add server-side attr when true

[Geliang Tang]

Hi Matt,

On Thu, 2025-09-11 at 10:20 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for the review!
> 
> On 11/09/2025 10:15, Geliang Tang wrote:
> > Hi Matt,
> > 
> > On Tue, 2025-09-09 at 18:30 +0200, Matthieu Baerts (NGI0) wrote:
> > > This attribute is a boolean. No need to add it to set it to
> > > 'false'.
> > > 
> > > Indeed, the default value when this attribute is not set is
> > > naturally
> > > 'false'. A few bytes can then be saved by not adding this
> > > attribute
> > > if
> > > the connection is not on the server side.
> > > 
> > > This prepares the future deprecation of its attribute, in favour
> > > of a
> > > new flag.
> > > 
> > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > > ---
> > >  Documentation/netlink/specs/mptcp_pm.yaml         | 4 ++--
> > >  include/uapi/linux/mptcp_pm.h                     | 4 ++--
> > >  net/mptcp/pm_netlink.c                            | 4 +++-
> > >  tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
> > >  4 files changed, 8 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
> > > b/Documentation/netlink/specs/mptcp_pm.yaml
> > > index
> > > d1b4829b580ad09baf4afd73b67abd7b4ef6883a..fc47a2931014c0304efd321
> > > 5cc2
> > > 4485ea22e1ede 100644
> > > --- a/Documentation/netlink/specs/mptcp_pm.yaml
> > > +++ b/Documentation/netlink/specs/mptcp_pm.yaml
> > > @@ -28,13 +28,13 @@ definitions:
> > >            traffic-patterns it can take a long time until the
> > >            MPTCP_EVENT_ESTABLISHED is sent.
> > >            Attributes: token, family, saddr4 | saddr6, daddr4 |
> > > daddr6, sport,
> > > -          dport, server-side, [flags].
> > > +          dport, [server-side], [flags].
> > >        -
> > >          name: established
> > >          doc: >-
> > >            A MPTCP connection is established (can start new
> > > subflows).
> > >            Attributes: token, family, saddr4 | saddr6, daddr4 |
> > > daddr6, sport,
> > > -          dport, server-side, [flags].
> > > +          dport, [server-side], [flags].
> > >        -
> > >          name: closed
> > >          doc: >-
> > > diff --git a/include/uapi/linux/mptcp_pm.h
> > > b/include/uapi/linux/mptcp_pm.h
> > > index
> > > 7359d34da446b94be148b363079120db03ba8549..bf44a5cf5b5a1e6d7896326
> > > 82a9
> > > bedbf8090feb9 100644
> > > --- a/include/uapi/linux/mptcp_pm.h
> > > +++ b/include/uapi/linux/mptcp_pm.h
> > > @@ -16,10 +16,10 @@
> > >   *   good time to allocate memory and send ADD_ADDR if needed.
> > > Depending on the
> > >   *   traffic-patterns it can take a long time until the
> > > MPTCP_EVENT_ESTABLISHED
> > >   *   is sent. Attributes: token, family, saddr4 | saddr6, daddr4
> > > |
> > > daddr6,
> > > - *   sport, dport, server-side, [flags].
> > > + *   sport, dport, [server-side], [flags].
> > >   * @MPTCP_EVENT_ESTABLISHED: A MPTCP connection is established
> > > (can
> > > start new
> > >   *   subflows). Attributes: token, family, saddr4 | saddr6,
> > > daddr4 |
> > > daddr6,
> > > - *   sport, dport, server-side, [flags].
> > > + *   sport, dport, [server-side], [flags].
> > >   * @MPTCP_EVENT_CLOSED: A MPTCP connection has stopped.
> > > Attribute:
> > > token.
> > >   * @MPTCP_EVENT_ANNOUNCED: A new address has been announced by
> > > the
> > > peer.
> > >   *   Attributes: token, rem_id, family, daddr4 | daddr6 [,
> > > dport].
> > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > > index
> > > 483ddbb9ec406a3e965376ee5a833ae295896a02..33a6bf536c020b59717472a
> > > ca2d
> > > 38add26255419 100644
> > > --- a/net/mptcp/pm_netlink.c
> > > +++ b/net/mptcp/pm_netlink.c
> > > @@ -413,7 +413,9 @@ static int mptcp_event_created(struct sk_buff
> > > *skb,
> > >  	if (err)
> > >  		return err;
> > >  
> > > -	if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE,
> > > READ_ONCE(msk-
> > > > pm.server_side)))
> > > +	/* only set when it is the server side */
> > > +	if (READ_ONCE(msk->pm.server_side) &&
> > > +	    nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
> > 
> > In patch 2, this will be modified to use two 'if':
> > 
> > 	if (READ_ONCE(msk->pm.server_side)) {
> > 		flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
> > 
> > 		/* only set when it is the server side */
> > 		if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
> > 			return -EMSGSIZE;
> > 	}
> > 
> > Why don't we just use two 'if' statements here, which will make
> > patch 2
> > simpler:
> > 
> >        if (READ_ONCE(msk->pm.server_side)) {
> >                /* only set when it is the server side */
> >                if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
> >                        return -EMSGSIZE;
> >        }
> > 
> > WDYT?
> 
> I'm not convinced: that might make the patch 2 simpler, but then the
> patch 1 will raise questions from reviewers and developers looking at
> the patches in the order they have been sent: why using 2 'if'
> statements while there is no need to use 2.
> 
> If we want that, I would have to add something in the commit message.
> I
> don't think that's worth it.

Sure. Let's keep this patch as is. If so, could you please update the
position of the comments in patch 2 and patch 3? Something like:

        /* only set when it is the server side */
        if (READ_ONCE(msk->pm.server_side)) {
                flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;

                /* Deprecated */
                if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1)) 
                        return -EMSGSIZE;
        }

Thanks,
-Geliang

> 
> Cheers,
> Matt

Re: [PATCH mptcp-next 1/4] mptcp: pm: netlink: only add server-side attr when true

[Matthieu Baerts]

Hi Geliang,

On 11/09/2025 10:33, Geliang Tang wrote:
> Hi Matt,
> 
> On Thu, 2025-09-11 at 10:20 +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> Thank you for the review!
>>
>> On 11/09/2025 10:15, Geliang Tang wrote:
>>> Hi Matt,
>>>
>>> On Tue, 2025-09-09 at 18:30 +0200, Matthieu Baerts (NGI0) wrote:
>>>> This attribute is a boolean. No need to add it to set it to
>>>> 'false'.
>>>>
>>>> Indeed, the default value when this attribute is not set is
>>>> naturally
>>>> 'false'. A few bytes can then be saved by not adding this
>>>> attribute
>>>> if
>>>> the connection is not on the server side.
>>>>
>>>> This prepares the future deprecation of its attribute, in favour
>>>> of a
>>>> new flag.
>>>>
>>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>>> ---
>>>>  Documentation/netlink/specs/mptcp_pm.yaml         | 4 ++--
>>>>  include/uapi/linux/mptcp_pm.h                     | 4 ++--
>>>>  net/mptcp/pm_netlink.c                            | 4 +++-
>>>>  tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
>>>>  4 files changed, 8 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
>>>> b/Documentation/netlink/specs/mptcp_pm.yaml
>>>> index
>>>> d1b4829b580ad09baf4afd73b67abd7b4ef6883a..fc47a2931014c0304efd321
>>>> 5cc2
>>>> 4485ea22e1ede 100644
>>>> --- a/Documentation/netlink/specs/mptcp_pm.yaml
>>>> +++ b/Documentation/netlink/specs/mptcp_pm.yaml
>>>> @@ -28,13 +28,13 @@ definitions:
>>>>            traffic-patterns it can take a long time until the
>>>>            MPTCP_EVENT_ESTABLISHED is sent.
>>>>            Attributes: token, family, saddr4 | saddr6, daddr4 |
>>>> daddr6, sport,
>>>> -          dport, server-side, [flags].
>>>> +          dport, [server-side], [flags].
>>>>        -
>>>>          name: established
>>>>          doc: >-
>>>>            A MPTCP connection is established (can start new
>>>> subflows).
>>>>            Attributes: token, family, saddr4 | saddr6, daddr4 |
>>>> daddr6, sport,
>>>> -          dport, server-side, [flags].
>>>> +          dport, [server-side], [flags].
>>>>        -
>>>>          name: closed
>>>>          doc: >-
>>>> diff --git a/include/uapi/linux/mptcp_pm.h
>>>> b/include/uapi/linux/mptcp_pm.h
>>>> index
>>>> 7359d34da446b94be148b363079120db03ba8549..bf44a5cf5b5a1e6d7896326
>>>> 82a9
>>>> bedbf8090feb9 100644
>>>> --- a/include/uapi/linux/mptcp_pm.h
>>>> +++ b/include/uapi/linux/mptcp_pm.h
>>>> @@ -16,10 +16,10 @@
>>>>   *   good time to allocate memory and send ADD_ADDR if needed.
>>>> Depending on the
>>>>   *   traffic-patterns it can take a long time until the
>>>> MPTCP_EVENT_ESTABLISHED
>>>>   *   is sent. Attributes: token, family, saddr4 | saddr6, daddr4
>>>> |
>>>> daddr6,
>>>> - *   sport, dport, server-side, [flags].
>>>> + *   sport, dport, [server-side], [flags].
>>>>   * @MPTCP_EVENT_ESTABLISHED: A MPTCP connection is established
>>>> (can
>>>> start new
>>>>   *   subflows). Attributes: token, family, saddr4 | saddr6,
>>>> daddr4 |
>>>> daddr6,
>>>> - *   sport, dport, server-side, [flags].
>>>> + *   sport, dport, [server-side], [flags].
>>>>   * @MPTCP_EVENT_CLOSED: A MPTCP connection has stopped.
>>>> Attribute:
>>>> token.
>>>>   * @MPTCP_EVENT_ANNOUNCED: A new address has been announced by
>>>> the
>>>> peer.
>>>>   *   Attributes: token, rem_id, family, daddr4 | daddr6 [,
>>>> dport].
>>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>>>> index
>>>> 483ddbb9ec406a3e965376ee5a833ae295896a02..33a6bf536c020b59717472a
>>>> ca2d
>>>> 38add26255419 100644
>>>> --- a/net/mptcp/pm_netlink.c
>>>> +++ b/net/mptcp/pm_netlink.c
>>>> @@ -413,7 +413,9 @@ static int mptcp_event_created(struct sk_buff
>>>> *skb,
>>>>  	if (err)
>>>>  		return err;
>>>>  
>>>> -	if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE,
>>>> READ_ONCE(msk-
>>>>> pm.server_side)))
>>>> +	/* only set when it is the server side */
>>>> +	if (READ_ONCE(msk->pm.server_side) &&
>>>> +	    nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
>>>
>>> In patch 2, this will be modified to use two 'if':
>>>
>>> 	if (READ_ONCE(msk->pm.server_side)) {
>>> 		flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
>>>
>>> 		/* only set when it is the server side */
>>> 		if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
>>> 			return -EMSGSIZE;
>>> 	}
>>>
>>> Why don't we just use two 'if' statements here, which will make
>>> patch 2
>>> simpler:
>>>
>>>        if (READ_ONCE(msk->pm.server_side)) {
>>>                /* only set when it is the server side */
>>>                if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
>>>                        return -EMSGSIZE;
>>>        }
>>>
>>> WDYT?
>>
>> I'm not convinced: that might make the patch 2 simpler, but then the
>> patch 1 will raise questions from reviewers and developers looking at
>> the patches in the order they have been sent: why using 2 'if'
>> statements while there is no need to use 2.
>>
>> If we want that, I would have to add something in the commit message.
>> I
>> don't think that's worth it.
> 
> Sure. Let's keep this patch as is. If so, could you please update the
> position of the comments in patch 2 and patch 3? Something like:
> 
>         /* only set when it is the server side */
>         if (READ_ONCE(msk->pm.server_side)) {
>                 flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;

Mmh, I'm not sure: for the flag, it makes sense to only set it when it
is the server side, otherwise that's wrong. Keeping the comment above is
then a bit confusing, no?

> 
>                 /* Deprecated */
>                 if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1)) 
>                         return -EMSGSIZE;
>         }
> 
> Thanks,
> -Geliang
> 
>>
>> Cheers,
>> Matt

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

Re: [PATCH mptcp-next 1/4] mptcp: pm: netlink: only add server-side attr when true

[Geliang Tang]

On Thu, 2025-09-11 at 10:37 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 11/09/2025 10:33, Geliang Tang wrote:
> > Hi Matt,
> > 
> > On Thu, 2025-09-11 at 10:20 +0200, Matthieu Baerts wrote:
> > > Hi Geliang,
> > > 
> > > Thank you for the review!
> > > 
> > > On 11/09/2025 10:15, Geliang Tang wrote:
> > > > Hi Matt,
> > > > 
> > > > On Tue, 2025-09-09 at 18:30 +0200, Matthieu Baerts (NGI0)
> > > > wrote:
> > > > > This attribute is a boolean. No need to add it to set it to
> > > > > 'false'.
> > > > > 
> > > > > Indeed, the default value when this attribute is not set is
> > > > > naturally
> > > > > 'false'. A few bytes can then be saved by not adding this
> > > > > attribute
> > > > > if
> > > > > the connection is not on the server side.
> > > > > 
> > > > > This prepares the future deprecation of its attribute, in
> > > > > favour
> > > > > of a
> > > > > new flag.
> > > > > 
> > > > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > > > > ---
> > > > >  Documentation/netlink/specs/mptcp_pm.yaml         | 4 ++--
> > > > >  include/uapi/linux/mptcp_pm.h                     | 4 ++--
> > > > >  net/mptcp/pm_netlink.c                            | 4 +++-
> > > > >  tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
> > > > >  4 files changed, 8 insertions(+), 6 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
> > > > > b/Documentation/netlink/specs/mptcp_pm.yaml
> > > > > index
> > > > > d1b4829b580ad09baf4afd73b67abd7b4ef6883a..fc47a2931014c0304ef
> > > > > d321
> > > > > 5cc2
> > > > > 4485ea22e1ede 100644
> > > > > --- a/Documentation/netlink/specs/mptcp_pm.yaml
> > > > > +++ b/Documentation/netlink/specs/mptcp_pm.yaml
> > > > > @@ -28,13 +28,13 @@ definitions:
> > > > >            traffic-patterns it can take a long time until the
> > > > >            MPTCP_EVENT_ESTABLISHED is sent.
> > > > >            Attributes: token, family, saddr4 | saddr6, daddr4
> > > > > |
> > > > > daddr6, sport,
> > > > > -          dport, server-side, [flags].
> > > > > +          dport, [server-side], [flags].
> > > > >        -
> > > > >          name: established
> > > > >          doc: >-
> > > > >            A MPTCP connection is established (can start new
> > > > > subflows).
> > > > >            Attributes: token, family, saddr4 | saddr6, daddr4
> > > > > |
> > > > > daddr6, sport,
> > > > > -          dport, server-side, [flags].
> > > > > +          dport, [server-side], [flags].
> > > > >        -
> > > > >          name: closed
> > > > >          doc: >-
> > > > > diff --git a/include/uapi/linux/mptcp_pm.h
> > > > > b/include/uapi/linux/mptcp_pm.h
> > > > > index
> > > > > 7359d34da446b94be148b363079120db03ba8549..bf44a5cf5b5a1e6d789
> > > > > 6326
> > > > > 82a9
> > > > > bedbf8090feb9 100644
> > > > > --- a/include/uapi/linux/mptcp_pm.h
> > > > > +++ b/include/uapi/linux/mptcp_pm.h
> > > > > @@ -16,10 +16,10 @@
> > > > >   *   good time to allocate memory and send ADD_ADDR if
> > > > > needed.
> > > > > Depending on the
> > > > >   *   traffic-patterns it can take a long time until the
> > > > > MPTCP_EVENT_ESTABLISHED
> > > > >   *   is sent. Attributes: token, family, saddr4 | saddr6,
> > > > > daddr4
> > > > > > 
> > > > > daddr6,
> > > > > - *   sport, dport, server-side, [flags].
> > > > > + *   sport, dport, [server-side], [flags].
> > > > >   * @MPTCP_EVENT_ESTABLISHED: A MPTCP connection is
> > > > > established
> > > > > (can
> > > > > start new
> > > > >   *   subflows). Attributes: token, family, saddr4 | saddr6,
> > > > > daddr4 |
> > > > > daddr6,
> > > > > - *   sport, dport, server-side, [flags].
> > > > > + *   sport, dport, [server-side], [flags].
> > > > >   * @MPTCP_EVENT_CLOSED: A MPTCP connection has stopped.
> > > > > Attribute:
> > > > > token.
> > > > >   * @MPTCP_EVENT_ANNOUNCED: A new address has been announced
> > > > > by
> > > > > the
> > > > > peer.
> > > > >   *   Attributes: token, rem_id, family, daddr4 | daddr6 [,
> > > > > dport].
> > > > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > > > > index
> > > > > 483ddbb9ec406a3e965376ee5a833ae295896a02..33a6bf536c020b59717
> > > > > 472a
> > > > > ca2d
> > > > > 38add26255419 100644
> > > > > --- a/net/mptcp/pm_netlink.c
> > > > > +++ b/net/mptcp/pm_netlink.c
> > > > > @@ -413,7 +413,9 @@ static int mptcp_event_created(struct
> > > > > sk_buff
> > > > > *skb,
> > > > >  	if (err)
> > > > >  		return err;
> > > > >  
> > > > > -	if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE,
> > > > > READ_ONCE(msk-
> > > > > > pm.server_side)))
> > > > > +	/* only set when it is the server side */
> > > > > +	if (READ_ONCE(msk->pm.server_side) &&
> > > > > +	    nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
> > > > 
> > > > In patch 2, this will be modified to use two 'if':
> > > > 
> > > > 	if (READ_ONCE(msk->pm.server_side)) {
> > > > 		flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
> > > > 
> > > > 		/* only set when it is the server side */
> > > > 		if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE,
> > > > 1))
> > > > 			return -EMSGSIZE;
> > > > 	}
> > > > 
> > > > Why don't we just use two 'if' statements here, which will make
> > > > patch 2
> > > > simpler:
> > > > 
> > > >        if (READ_ONCE(msk->pm.server_side)) {
> > > >                /* only set when it is the server side */
> > > >                if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
> > > >                        return -EMSGSIZE;
> > > >        }
> > > > 
> > > > WDYT?
> > > 
> > > I'm not convinced: that might make the patch 2 simpler, but then
> > > the
> > > patch 1 will raise questions from reviewers and developers
> > > looking at
> > > the patches in the order they have been sent: why using 2 'if'
> > > statements while there is no need to use 2.
> > > 
> > > If we want that, I would have to add something in the commit
> > > message.
> > > I
> > > don't think that's worth it.
> > 
> > Sure. Let's keep this patch as is. If so, could you please update
> > the
> > position of the comments in patch 2 and patch 3? Something like:
> > 
> >         /* only set when it is the server side */
> >         if (READ_ONCE(msk->pm.server_side)) {
> >                 flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
> 
> Mmh, I'm not sure: for the flag, it makes sense to only set it when
> it
> is the server side, otherwise that's wrong. Keeping the comment above
> is
> then a bit confusing, no?

If this comment needs to be just above nla_put_u8(), it makes sense to
use 2 'if' here. But up to you.

Thanks,
-Geliang

> 
> > 
> >                 /* Deprecated */
> >                 if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1)) 
> >                         return -EMSGSIZE;
> >         }
> > 
> > Thanks,
> > -Geliang
> > 
> > > 
> > > Cheers,
> > > Matt
> 
> Cheers,
> Matt

Re: [PATCH mptcp-next 1/4] mptcp: pm: netlink: only add server-side attr when true

[Matthieu Baerts]

On 11/09/2025 10:45, Geliang Tang wrote:
> On Thu, 2025-09-11 at 10:37 +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 11/09/2025 10:33, Geliang Tang wrote:
>>> Hi Matt,
>>>
>>> On Thu, 2025-09-11 at 10:20 +0200, Matthieu Baerts wrote:
>>>> Hi Geliang,
>>>>
>>>> Thank you for the review!
>>>>
>>>> On 11/09/2025 10:15, Geliang Tang wrote:
>>>>> Hi Matt,
>>>>>
>>>>> On Tue, 2025-09-09 at 18:30 +0200, Matthieu Baerts (NGI0)
>>>>> wrote:
>>>>>> This attribute is a boolean. No need to add it to set it to
>>>>>> 'false'.
>>>>>>
>>>>>> Indeed, the default value when this attribute is not set is
>>>>>> naturally
>>>>>> 'false'. A few bytes can then be saved by not adding this
>>>>>> attribute
>>>>>> if
>>>>>> the connection is not on the server side.
>>>>>>
>>>>>> This prepares the future deprecation of its attribute, in
>>>>>> favour
>>>>>> of a
>>>>>> new flag.
>>>>>>
>>>>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>>>>> ---
>>>>>>  Documentation/netlink/specs/mptcp_pm.yaml         | 4 ++--
>>>>>>  include/uapi/linux/mptcp_pm.h                     | 4 ++--
>>>>>>  net/mptcp/pm_netlink.c                            | 4 +++-
>>>>>>  tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
>>>>>>  4 files changed, 8 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
>>>>>> b/Documentation/netlink/specs/mptcp_pm.yaml
>>>>>> index
>>>>>> d1b4829b580ad09baf4afd73b67abd7b4ef6883a..fc47a2931014c0304ef
>>>>>> d321
>>>>>> 5cc2
>>>>>> 4485ea22e1ede 100644
>>>>>> --- a/Documentation/netlink/specs/mptcp_pm.yaml
>>>>>> +++ b/Documentation/netlink/specs/mptcp_pm.yaml
>>>>>> @@ -28,13 +28,13 @@ definitions:
>>>>>>            traffic-patterns it can take a long time until the
>>>>>>            MPTCP_EVENT_ESTABLISHED is sent.
>>>>>>            Attributes: token, family, saddr4 | saddr6, daddr4
>>>>>> |
>>>>>> daddr6, sport,
>>>>>> -          dport, server-side, [flags].
>>>>>> +          dport, [server-side], [flags].
>>>>>>        -
>>>>>>          name: established
>>>>>>          doc: >-
>>>>>>            A MPTCP connection is established (can start new
>>>>>> subflows).
>>>>>>            Attributes: token, family, saddr4 | saddr6, daddr4
>>>>>> |
>>>>>> daddr6, sport,
>>>>>> -          dport, server-side, [flags].
>>>>>> +          dport, [server-side], [flags].
>>>>>>        -
>>>>>>          name: closed
>>>>>>          doc: >-
>>>>>> diff --git a/include/uapi/linux/mptcp_pm.h
>>>>>> b/include/uapi/linux/mptcp_pm.h
>>>>>> index
>>>>>> 7359d34da446b94be148b363079120db03ba8549..bf44a5cf5b5a1e6d789
>>>>>> 6326
>>>>>> 82a9
>>>>>> bedbf8090feb9 100644
>>>>>> --- a/include/uapi/linux/mptcp_pm.h
>>>>>> +++ b/include/uapi/linux/mptcp_pm.h
>>>>>> @@ -16,10 +16,10 @@
>>>>>>   *   good time to allocate memory and send ADD_ADDR if
>>>>>> needed.
>>>>>> Depending on the
>>>>>>   *   traffic-patterns it can take a long time until the
>>>>>> MPTCP_EVENT_ESTABLISHED
>>>>>>   *   is sent. Attributes: token, family, saddr4 | saddr6,
>>>>>> daddr4
>>>>>>>
>>>>>> daddr6,
>>>>>> - *   sport, dport, server-side, [flags].
>>>>>> + *   sport, dport, [server-side], [flags].
>>>>>>   * @MPTCP_EVENT_ESTABLISHED: A MPTCP connection is
>>>>>> established
>>>>>> (can
>>>>>> start new
>>>>>>   *   subflows). Attributes: token, family, saddr4 | saddr6,
>>>>>> daddr4 |
>>>>>> daddr6,
>>>>>> - *   sport, dport, server-side, [flags].
>>>>>> + *   sport, dport, [server-side], [flags].
>>>>>>   * @MPTCP_EVENT_CLOSED: A MPTCP connection has stopped.
>>>>>> Attribute:
>>>>>> token.
>>>>>>   * @MPTCP_EVENT_ANNOUNCED: A new address has been announced
>>>>>> by
>>>>>> the
>>>>>> peer.
>>>>>>   *   Attributes: token, rem_id, family, daddr4 | daddr6 [,
>>>>>> dport].
>>>>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>>>>>> index
>>>>>> 483ddbb9ec406a3e965376ee5a833ae295896a02..33a6bf536c020b59717
>>>>>> 472a
>>>>>> ca2d
>>>>>> 38add26255419 100644
>>>>>> --- a/net/mptcp/pm_netlink.c
>>>>>> +++ b/net/mptcp/pm_netlink.c
>>>>>> @@ -413,7 +413,9 @@ static int mptcp_event_created(struct
>>>>>> sk_buff
>>>>>> *skb,
>>>>>>  	if (err)
>>>>>>  		return err;
>>>>>>  
>>>>>> -	if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE,
>>>>>> READ_ONCE(msk-
>>>>>>> pm.server_side)))
>>>>>> +	/* only set when it is the server side */
>>>>>> +	if (READ_ONCE(msk->pm.server_side) &&
>>>>>> +	    nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
>>>>>
>>>>> In patch 2, this will be modified to use two 'if':
>>>>>
>>>>> 	if (READ_ONCE(msk->pm.server_side)) {
>>>>> 		flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
>>>>>
>>>>> 		/* only set when it is the server side */
>>>>> 		if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE,
>>>>> 1))
>>>>> 			return -EMSGSIZE;
>>>>> 	}
>>>>>
>>>>> Why don't we just use two 'if' statements here, which will make
>>>>> patch 2
>>>>> simpler:
>>>>>
>>>>>        if (READ_ONCE(msk->pm.server_side)) {
>>>>>                /* only set when it is the server side */
>>>>>                if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
>>>>>                        return -EMSGSIZE;
>>>>>        }
>>>>>
>>>>> WDYT?
>>>>
>>>> I'm not convinced: that might make the patch 2 simpler, but then
>>>> the
>>>> patch 1 will raise questions from reviewers and developers
>>>> looking at
>>>> the patches in the order they have been sent: why using 2 'if'
>>>> statements while there is no need to use 2.
>>>>
>>>> If we want that, I would have to add something in the commit
>>>> message.
>>>> I
>>>> don't think that's worth it.
>>>
>>> Sure. Let's keep this patch as is. If so, could you please update
>>> the
>>> position of the comments in patch 2 and patch 3? Something like:
>>>
>>>         /* only set when it is the server side */
>>>         if (READ_ONCE(msk->pm.server_side)) {
>>>                 flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
>>
>> Mmh, I'm not sure: for the flag, it makes sense to only set it when
>> it
>> is the server side, otherwise that's wrong. Keeping the comment above
>> is
>> then a bit confusing, no?
> 
> If this comment needs to be just above nla_put_u8(), it makes sense to
> use 2 'if' here. But up to you.

Sorry, I'm lost:

- the comment above is for nla_put_u8(): that's strange to have such
  comment with a flag that is obviously only set for the server side

- we need 2 'if' because there are two different actions here: set flags
  + return error if any, no?

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

Re: [PATCH mptcp-next 1/4] mptcp: pm: netlink: only add server-side attr when true

[Geliang Tang]

On Thu, 2025-09-11 at 11:28 +0200, Matthieu Baerts wrote:
> On 11/09/2025 10:45, Geliang Tang wrote:
> > On Thu, 2025-09-11 at 10:37 +0200, Matthieu Baerts wrote:
> > > Hi Geliang,
> > > 
> > > On 11/09/2025 10:33, Geliang Tang wrote:
> > > > Hi Matt,
> > > > 
> > > > On Thu, 2025-09-11 at 10:20 +0200, Matthieu Baerts wrote:
> > > > > Hi Geliang,
> > > > > 
> > > > > Thank you for the review!
> > > > > 
> > > > > On 11/09/2025 10:15, Geliang Tang wrote:
> > > > > > Hi Matt,
> > > > > > 
> > > > > > On Tue, 2025-09-09 at 18:30 +0200, Matthieu Baerts (NGI0)
> > > > > > wrote:
> > > > > > > This attribute is a boolean. No need to add it to set it
> > > > > > > to
> > > > > > > 'false'.
> > > > > > > 
> > > > > > > Indeed, the default value when this attribute is not set
> > > > > > > is
> > > > > > > naturally
> > > > > > > 'false'. A few bytes can then be saved by not adding this
> > > > > > > attribute
> > > > > > > if
> > > > > > > the connection is not on the server side.
> > > > > > > 
> > > > > > > This prepares the future deprecation of its attribute, in
> > > > > > > favour
> > > > > > > of a
> > > > > > > new flag.
> > > > > > > 
> > > > > > > Signed-off-by: Matthieu Baerts (NGI0)
> > > > > > > <matttbe@kernel.org>
> > > > > > > ---
> > > > > > >  Documentation/netlink/specs/mptcp_pm.yaml         | 4
> > > > > > > ++--
> > > > > > >  include/uapi/linux/mptcp_pm.h                     | 4
> > > > > > > ++--
> > > > > > >  net/mptcp/pm_netlink.c                            | 4
> > > > > > > +++-
> > > > > > >  tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
> > > > > > >  4 files changed, 8 insertions(+), 6 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
> > > > > > > b/Documentation/netlink/specs/mptcp_pm.yaml
> > > > > > > index
> > > > > > > d1b4829b580ad09baf4afd73b67abd7b4ef6883a..fc47a2931014c03
> > > > > > > 04ef
> > > > > > > d321
> > > > > > > 5cc2
> > > > > > > 4485ea22e1ede 100644
> > > > > > > --- a/Documentation/netlink/specs/mptcp_pm.yaml
> > > > > > > +++ b/Documentation/netlink/specs/mptcp_pm.yaml
> > > > > > > @@ -28,13 +28,13 @@ definitions:
> > > > > > >            traffic-patterns it can take a long time until
> > > > > > > the
> > > > > > >            MPTCP_EVENT_ESTABLISHED is sent.
> > > > > > >            Attributes: token, family, saddr4 | saddr6,
> > > > > > > daddr4
> > > > > > > > 
> > > > > > > daddr6, sport,
> > > > > > > -          dport, server-side, [flags].
> > > > > > > +          dport, [server-side], [flags].
> > > > > > >        -
> > > > > > >          name: established
> > > > > > >          doc: >-
> > > > > > >            A MPTCP connection is established (can start
> > > > > > > new
> > > > > > > subflows).
> > > > > > >            Attributes: token, family, saddr4 | saddr6,
> > > > > > > daddr4
> > > > > > > > 
> > > > > > > daddr6, sport,
> > > > > > > -          dport, server-side, [flags].
> > > > > > > +          dport, [server-side], [flags].
> > > > > > >        -
> > > > > > >          name: closed
> > > > > > >          doc: >-
> > > > > > > diff --git a/include/uapi/linux/mptcp_pm.h
> > > > > > > b/include/uapi/linux/mptcp_pm.h
> > > > > > > index
> > > > > > > 7359d34da446b94be148b363079120db03ba8549..bf44a5cf5b5a1e6
> > > > > > > d789
> > > > > > > 6326
> > > > > > > 82a9
> > > > > > > bedbf8090feb9 100644
> > > > > > > --- a/include/uapi/linux/mptcp_pm.h
> > > > > > > +++ b/include/uapi/linux/mptcp_pm.h
> > > > > > > @@ -16,10 +16,10 @@
> > > > > > >   *   good time to allocate memory and send ADD_ADDR if
> > > > > > > needed.
> > > > > > > Depending on the
> > > > > > >   *   traffic-patterns it can take a long time until the
> > > > > > > MPTCP_EVENT_ESTABLISHED
> > > > > > >   *   is sent. Attributes: token, family, saddr4 |
> > > > > > > saddr6,
> > > > > > > daddr4
> > > > > > > > 
> > > > > > > daddr6,
> > > > > > > - *   sport, dport, server-side, [flags].
> > > > > > > + *   sport, dport, [server-side], [flags].
> > > > > > >   * @MPTCP_EVENT_ESTABLISHED: A MPTCP connection is
> > > > > > > established
> > > > > > > (can
> > > > > > > start new
> > > > > > >   *   subflows). Attributes: token, family, saddr4 |
> > > > > > > saddr6,
> > > > > > > daddr4 |
> > > > > > > daddr6,
> > > > > > > - *   sport, dport, server-side, [flags].
> > > > > > > + *   sport, dport, [server-side], [flags].
> > > > > > >   * @MPTCP_EVENT_CLOSED: A MPTCP connection has stopped.
> > > > > > > Attribute:
> > > > > > > token.
> > > > > > >   * @MPTCP_EVENT_ANNOUNCED: A new address has been
> > > > > > > announced
> > > > > > > by
> > > > > > > the
> > > > > > > peer.
> > > > > > >   *   Attributes: token, rem_id, family, daddr4 | daddr6
> > > > > > > [,
> > > > > > > dport].
> > > > > > > diff --git a/net/mptcp/pm_netlink.c
> > > > > > > b/net/mptcp/pm_netlink.c
> > > > > > > index
> > > > > > > 483ddbb9ec406a3e965376ee5a833ae295896a02..33a6bf536c020b5
> > > > > > > 9717
> > > > > > > 472a
> > > > > > > ca2d
> > > > > > > 38add26255419 100644
> > > > > > > --- a/net/mptcp/pm_netlink.c
> > > > > > > +++ b/net/mptcp/pm_netlink.c
> > > > > > > @@ -413,7 +413,9 @@ static int mptcp_event_created(struct
> > > > > > > sk_buff
> > > > > > > *skb,
> > > > > > >  	if (err)
> > > > > > >  		return err;
> > > > > > >  
> > > > > > > -	if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE,
> > > > > > > READ_ONCE(msk-
> > > > > > > > pm.server_side)))
> > > > > > > +	/* only set when it is the server side */
> > > > > > > +	if (READ_ONCE(msk->pm.server_side) &&
> > > > > > > +	    nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
> > > > > > 
> > > > > > In patch 2, this will be modified to use two 'if':
> > > > > > 
> > > > > > 	if (READ_ONCE(msk->pm.server_side)) {
> > > > > > 		flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
> > > > > > 
> > > > > > 		/* only set when it is the server side */
> > > > > > 		if (nla_put_u8(skb,
> > > > > > MPTCP_ATTR_SERVER_SIDE,
> > > > > > 1))
> > > > > > 			return -EMSGSIZE;
> > > > > > 	}
> > > > > > 
> > > > > > Why don't we just use two 'if' statements here, which will
> > > > > > make
> > > > > > patch 2
> > > > > > simpler:
> > > > > > 
> > > > > >        if (READ_ONCE(msk->pm.server_side)) {
> > > > > >                /* only set when it is the server side */
> > > > > >                if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE,
> > > > > > 1))
> > > > > >                        return -EMSGSIZE;
> > > > > >        }
> > > > > > 
> > > > > > WDYT?
> > > > > 
> > > > > I'm not convinced: that might make the patch 2 simpler, but
> > > > > then
> > > > > the
> > > > > patch 1 will raise questions from reviewers and developers
> > > > > looking at
> > > > > the patches in the order they have been sent: why using 2
> > > > > 'if'
> > > > > statements while there is no need to use 2.
> > > > > 
> > > > > If we want that, I would have to add something in the commit
> > > > > message.
> > > > > I
> > > > > don't think that's worth it.
> > > > 
> > > > Sure. Let's keep this patch as is. If so, could you please
> > > > update
> > > > the
> > > > position of the comments in patch 2 and patch 3? Something
> > > > like:
> > > > 
> > > >         /* only set when it is the server side */
> > > >         if (READ_ONCE(msk->pm.server_side)) {
> > > >                 flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
> > > 
> > > Mmh, I'm not sure: for the flag, it makes sense to only set it
> > > when
> > > it
> > > is the server side, otherwise that's wrong. Keeping the comment
> > > above
> > > is
> > > then a bit confusing, no?
> > 
> > If this comment needs to be just above nla_put_u8(), it makes sense
> > to
> > use 2 'if' here. But up to you.
> 
> Sorry, I'm lost:
> 
> - the comment above is for nla_put_u8(): that's strange to have such
>   comment with a flag that is obviously only set for the server side
> 
> - we need 2 'if' because there are two different actions here: set
> flags
>   + return error if any, no?

Sorry for the confusion, let's keep them as is.

Thanks,
-Geliang

> 
> Cheers,
> Matt

[PATCH mptcp-next 2/4] mptcp: pm: netlink: announce server-side flag

[Matthieu Baerts (NGI0)]

Now that the 'flags' attribute is used, it seems interesting to add one
flag for 'server-side', a boolean value.

This is duplicating the info from the dedicated 'server-side' attribute,
but it will be deprecated in the next commit, and removed in a few
versions.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 include/uapi/linux/mptcp.h |  1 +
 net/mptcp/pm_netlink.c     | 11 +++++++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 5fd5b4cf75ca1e0099e0effa351507d3ab002f1e..95d621f6d59810126cbc37b1d6baf896a40dd9bc 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -32,6 +32,7 @@
 #define MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED	_BITUL(1)
 
 #define MPTCP_PM_EV_FLAG_DENY_JOIN_ID0		_BITUL(0)
+#define MPTCP_PM_EV_FLAG_SERVER_SIDE		_BITUL(1)
 
 #define MPTCP_PM_ADDR_FLAG_SIGNAL                      (1 << 0)
 #define MPTCP_PM_ADDR_FLAG_SUBFLOW                     (1 << 1)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 33a6bf536c020b59717472aca2d38add26255419..aa0c73faaa6acad3fd66ea0942726ecd4d0abcc0 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -413,10 +413,13 @@ static int mptcp_event_created(struct sk_buff *skb,
 	if (err)
 		return err;
 
-	/* only set when it is the server side */
-	if (READ_ONCE(msk->pm.server_side) &&
-	    nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
-		return -EMSGSIZE;
+	if (READ_ONCE(msk->pm.server_side)) {
+		flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
+
+		/* only set when it is the server side */
+		if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
+			return -EMSGSIZE;
+	}
 
 	if (READ_ONCE(msk->pm.remote_deny_join_id0))
 		flags |= MPTCP_PM_EV_FLAG_DENY_JOIN_ID0;

-- 
2.51.0

[PATCH mptcp-next 3/4] mptcp: pm: netlink: deprecate server-side attribute

[Matthieu Baerts (NGI0)]

Now that such info is in the 'flags' attribute, it is time to deprecate
the dedicated 'server-side' attribute.

It will be removed in a few versions.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 Documentation/netlink/specs/mptcp_pm.yaml | 1 +
 net/mptcp/pm_netlink.c                    | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
index fc47a2931014c0304efd3215cc24485ea22e1ede..ba30a40b9dbf2d2d4f25cc07b309ea560712f65e 100644
--- a/Documentation/netlink/specs/mptcp_pm.yaml
+++ b/Documentation/netlink/specs/mptcp_pm.yaml
@@ -266,6 +266,7 @@ attribute-sets:
       -
         name: server-side
         type: u8
+        doc: "Deprecated: use 'flags'"
 
 operations:
   list:
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index aa0c73faaa6acad3fd66ea0942726ecd4d0abcc0..d5b383870f79956ce5e10bf384695621604f3ce9 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -416,7 +416,7 @@ static int mptcp_event_created(struct sk_buff *skb,
 	if (READ_ONCE(msk->pm.server_side)) {
 		flags |= MPTCP_PM_EV_FLAG_SERVER_SIDE;
 
-		/* only set when it is the server side */
+		/* Deprecated, and only set when it is the server side */
 		if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, 1))
 			return -EMSGSIZE;
 	}

-- 
2.51.0

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

[Matthieu Baerts (NGI0)]

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

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

[Geliang Tang]

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);
>  

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

[Matthieu Baerts]

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.

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

[Geliang Tang]

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

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

[Matthieu Baerts]

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.

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

[Geliang Tang]

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

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

[Matthieu Baerts]

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.

Re: [PATCH mptcp-next 0/4] mptcp: pm: netlink: announce server-side flag

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Unstable: 2 failed test(s): packetdrill_dss selftest_mptcp_connect_checksum 🔴
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/17590195633

Initiator: Matthieu Baerts (NGI0)
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/058da6352a3c
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1000598


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next 0/4] mptcp: pm: netlink: announce server-side flag

[Geliang Tang]

Hi Matt,

Thanks for this new patchset. It looks good to me overall. I have two
comments in patch 1 and patch 4.

-Geliang

On Tue, 2025-09-09 at 18:30 +0200, Matthieu Baerts (NGI0) wrote:
> Now that the 'flags' attribute is used, it seems interesting to add
> one
> flag for 'server-side', a boolean value.
> 
> Here are a few patches related to the 'server-side' attribute:
> 
> - Patch 1: only announce this attribute on the server side.
> 
> - Patch 2: announce the 'server-side' flag when this is the case.
> 
> - Patch 3: deprecate the 'server-side' attribute.
> 
> - Patch 4: use the 'server-side' flag in the selftests.
> 
> Based-on:
> <20250909-mptcp-pm-user-c-flag-v2-0-a6f9542481c5@kernel.org>
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Matthieu Baerts (NGI0) (4):
>       mptcp: pm: netlink: only add server-side attr when true
>       mptcp: pm: netlink: announce server-side flag
>       mptcp: pm: netlink: deprecate server-side attribute
>       selftests: mptcp: pm: get server-side flag
> 
>  Documentation/netlink/specs/mptcp_pm.yaml         | 5 +++--
>  include/uapi/linux/mptcp.h                        | 1 +
>  include/uapi/linux/mptcp_pm.h                     | 4 ++--
>  net/mptcp/pm_netlink.c                            | 9 +++++++--
>  tools/testing/selftests/net/mptcp/pm_nl_ctl.c     | 9 ++++++++-
>  tools/testing/selftests/net/mptcp/userspace_pm.sh | 2 +-
>  6 files changed, 22 insertions(+), 8 deletions(-)
> ---
> base-commit: 12e66f7051b4b535a990cd20c149d76a98ec68c3
> change-id: 20250909-mptcp-pm-user-server-side-flag-214c84b90b21
> prerequisite-change-id: 20250720-mptcp-pm-user-c-flag-a7d5d7a00c6f:v2
> prerequisite-patch-id: cc65204dc8620f1675f8b8cbdf6dc0fecf20379e
> prerequisite-patch-id: 88640c51fc66f4d630e2f99db72c18830273fc83
> prerequisite-patch-id: 5773e6ceede07a2c43550774eddad1c32cde67e0
> prerequisite-patch-id: f9196f35eff69835799db4312e82c795f1b15eb0
> prerequisite-patch-id: 8ec26f3fe4baa5128ac2f01f7284184bbb0e651b
> 
> Best regards,