1. Patchew
  2. MPTCP
  3. [PATCH mptcp-net 0/7] mptcp: pm: nl: announce deny-join-id0 attribute
  4. View patch

[PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr

Matthieu Baerts (NGI0) posted 7 patches 2 weeks, 4 days ago
There is a newer version of this series
[PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr
Posted by Matthieu Baerts (NGI0) 2 weeks, 4 days ago 
During the connection establishment, a peer can tell the other that it
cannot establish new subflows to the initial IP address and port by
setting the 'C' flag [1]. The RFC8684 is strict about that:

  (...) therefore the receiver MUST NOT try to open any additional
  subflows toward this address and port.

It is then important not to let the userspace PM establishing such
subflows, and return an error (ECONNREFUSED) if it tries to do so.

Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6 [1]
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_userspace.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	if (err < 0)
 		goto create_err;
 
+	if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {
+		GENL_SET_ERR_MSG(info, "deny join id0");
+		err = -ECONNREFUSED;
+		goto create_err;
+	}
+
 	if (!mptcp_pm_addr_families_match(sk, &entry.addr, &addr_r)) {
 		GENL_SET_ERR_MSG(info, "families mismatch");
 		err = -EINVAL;

-- 
2.50.1
Re: [PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr
Posted by Christoph Paasch 2 weeks, 4 days ago 
On Fri, Aug 29, 2025 at 1:33 PM Matthieu Baerts (NGI0)
<matttbe@kernel.org> wrote:
>
> During the connection establishment, a peer can tell the other that it
> cannot establish new subflows to the initial IP address and port by
> setting the 'C' flag [1]. The RFC8684 is strict about that:
>
>   (...) therefore the receiver MUST NOT try to open any additional
>   subflows toward this address and port.
>
> It is then important not to let the userspace PM establishing such
> subflows, and return an error (ECONNREFUSED) if it tries to do so.
>
> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
> Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6 [1]
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm_userspace.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
>         if (err < 0)
>                 goto create_err;
>
> +       if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {

If userspace wants to, it could still work around that by still using
the initial subflow's IP-address and just use a non-zero address-ID.

Is that really a concern ? Sure, the IETF-draft says "MUST NOT", and
so, shouldn't we leave it up to the userspace PM to make that decision
?


Christoph

> +               GENL_SET_ERR_MSG(info, "deny join id0");
> +               err = -ECONNREFUSED;
> +               goto create_err;
> +       }
> +
>         if (!mptcp_pm_addr_families_match(sk, &entry.addr, &addr_r)) {
>                 GENL_SET_ERR_MSG(info, "families mismatch");
>                 err = -EINVAL;
>
> --
> 2.50.1
>
>
Re: [PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr
Posted by Mat Martineau 2 weeks, 4 days ago 
On Fri, 29 Aug 2025, Christoph Paasch wrote:

> On Fri, Aug 29, 2025 at 1:33 PM Matthieu Baerts (NGI0)
> <matttbe@kernel.org> wrote:
>>
>> During the connection establishment, a peer can tell the other that it
>> cannot establish new subflows to the initial IP address and port by
>> setting the 'C' flag [1]. The RFC8684 is strict about that:
>>
>>   (...) therefore the receiver MUST NOT try to open any additional
>>   subflows toward this address and port.
>>
>> It is then important not to let the userspace PM establishing such
>> subflows, and return an error (ECONNREFUSED) if it tries to do so.
>>
>> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
>> Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6 [1]
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  net/mptcp/pm_userspace.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
>> index 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
>> --- a/net/mptcp/pm_userspace.c
>> +++ b/net/mptcp/pm_userspace.c
>> @@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
>>         if (err < 0)
>>                 goto create_err;
>>
>> +       if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {
>
> If userspace wants to, it could still work around that by still using
> the initial subflow's IP-address and just use a non-zero address-ID.
>
> Is that really a concern ? Sure, the IETF-draft says "MUST NOT", and
> so, shouldn't we leave it up to the userspace PM to make that decision
> ?
>

I concur with Christoph on this one, I think the PM (userspace or kernel) 
is responsible for this aspect of the protocol so it doesn't need to be 
double-checked here.

- Mat

>
>> +               GENL_SET_ERR_MSG(info, "deny join id0");
>> +               err = -ECONNREFUSED;
>> +               goto create_err;
>> +       }
>> +
>>         if (!mptcp_pm_addr_families_match(sk, &entry.addr, &addr_r)) {
>>                 GENL_SET_ERR_MSG(info, "families mismatch");
>>                 err = -EINVAL;
>>
>> --
>> 2.50.1
>>
>>
>
>
Re: [PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr
Posted by Matthieu Baerts 2 weeks, 2 days ago 
Hi Christoph, Mat,

On 30/08/2025 02:58, Mat Martineau wrote:
> On Fri, 29 Aug 2025, Christoph Paasch wrote:
> 
>> On Fri, Aug 29, 2025 at 1:33 PM Matthieu Baerts (NGI0)
>> <matttbe@kernel.org> wrote:
>>>
>>> During the connection establishment, a peer can tell the other that it
>>> cannot establish new subflows to the initial IP address and port by
>>> setting the 'C' flag [1]. The RFC8684 is strict about that:
>>>
>>>   (...) therefore the receiver MUST NOT try to open any additional
>>>   subflows toward this address and port.
>>>
>>> It is then important not to let the userspace PM establishing such
>>> subflows, and return an error (ECONNREFUSED) if it tries to do so.
>>>
>>> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow
>>> establishment")
>>> Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6
>>> [1]
>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>> ---
>>>  net/mptcp/pm_userspace.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
>>> index
>>> 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
>>> --- a/net/mptcp/pm_userspace.c
>>> +++ b/net/mptcp/pm_userspace.c
>>> @@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct
>>> sk_buff *skb, struct genl_info *info)
>>>         if (err < 0)
>>>                 goto create_err;
>>>
>>> +       if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {
>>
>> If userspace wants to, it could still work around that by still using
>> the initial subflow's IP-address and just use a non-zero address-ID.

Good point, I missed that!

>> Is that really a concern ? Sure, the IETF-draft says "MUST NOT", and
>> so, shouldn't we leave it up to the userspace PM to make that decision
>> ?
>>
> 
> I concur with Christoph on this one, I think the PM (userspace or
> kernel) is responsible for this aspect of the protocol so it doesn't
> need to be double-checked here.

Indeed, we could leave it up to the different PMs to make that decision.
Initially, I didn't have this patch, and I had a simple patch setting
the corresponding sysctl to 0 in userspace_pm.sh, just to check the
event was correctly announced. But when writing the last patch's commit
message ("mptcp: pm: nl: announce deny-join-id0 attribute"), I realised
that the main missing feature was a way for the userspace daemon to know
that the C flag was set. We cannot backport patch 7/7, but we can
backport this one, and have a way to know this info on older kernels. I
should have mentioned that in the commit message.

In this case, I think we have a few options:

- Keep this patch, but update the commit message to make it clearer
what's its purpose, see [1]

- Same, but revert it after patch 7/7 on net-next.

- Keep it, but also check for the address and port.

- Drop it.

What would you prefer?

Personally, I think we should keep it, at least for stable, but we don't
need to check for the address and port: even if it is easy to check, it
might be good to let a way for the userspace to still create such subflow.

[1]: The last paragraph in this commit message would then be:

> It is then important to let the userspace daemon know it is not allowed
> to establish such subflows, and return an error (ECONNREFUSED) if it
> tries to do so. Note that the userspace daemon will still be able to
> establish subflows to the initial IP address and port, but by giving a
> different remote ID: that's OK, that's its responsibility ; worst case,
> the MP_JOIN is rejected by the other peer.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr
Posted by Christoph Paasch 2 weeks ago 
On Mon, Sep 1, 2025 at 3:08 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi Christoph, Mat,
>
> On 30/08/2025 02:58, Mat Martineau wrote:
> > On Fri, 29 Aug 2025, Christoph Paasch wrote:
> >
> >> On Fri, Aug 29, 2025 at 1:33 PM Matthieu Baerts (NGI0)
> >> <matttbe@kernel.org> wrote:
> >>>
> >>> During the connection establishment, a peer can tell the other that it
> >>> cannot establish new subflows to the initial IP address and port by
> >>> setting the 'C' flag [1]. The RFC8684 is strict about that:
> >>>
> >>>   (...) therefore the receiver MUST NOT try to open any additional
> >>>   subflows toward this address and port.
> >>>
> >>> It is then important not to let the userspace PM establishing such
> >>> subflows, and return an error (ECONNREFUSED) if it tries to do so.
> >>>
> >>> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow
> >>> establishment")
> >>> Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6
> >>> [1]
> >>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> >>> ---
> >>>  net/mptcp/pm_userspace.c | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> >>> index
> >>> 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
> >>> --- a/net/mptcp/pm_userspace.c
> >>> +++ b/net/mptcp/pm_userspace.c
> >>> @@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct
> >>> sk_buff *skb, struct genl_info *info)
> >>>         if (err < 0)
> >>>                 goto create_err;
> >>>
> >>> +       if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {
> >>
> >> If userspace wants to, it could still work around that by still using
> >> the initial subflow's IP-address and just use a non-zero address-ID.
>
> Good point, I missed that!
>
> >> Is that really a concern ? Sure, the IETF-draft says "MUST NOT", and
> >> so, shouldn't we leave it up to the userspace PM to make that decision
> >> ?
> >>
> >
> > I concur with Christoph on this one, I think the PM (userspace or
> > kernel) is responsible for this aspect of the protocol so it doesn't
> > need to be double-checked here.
>
> Indeed, we could leave it up to the different PMs to make that decision.
> Initially, I didn't have this patch, and I had a simple patch setting
> the corresponding sysctl to 0 in userspace_pm.sh, just to check the
> event was correctly announced. But when writing the last patch's commit
> message ("mptcp: pm: nl: announce deny-join-id0 attribute"), I realised
> that the main missing feature was a way for the userspace daemon to know
> that the C flag was set. We cannot backport patch 7/7

Why not ? Yes, it is not typical to backport a new netlink message,
but I think it makes sense here.
It basically means that the fix involves not only the kernel but also
the userspace PMs.

> , but we can
> backport this one, and have a way to know this info on older kernels. I
> should have mentioned that in the commit message.

The problem is that even this here is not complete, right ? A
userspace pm can still attempt to connect to the IP of the initial
subflow simply by using another address-ID.


Christoph

> In this case, I think we have a few options:
>
> - Keep this patch, but update the commit message to make it clearer
> what's its purpose, see [1]
>
> - Same, but revert it after patch 7/7 on net-next.
>
> - Keep it, but also check for the address and port.
>
> - Drop it.
>
> What would you prefer?
>
> Personally, I think we should keep it, at least for stable, but we don't
> need to check for the address and port: even if it is easy to check, it
> might be good to let a way for the userspace to still create such subflow.
>
> [1]: The last paragraph in this commit message would then be:
>
> > It is then important to let the userspace daemon know it is not allowed
> > to establish such subflows, and return an error (ECONNREFUSED) if it
> > tries to do so. Note that the userspace daemon will still be able to
> > establish subflows to the initial IP address and port, but by giving a
> > different remote ID: that's OK, that's its responsibility ; worst case,
> > the MP_JOIN is rejected by the other peer.
>
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
>
Re: [PATCH mptcp-net 2/7] mptcp: pm: userspace: respect deny_join_id0 attr
Posted by Matthieu Baerts 2 weeks ago 
On 02/09/2025 18:43, Christoph Paasch wrote:
> On Mon, Sep 1, 2025 at 3:08 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>>
>> Hi Christoph, Mat,
>>
>> On 30/08/2025 02:58, Mat Martineau wrote:
>>> On Fri, 29 Aug 2025, Christoph Paasch wrote:
>>>
>>>> On Fri, Aug 29, 2025 at 1:33 PM Matthieu Baerts (NGI0)
>>>> <matttbe@kernel.org> wrote:
>>>>>
>>>>> During the connection establishment, a peer can tell the other that it
>>>>> cannot establish new subflows to the initial IP address and port by
>>>>> setting the 'C' flag [1]. The RFC8684 is strict about that:
>>>>>
>>>>>   (...) therefore the receiver MUST NOT try to open any additional
>>>>>   subflows toward this address and port.
>>>>>
>>>>> It is then important not to let the userspace PM establishing such
>>>>> subflows, and return an error (ECONNREFUSED) if it tries to do so.
>>>>>
>>>>> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow
>>>>> establishment")
>>>>> Link: https://datatracker.ietf.org/doc/html/rfc8684\#section-3.1-20.6
>>>>> [1]
>>>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>>>> ---
>>>>>  net/mptcp/pm_userspace.c | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
>>>>> index
>>>>> 1911fe1799fa38a53381247a830a9a0daf1c4492..9968dc9a8b45112114953f66848ea22c971136d6 100644
>>>>> --- a/net/mptcp/pm_userspace.c
>>>>> +++ b/net/mptcp/pm_userspace.c
>>>>> @@ -391,6 +391,12 @@ int mptcp_pm_nl_subflow_create_doit(struct
>>>>> sk_buff *skb, struct genl_info *info)
>>>>>         if (err < 0)
>>>>>                 goto create_err;
>>>>>
>>>>> +       if (READ_ONCE(msk->pm.remote_deny_join_id0) && addr_r.id == 0) {
>>>>
>>>> If userspace wants to, it could still work around that by still using
>>>> the initial subflow's IP-address and just use a non-zero address-ID.
>>
>> Good point, I missed that!
>>
>>>> Is that really a concern ? Sure, the IETF-draft says "MUST NOT", and
>>>> so, shouldn't we leave it up to the userspace PM to make that decision
>>>> ?
>>>>
>>>
>>> I concur with Christoph on this one, I think the PM (userspace or
>>> kernel) is responsible for this aspect of the protocol so it doesn't
>>> need to be double-checked here.
>>
>> Indeed, we could leave it up to the different PMs to make that decision.
>> Initially, I didn't have this patch, and I had a simple patch setting
>> the corresponding sysctl to 0 in userspace_pm.sh, just to check the
>> event was correctly announced. But when writing the last patch's commit
>> message ("mptcp: pm: nl: announce deny-join-id0 attribute"), I realised
>> that the main missing feature was a way for the userspace daemon to know
>> that the C flag was set. We cannot backport patch 7/7
> 
> Why not ? Yes, it is not typical to backport a new netlink message,
> but I think it makes sense here.
> It basically means that the fix involves not only the kernel but also
> the userspace PMs.

Yes, I guess we could backport it.

I still don't know if we should use the (existing but not used) "flags"
attribute, or a new dedicated one.

>> , but we can
>> backport this one, and have a way to know this info on older kernels. I
>> should have mentioned that in the commit message.
> 
> The problem is that even this here is not complete, right ? A
> userspace pm can still attempt to connect to the IP of the initial
> subflow simply by using another address-ID.

Yes, but that's not an issue for me: that's because the userspace PM
didn't use the expected ID, so it is the userspace PM daemon
responsibility :)

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.