[PATCH mptcp-net] mptcp: pm: in-kernel: always mark signal+subflow endp as used

Matthieu Baerts (NGI0) posted 1 patch 1 week, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20260218-mptcp-issue-613-v1-1-f8e9adb12010@kernel.org
net/mptcp/pm_kernel.c | 9 +++++++++
1 file changed, 9 insertions(+)
[PATCH mptcp-net] mptcp: pm: in-kernel: always mark signal+subflow endp as used
Posted by Matthieu Baerts (NGI0) 1 week, 4 days ago
Syzkaller managed to find a combination of actions that was generating
this warning:

  msk->pm.local_addr_used == 0
  WARNING: net/mptcp/pm_kernel.c:1071 at __mark_subflow_endp_available net/mptcp/pm_kernel.c:1071 [inline], CPU#1: syz.2.17/961
  WARNING: net/mptcp/pm_kernel.c:1071 at mptcp_nl_remove_subflow_and_signal_addr net/mptcp/pm_kernel.c:1103 [inline], CPU#1: syz.2.17/961
  WARNING: net/mptcp/pm_kernel.c:1071 at mptcp_pm_nl_del_addr_doit+0x81d/0x8f0 net/mptcp/pm_kernel.c:1210, CPU#1: syz.2.17/961
  Modules linked in:
  CPU: 1 UID: 0 PID: 961 Comm: syz.2.17 Not tainted 6.19.0-08368-gfafda3b4b06b #22 PREEMPT(full)
  Hardware name: QEMU Ubuntu 25.10 PC v2 (i440FX + PIIX, + 10.1 machine, 1996), BIOS 1.17.0-debian-1.17.0-1build1 04/01/2014
  RIP: 0010:__mark_subflow_endp_available net/mptcp/pm_kernel.c:1071 [inline]
  RIP: 0010:mptcp_nl_remove_subflow_and_signal_addr net/mptcp/pm_kernel.c:1103 [inline]
  RIP: 0010:mptcp_pm_nl_del_addr_doit+0x81d/0x8f0 net/mptcp/pm_kernel.c:1210
  Code: 89 c5 e8 46 30 6f fe e9 21 fd ff ff 49 83 ed 80 e8 38 30 6f fe 4c 89 ef be 03 00 00 00 e8 db 49 df fe eb ac e8 24 30 6f fe 90 <0f> 0b 90 e9 1d ff ff ff e8 16 30 6f fe eb 05 e8 0f 30 6f fe e8 9a
  RSP: 0018:ffffc90001663880 EFLAGS: 00010293
  RAX: ffffffff82de1a6c RBX: 0000000000000000 RCX: ffff88800722b500
  RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
  RBP: ffff8880158b22d0 R08: 0000000000010425 R09: ffffffffffffffff
  R10: ffffffff82de18ba R11: 0000000000000000 R12: ffff88800641a640
  R13: ffff8880158b1880 R14: ffff88801ec3c900 R15: ffff88800641a650
  FS:  00005555722c3500(0000) GS:ffff8880f909d000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 00007f66346e0f60 CR3: 000000001607c000 CR4: 0000000000350ef0
  Call Trace:
   <TASK>
   genl_family_rcv_msg_doit+0x117/0x180 net/netlink/genetlink.c:1115
   genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
   genl_rcv_msg+0x3a8/0x3f0 net/netlink/genetlink.c:1210
   netlink_rcv_skb+0x16d/0x240 net/netlink/af_netlink.c:2550
   genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
   netlink_unicast_kernel net/netlink/af_netlink.c:1318 [inline]
   netlink_unicast+0x3e9/0x4c0 net/netlink/af_netlink.c:1344
   netlink_sendmsg+0x4aa/0x5b0 net/netlink/af_netlink.c:1894
   sock_sendmsg_nosec net/socket.c:727 [inline]
   __sock_sendmsg+0xc9/0xf0 net/socket.c:742
   ____sys_sendmsg+0x272/0x3b0 net/socket.c:2592
   ___sys_sendmsg+0x2de/0x320 net/socket.c:2646
   __sys_sendmsg net/socket.c:2678 [inline]
   __do_sys_sendmsg net/socket.c:2683 [inline]
   __se_sys_sendmsg net/socket.c:2681 [inline]
   __x64_sys_sendmsg+0x110/0x1a0 net/socket.c:2681
   do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
   do_syscall_64+0x143/0x440 arch/x86/entry/syscall_64.c:94
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
  RIP: 0033:0x7f66346f826d
  Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
  RSP: 002b:00007ffc83d8bdc8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
  RAX: ffffffffffffffda RBX: 00007f6634985fa0 RCX: 00007f66346f826d
  RDX: 00000000040000b0 RSI: 0000200000000740 RDI: 0000000000000007
  RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
  R10: 0000000000000000 R11: 0000000000000246 R12: 00007f6634985fa8
  R13: 00007f6634985fac R14: 0000000000000000 R15: 0000000000001770
   </TASK>

