[PATCH mptcp-next v1 0/3] bugfixes for "BPF path manager, part 6, v2"

[PATCH mptcp-next v1 0/3] bugfixes for "BPF path manager, part 6, v2"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Bugfixes for "BPF path manager, part 6, v2".

Based-on: <cover.1741919893.git.tanggeliang@kylinos.cn>

Geliang Tang (3):
  Squash to "mptcp: pm: add subflow_established() interface"
  Squash to "mptcp: pm: add established() interface"
  Squash to "mptcp: pm: add rm_addr_received() interface"

 net/mptcp/pm.c        | 6 +++++-
 net/mptcp/pm_kernel.c | 4 ++++
 2 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH mptcp-next v1 1/3] Squash to "mptcp: pm: add subflow_established() interface"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

.subflow_established can't be invoked under mptcp pm lock, otherwise
this error occurs:

TAP version 13
1..1
[   65.985960][   T10] BUG: sleeping function called from invalid context at net/core/sock.c:3723
[   65.986336][   T10] in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 10, name: kworker/0:1
[   65.986630][   T10] preempt_count: 0, expected: 0
[   65.986964][   T10] RCU nest depth: 1, expected: 0
[   65.987157][   T10] 5 locks held by kworker/0:1/10:
[   65.987373][   T10]  #0: ffff888001134d48 ((wq_completion)events){+.+.}-{0:0}, at: process_one_work+0x7e4/0x16b0
[   65.987799][   T10]  #1: ffffc900000a7d30 ((work_completion)(&msk->work)){+.+.}-{0:0}, at: process_one_work+0xdf9/0x16b0
[   65.988348][   T10]  #2: ffff88800816d818 (sk_lock-AF_INET6){+.+.}-{0:0}, at: mptcp_worker+0x7b/0xad0
[   65.988806][   T10]  #3: ffffffffae584460 (rcu_read_lock){....}-{1:3}, at: __bpf_prog_enter+0x1f/0x170
[   65.989204][   T10]  #4: ffff888011042258 (k-sk_lock-AF_INET6#2){+.+.}-{0:0}, at: mptcp_pm_addr_send_ack+0x31d/0x3b0
[   65.989583][   T10] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Tainted: G        W  OE      6.14.0-rc6+ #137
[   65.989606][   T10] Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
[   65.989608][   T10] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[   65.989609][   T10] Workqueue: events mptcp_worker
[   65.989632][   T10] Call Trace:
[   65.989634][   T10]  <TASK>
[   65.989637][   T10]  dump_stack_lvl+0x9e/0xe0
[   65.989642][   T10]  __might_resched+0x35d/0x590
[   65.989665][   T10]  ? __pfx___might_resched+0x10/0x10
[   65.989676][   T10]  __lock_sock_fast+0x2f/0xd0
[   65.989682][   T10]  __mptcp_pm_send_ack+0x72/0x190
[   65.989684][   T10]  ? mptcp_pm_addr_send_ack+0x31d/0x3b0
[   65.989709][   T10]  mptcp_pm_addr_send_ack+0x31d/0x3b0
[   65.989714][   T10]  ? __pfx_mptcp_pm_addr_send_ack+0x10/0x10
[   65.989719][   T10]  ? mptcp_pm_announce_addr+0x2ef/0x410
[   65.989726][   T10]  mptcp_pm_create_subflow_or_signal_addr+0x75a/0xd10
[   65.989731][   T10]  ? hlock_class+0x4e/0x130
[   65.989739][   T10]  ? __pfx_mptcp_pm_create_subflow_or_signal_addr+0x10/0x10
[   65.989746][   T10]  ? __lock_acquire+0xb70/0x1650
[   65.989762][   T10]  ? lock_acquire.part.0+0xed/0x340
[   65.989766][   T10]  ? bpf_prog_66794828a779e50d_mptcp_pm_netlink_subflow_established+0x2e/0x43
[   65.989775][   T10]  ? __pfx_lock_acquire.part.0+0x10/0x10
[   65.989782][   T10]  ? do_raw_spin_lock+0x131/0x270
[   65.989787][   T10]  ? __pfx_do_raw_spin_lock+0x10/0x10
[   65.989791][   T10]  ? lock_acquire+0x31/0xc0
[   65.989795][   T10]  ? bpf_prog_66794828a779e50d_mptcp_pm_netlink_subflow_established+0x2e/0x43
[   65.989806][   T10]  bpf_prog_66794828a779e50d_mptcp_pm_netlink_subflow_established+0x36/0x43
[   65.989811][   T10]  bpf__mptcp_pm_ops_subflow_established+0x47/0xa3
[   65.989818][   T10]  mptcp_pm_worker+0x221/0x4e0
[   65.989825][   T10]  mptcp_worker+0xcd/0xad0
[   65.989828][   T10]  ? __pfx_lock_acquire.part.0+0x10/0x10
[   65.989833][   T10]  ? trace_lock_acquire+0x14e/0x1e0
[   65.989836][   T10]  ? trace_lock_acquire+0x14e/0x1e0
[   65.989840][   T10]  ? __pfx_mptcp_worker+0x10/0x10
[   65.989845][   T10]  ? process_one_work+0xdf9/0x16b0
[   65.989848][   T10]  ? lock_acquire+0x31/0xc0
[   65.989850][   T10]  ? process_one_work+0xdf9/0x16b0
[   65.989860][   T10]  process_one_work+0xe43/0x16b0
[   65.989875][   T10]  ? __pfx_process_one_work+0x10/0x10
[   65.989888][   T10]  ? assign_work+0x16c/0x240
[   65.989898][   T10]  worker_thread+0x593/0xf90
[   65.989911][   T10]  ? __kthread_parkme+0xba/0x1e0
[   65.989915][   T10]  ? __pfx_worker_thread+0x10/0x10
[   65.989920][   T10]  ? __pfx_worker_thread+0x10/0x10
[   65.989924][   T10]  kthread+0x34f/0x5d0
[   65.989928][   T10]  ? __pfx_kthread+0x10/0x10
[   65.989938][   T10]  ? __pfx_kthread+0x10/0x10
[   65.989943][   T10]  ret_from_fork+0x31/0x70
[   65.989946][   T10]  ? __pfx_kthread+0x10/0x10
[   65.989949][   T10]  ret_from_fork_asm+0x1a/0x30
[   65.989969][   T10]  </TASK>
[   66.432902][ T1536] ip (1536) used greatest stack depth: 23384 bytes left

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c        | 2 ++
 net/mptcp/pm_kernel.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index d504f9b31893..38abf15718b9 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -942,7 +942,9 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
 	}
 	if (pm->status & BIT(MPTCP_PM_SUBFLOW_ESTABLISHED)) {
 		pm->status &= ~BIT(MPTCP_PM_SUBFLOW_ESTABLISHED);
+		spin_unlock_bh(&msk->pm.lock);
 		pm->ops->subflow_established(msk);
+		spin_lock_bh(&msk->pm.lock);
 	}
 	__mptcp_pm_kernel_worker(msk);
 
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index d04dd1cece09..0c78715ed87f 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -374,7 +374,9 @@ static void mptcp_pm_kernel_established(struct mptcp_sock *msk)
 
 static void mptcp_pm_kernel_subflow_established(struct mptcp_sock *msk)
 {
+	spin_lock_bh(&msk->pm.lock);
 	mptcp_pm_create_subflow_or_signal_addr(msk);
+	spin_unlock_bh(&msk->pm.lock);
 }
 
 /* Fill all the local addresses into the array addrs[],
-- 
2.43.0

[PATCH mptcp-next v1 2/3] Squash to "mptcp: pm: add established() interface"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Same as .subflow_established, .established can't be invoked under mptcp
pm lock too.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c        | 2 ++
 net/mptcp/pm_kernel.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 38abf15718b9..788fbe31a40f 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -938,7 +938,9 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
 	}
 	if (pm->status & BIT(MPTCP_PM_ESTABLISHED)) {
 		pm->status &= ~BIT(MPTCP_PM_ESTABLISHED);
+		spin_unlock_bh(&msk->pm.lock);
 		pm->ops->established(msk);
+		spin_lock_bh(&msk->pm.lock);
 	}
 	if (pm->status & BIT(MPTCP_PM_SUBFLOW_ESTABLISHED)) {
 		pm->status &= ~BIT(MPTCP_PM_SUBFLOW_ESTABLISHED);
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 0c78715ed87f..f8cac996694c 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -369,7 +369,9 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 
 static void mptcp_pm_kernel_established(struct mptcp_sock *msk)
 {
+	spin_lock_bh(&msk->pm.lock);
 	mptcp_pm_create_subflow_or_signal_addr(msk);
+	spin_unlock_bh(&msk->pm.lock);
 }
 
 static void mptcp_pm_kernel_subflow_established(struct mptcp_sock *msk)
-- 
2.43.0

[PATCH mptcp-next v1 3/3] Squash to "mptcp: pm: add rm_addr_received() interface"

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Shouldn't return here, should give a chance to call
__mptcp_pm_kernel_worker().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 788fbe31a40f..5ef989abb4c0 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -934,7 +934,7 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
 		pm->status &= ~BIT(MPTCP_PM_RM_ADDR_RECEIVED);
 		spin_unlock_bh(&msk->pm.lock);
 		pm->ops->rm_addr_received(msk);
-		return;
+		spin_lock_bh(&msk->pm.lock);
 	}
 	if (pm->status & BIT(MPTCP_PM_ESTABLISHED)) {
 		pm->status &= ~BIT(MPTCP_PM_ESTABLISHED);
-- 
2.43.0

Re: [PATCH mptcp-next v1 0/3] bugfixes for "BPF path manager, part 6, v2"

[MPTCP CI]

Hi Geliang,

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: Critical: Global Timeout ❌
- 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/13891109215

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


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)