The actions that caused that seem to be:

 - Set the MPTCP subflows limit to 0
 - Create an MPTCP endpoint with both the 'signal' and 'subflow' flags
 - Create a new MPTCP connection from a different address: an ADD_ADDR
   linked to the MPTCP endpoint will be sent ('signal' flag), but no
   subflows is initiated ('subflow' flag)
 - Remove the MPTCP endpoint

In this case, msk->pm.local_addr_used has been kept to 0 -- because no
subflows have been created -- but the corresponding bit in
msk->pm.id_avail_bitmap has been cleared when the ADD_ADDR has been
sent. This later causes a splat when removing the MPTCP endpoint because
msk->pm.local_addr_used has been kept to 0.

Now, if an endpoint has both the signal and subflow flags, but it is not
possible to create subflows because of the limits or the c-flag case,
then the local endpoint counter is still incremented: the endpoint is
used at the end. This avoids issues later when removing the endpoint and
calling __mark_subflow_endp_available(), which expects
msk->pm.local_addr_used to have been previously incremented if the
endpoint was marked as used according to msk->pm.id_avail_bitmap.

Note that signal_and_subflow variable is reset to false when the limits
and the c-flag case allows subflows creation. Also, local_addr_used is
only incremented for non ID0 subflows.

Fixes: 85df533a787b ("mptcp: pm: do not ignore 'subflow' if 'signal' flag is also set")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/613
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_kernel.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 87e37c729f81..2710abcd4065 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -418,6 +418,15 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 	}
 
 exit:
+	/* If an endpoint has both the signal and subflow flags, but it is not
+	 * possible to create subflows because of the limits or the c-flag case,
+	 * then still mark the endp as used, which is the case. This avoids
+	 * issues later when removing the endpoint and calling
+	 * __mark_subflow_endp_available(), which expects the increment here.
+	 */
+	if (signal_and_subflow && local.addr.id != msk->mpc_endpoint_id)
+		msk->pm.local_addr_used++;
+
 	mptcp_pm_nl_check_work_pending(msk);
 }
 

---
base-commit: eb01783b4b05934951774780509d96f7fe64c4a3
change-id: 20260218-mptcp-issue-613-0ad1b55ca18c

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>
Re: [PATCH mptcp-net] mptcp: pm: in-kernel: always mark signal+subflow endp as used
Posted by Mat Martineau 4 days, 10 hours ago
On Wed, 18 Feb 2026, Matthieu Baerts (NGI0) wrote:

> Syzkaller managed to find a combination of actions that was generating
> this warning:
>
>  msk->pm.local_addr_used == 0
>  WARNING: net/mptcp/pm_kernel.c:1071 at __mark_subflow_endp_available net/mptcp/pm_kernel.c:1071 [inline], CPU#1: syz.2.17/961
>  WARNING: net/mptcp/pm_kernel.c:1071 at mptcp_nl_remove_subflow_and_signal_addr net/mptcp/pm_kernel.c:1103 [inline], CPU#1: syz.2.17/961
>  WARNING: net/mptcp/pm_kernel.c:1071 at mptcp_pm_nl_del_addr_doit+0x81d/0x8f0 net/mptcp/pm_kernel.c:1210, CPU#1: syz.2.17/961
>  Modules linked in:
>  CPU: 1 UID: 0 PID: 961 Comm: syz.2.17 Not tainted 6.19.0-08368-gfafda3b4b06b #22 PREEMPT(full)
>  Hardware name: QEMU Ubuntu 25.10 PC v2 (i440FX + PIIX, + 10.1 machine, 1996), BIOS 1.17.0-debian-1.17.0-1build1 04/01/2014
>  RIP: 0010:__mark_subflow_endp_available net/mptcp/pm_kernel.c:1071 [inline]
>  RIP: 0010:mptcp_nl_remove_subflow_and_signal_addr net/mptcp/pm_kernel.c:1103 [inline]
>  RIP: 0010:mptcp_pm_nl_del_addr_doit+0x81d/0x8f0 net/mptcp/pm_kernel.c:1210
>  Code: 89 c5 e8 46 30 6f fe e9 21 fd ff ff 49 83 ed 80 e8 38 30 6f fe 4c 89 ef be 03 00 00 00 e8 db 49 df fe eb ac e8 24 30 6f fe 90 <0f> 0b 90 e9 1d ff ff ff e8 16 30 6f fe eb 05 e8 0f 30 6f fe e8 9a
>  RSP: 0018:ffffc90001663880 EFLAGS: 00010293
>  RAX: ffffffff82de1a6c RBX: 0000000000000000 RCX: ffff88800722b500
>  RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
>  RBP: ffff8880158b22d0 R08: 0000000000010425 R09: ffffffffffffffff
>  R10: ffffffff82de18ba R11: 0000000000000000 R12: ffff88800641a640
>  R13: ffff8880158b1880 R14: ffff88801ec3c900 R15: ffff88800641a650
>  FS:  00005555722c3500(0000) GS:ffff8880f909d000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00007f66346e0f60 CR3: 000000001607c000 CR4: 0000000000350ef0
>  Call Trace:
>   <TASK>
>   genl_family_rcv_msg_doit+0x117/0x180 net/netlink/genetlink.c:1115
>   genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
>   genl_rcv_msg+0x3a8/0x3f0 net/netlink/genetlink.c:1210
>   netlink_rcv_skb+0x16d/0x240 net/netlink/af_netlink.c:2550
>   genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
>   netlink_unicast_kernel net/netlink/af_netlink.c:1318 [inline]
>   netlink_unicast+0x3e9/0x4c0 net/netlink/af_netlink.c:1344
>   netlink_sendmsg+0x4aa/0x5b0 net/netlink/af_netlink.c:1894
>   sock_sendmsg_nosec net/socket.c:727 [inline]
>   __sock_sendmsg+0xc9/0xf0 net/socket.c:742
>   ____sys_sendmsg+0x272/0x3b0 net/socket.c:2592
>   ___sys_sendmsg+0x2de/0x320 net/socket.c:2646
>   __sys_sendmsg net/socket.c:2678 [inline]
>   __do_sys_sendmsg net/socket.c:2683 [inline]
>   __se_sys_sendmsg net/socket.c:2681 [inline]
>   __x64_sys_sendmsg+0x110/0x1a0 net/socket.c:2681
>   do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>   do_syscall_64+0x143/0x440 arch/x86/entry/syscall_64.c:94
>   entry_SYSCALL_64_after_hwframe+0x77/0x7f
>  RIP: 0033:0x7f66346f826d
>  Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
>  RSP: 002b:00007ffc83d8bdc8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>  RAX: ffffffffffffffda RBX: 00007f6634985fa0 RCX: 00007f66346f826d
>  RDX: 00000000040000b0 RSI: 0000200000000740 RDI: 0000000000000007
>  RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
>  R10: 0000000000000000 R11: 0000000000000246 R12: 00007f6634985fa8
>  R13: 00007f6634985fac R14: 0000000000000000 R15: 0000000000001770
>   </TASK>
>
> The actions that caused that seem to be:
>
> - Set the MPTCP subflows limit to 0
> - Create an MPTCP endpoint with both the 'signal' and 'subflow' flags
> - Create a new MPTCP connection from a different address: an ADD_ADDR
>   linked to the MPTCP endpoint will be sent ('signal' flag), but no
>   subflows is initiated ('subflow' flag)
> - Remove the MPTCP endpoint
>
> In this case, msk->pm.local_addr_used has been kept to 0 -- because no
> subflows have been created -- but the corresponding bit in
> msk->pm.id_avail_bitmap has been cleared when the ADD_ADDR has been
> sent. This later causes a splat when removing the MPTCP endpoint because
> msk->pm.local_addr_used has been kept to 0.
>
> Now, if an endpoint has both the signal and subflow flags, but it is not
> possible to create subflows because of the limits or the c-flag case,
> then the local endpoint counter is still incremented: the endpoint is
> used at the end. This avoids issues later when removing the endpoint and
> calling __mark_subflow_endp_available(), which expects
> msk->pm.local_addr_used to have been previously incremented if the
> endpoint was marked as used according to msk->pm.id_avail_bitmap.
>
> Note that signal_and_subflow variable is reset to false when the limits
> and the c-flag case allows subflows creation. Also, local_addr_used is
> only incremented for non ID0 subflows.
>
> Fixes: 85df533a787b ("mptcp: pm: do not ignore 'subflow' if 'signal' flag is also set")
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/613
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> net/mptcp/pm_kernel.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index 87e37c729f81..2710abcd4065 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -418,6 +418,15 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
> 	}
>
> exit:
> +	/* If an endpoint has both the signal and subflow flags, but it is not
> +	 * possible to create subflows because of the limits or the c-flag case,
> +	 * then still mark the endp as used, which is the case. This avoids
> +	 * issues later when removing the endpoint and calling
> +	 * __mark_subflow_endp_available(), which expects the increment here.
> +	 */
> +	if (signal_and_subflow && local.addr.id != msk->mpc_endpoint_id)
> +		msk->pm.local_addr_used++;
> +

Looking at the code above, signal_and_subflow == TRUE is an indicator that 
the body of the while loop never executed. Please add a comment above this 
code in the while loop:

 		if (signal_and_subflow)
 			signal_and_subflow = false;

to point out that the code at the "exit:" label is assuming that 
signal_and_subflow is always false if the loop body runs.

Otherwise LGTM:

Reviewed-by: Mat Martineau <martineau@kernel.org>

> 	mptcp_pm_nl_check_work_pending(msk);
> }
>
>
> ---
> base-commit: eb01783b4b05934951774780509d96f7fe64c4a3
> change-id: 20260218-mptcp-issue-613-0ad1b55ca18c
>
> Best regards,
> -- 
> Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
>
>
Re: [PATCH mptcp-net] mptcp: pm: in-kernel: always mark signal+subflow endp as used
Posted by Matthieu Baerts 4 days, 10 hours ago
Hi Mat,

Thank you for the review!

On 25/02/2026 18:50, Mat Martineau wrote:
> On Wed, 18 Feb 2026, Matthieu Baerts (NGI0) wrote:
> 
>> Syzkaller managed to find a combination of actions that was generating
>> this warning:

(...)

>> Note that signal_and_subflow variable is reset to false when the limits
>> and the c-flag case allows subflows creation. Also, local_addr_used is
>> only incremented for non ID0 subflows.

(...)

>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>> index 87e37c729f81..2710abcd4065 100644
>> --- a/net/mptcp/pm_kernel.c
>> +++ b/net/mptcp/pm_kernel.c
>> @@ -418,6 +418,15 @@ static void
>> mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
>>     }
>>
>> exit:
>> +    /* If an endpoint has both the signal and subflow flags, but it
>> is not
>> +     * possible to create subflows because of the limits or the c-
>> flag case,
>> +     * then still mark the endp as used, which is the case. This avoids
>> +     * issues later when removing the endpoint and calling
>> +     * __mark_subflow_endp_available(), which expects the increment
>> here.
>> +     */
>> +    if (signal_and_subflow && local.addr.id != msk->mpc_endpoint_id)
>> +        msk->pm.local_addr_used++;
>> +
> 
> Looking at the code above, signal_and_subflow == TRUE is an indicator
> that the body of the while loop never executed. Please add a comment
> above this code in the while loop:
> 
>         if (signal_and_subflow)
>             signal_and_subflow = false;
> 
> to point out that the code at the "exit:" label is assuming that
> signal_and_subflow is always false if the loop body runs.

That's a good idea, but I'm hesitating to add that: adding such comment
might cause more conflicts. Plus, the use of this variable is quite
limited I think: init to false, set only in one case, and reset after
use. Do you think this comment is helpful there? Or should I add
something else here at the end of the function?

(There is something about that in the commit message, but that's
probably not enough I suppose)

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

Re: [PATCH mptcp-net] mptcp: pm: in-kernel: always mark signal+subflow endp as used
Posted by Mat Martineau 4 days, 9 hours ago
On Wed, 25 Feb 2026, Matthieu Baerts wrote:

> Hi Mat,
>
> Thank you for the review!
>
> On 25/02/2026 18:50, Mat Martineau wrote:
>> On Wed, 18 Feb 2026, Matthieu Baerts (NGI0) wrote:
>>
>>> Syzkaller managed to find a combination of actions that was generating
>>> this warning:
>
> (...)
>
>>> Note that signal_and_subflow variable is reset to false when the limits
>>> and the c-flag case allows subflows creation. Also, local_addr_used is
>>> only incremented for non ID0 subflows.
>
> (...)
>
>>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>>> index 87e37c729f81..2710abcd4065 100644
>>> --- a/net/mptcp/pm_kernel.c
>>> +++ b/net/mptcp/pm_kernel.c
>>> @@ -418,6 +418,15 @@ static void
>>> mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
>>>     }
>>>
>>> exit:
>>> +    /* If an endpoint has both the signal and subflow flags, but it is not
>>> +     * possible to create subflows because of the limits or the c-flag case,
>>> +     * then still mark the endp as used, which is the case. This avoids
>>> +     * issues later when removing the endpoint and calling
>>> +     * __mark_subflow_endp_available(), which expects the increment
>>> here.
>>> +     */
>>> +    if (signal_and_subflow && local.addr.id != msk->mpc_endpoint_id)
>>> +        msk->pm.local_addr_used++;
>>> +
>>
>> Looking at the code above, signal_and_subflow == TRUE is an indicator
>> that the body of the while loop never executed. Please add a comment
>> above this code in the while loop:
>>
>>         if (signal_and_subflow)
>>             signal_and_subflow = false;
>>
>> to point out that the code at the "exit:" label is assuming that
>> signal_and_subflow is always false if the loop body runs.
>
> That's a good idea, but I'm hesitating to add that: adding such comment
> might cause more conflicts. Plus, the use of this variable is quite
> limited I think: init to false, set only in one case, and reset after
> use. Do you think this comment is helpful there? Or should I add
> something else here at the end of the function?
>

True, it does run the risk of the comment and the code below getting out 
of sync and it's pointless to say "if you change the value of the variable 
it affects other code using that variable" (of course it does!).

I agree with you, and suggest updating the comment in the patch similar to 
"If an endpoint has both the signal and subflow flags and the 'while' loop 
body above never executed, ..."

Thanks,
Mat
Re: [PATCH mptcp-net] mptcp: pm: in-kernel: always mark signal+subflow endp as used
Posted by Matthieu Baerts 3 days, 20 hours ago
Hi Mat,

On 25/02/2026 19:46, Mat Martineau wrote:
> On Wed, 25 Feb 2026, Matthieu Baerts wrote:
> 
>> Hi Mat,
>>
>> Thank you for the review!
>>
>> On 25/02/2026 18:50, Mat Martineau wrote:
>>> On Wed, 18 Feb 2026, Matthieu Baerts (NGI0) wrote:
>>>
>>>> Syzkaller managed to find a combination of actions that was generating
>>>> this warning:
>>
>> (...)
>>
>>>> Note that signal_and_subflow variable is reset to false when the limits
>>>> and the c-flag case allows subflows creation. Also, local_addr_used is
>>>> only incremented for non ID0 subflows.
>>
>> (...)
>>
>>>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>>>> index 87e37c729f81..2710abcd4065 100644
>>>> --- a/net/mptcp/pm_kernel.c
>>>> +++ b/net/mptcp/pm_kernel.c
>>>> @@ -418,6 +418,15 @@ static void
>>>> mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
>>>>     }
>>>>
>>>> exit:
>>>> +    /* If an endpoint has both the signal and subflow flags, but it
>>>> is not
>>>> +     * possible to create subflows because of the limits or the c-
>>>> flag case,
>>>> +     * then still mark the endp as used, which is the case. This
>>>> avoids
>>>> +     * issues later when removing the endpoint and calling
>>>> +     * __mark_subflow_endp_available(), which expects the increment
>>>> here.
>>>> +     */
>>>> +    if (signal_and_subflow && local.addr.id != msk->mpc_endpoint_id)
>>>> +        msk->pm.local_addr_used++;
>>>> +
>>>
>>> Looking at the code above, signal_and_subflow == TRUE is an indicator
>>> that the body of the while loop never executed. Please add a comment
>>> above this code in the while loop:
>>>
>>>         if (signal_and_subflow)
>>>             signal_and_subflow = false;
>>>
>>> to point out that the code at the "exit:" label is assuming that
>>> signal_and_subflow is always false if the loop body runs.
>>
>> That's a good idea, but I'm hesitating to add that: adding such comment
>> might cause more conflicts. Plus, the use of this variable is quite
>> limited I think: init to false, set only in one case, and reset after
>> use. Do you think this comment is helpful there? Or should I add
>> something else here at the end of the function?
>>
> 
> True, it does run the risk of the comment and the code below getting out
> of sync and it's pointless to say "if you change the value of the
> variable it affects other code using that variable" (of course it does!).
> 
> I agree with you, and suggest updating the comment in the patch similar
> to "If an endpoint has both the signal and subflow flags and the 'while'
> loop body above never executed, ..."

Good idea, I just updated this comment before applying the patch:

/* If an endpoint has both the signal and subflow flags, but it is not
 * possible to create subflows -- the 'while' loop body above never
 * executed --  then still mark the endp as used, which is somehow the
 * case. This avoids issues later when removing the endpoint and calling
 * __mark_subflow_endp_available(), which expects the increment here.
 */

New patches for t/upstream-net and t/upstream:
- 402de2bca118: mptcp: pm: in-kernel: always mark signal+subflow endp as
used
- e8f38bbd8b67: selftests: mptcp: join: check removing signal+subflow endp
- Results: 652750a9d6a6..eefe19f1a175 (export-net)
- Results: d3854ef490c9..c0858783271a (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/ca3a60dc7df9f019289be14d7ca6898f8431381e/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/866d58409123388cf9032de57af875fbe418a642/checks


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

Re: [PATCH mptcp-net] mptcp: pm: in-kernel: always mark signal+subflow endp as used
Posted by MPTCP CI 1 week, 4 days ago
Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Unstable: 1 failed test(s): packetdrill_dss 🔴
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Notice: Call Traces at boot time, rebooted and continued 🔴
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/22155265067

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/efdd4665fff2
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1055307


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)