net/mptcp/pm.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
From: Gang Yan <yangang@kylinos.cn>
This patch removes the need to handle a NULL pm_ops in the fallback
path, and moves the rcu_read_lock\unlock into mptcp_pm_ops_init.
Following sashiko's comments, it also moves the pr_debug() before
bpf_module_put() in mptcp_pm_ops_release.
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
net/mptcp/pm.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 9dc7b41fb562..5fe4276cf895 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -1137,12 +1137,14 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
spin_unlock_bh(&msk->pm.lock);
}
-static void mptcp_pm_ops_init(struct mptcp_sock *msk,
- struct mptcp_pm_ops *pm_ops)
+static void mptcp_pm_ops_init(struct mptcp_sock *msk, const char *pm_name)
{
+ struct mptcp_pm_ops *pm_ops;
+
+ rcu_read_lock();
+ pm_ops = mptcp_pm_find(pm_name);
if (!pm_ops || !bpf_try_module_get(pm_ops, pm_ops->owner)) {
- pr_warn_once("pm %s fails, fallback to default pm",
- pm_ops->name);
+ pr_warn_once("pm %s fails, fallback to default pm", pm_name);
pm_ops = &mptcp_pm_kernel;
}
@@ -1151,6 +1153,7 @@ static void mptcp_pm_ops_init(struct mptcp_sock *msk,
msk->pm.ops->init(msk);
pr_debug("pm %s initialized\n", pm_ops->name);
+ rcu_read_unlock();
}
static void mptcp_pm_ops_release(struct mptcp_sock *msk)
@@ -1161,9 +1164,9 @@ static void mptcp_pm_ops_release(struct mptcp_sock *msk)
if (pm_ops->release)
pm_ops->release(msk);
- bpf_module_put(pm_ops, pm_ops->owner);
-
pr_debug("pm %s released\n", pm_ops->name);
+
+ bpf_module_put(pm_ops, pm_ops->owner);
}
void mptcp_pm_destroy(struct mptcp_sock *msk)
@@ -1184,9 +1187,7 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
pm->rm_list_rx.nr = 0;
WRITE_ONCE(pm->pm_type, pm_type);
- rcu_read_lock();
- mptcp_pm_ops_init(msk, mptcp_pm_find(pm_name));
- rcu_read_unlock();
+ mptcp_pm_ops_init(msk, pm_name);
}
void mptcp_pm_data_init(struct mptcp_sock *msk)
--
2.43.0
Hi Gang,
Thanks for this v2.
On Wed, 2026-07-01 at 15:37 +0800, Gang Yan wrote:
> From: Gang Yan <yangang@kylinos.cn>
>
> This patch removes the need to handle a NULL pm_ops in the fallback
> path, and moves the rcu_read_lock\unlock into mptcp_pm_ops_init.
>
> Following sashiko's comments, it also moves the pr_debug() before
> bpf_module_put() in mptcp_pm_ops_release.
>
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
> net/mptcp/pm.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 9dc7b41fb562..5fe4276cf895 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -1137,12 +1137,14 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
> spin_unlock_bh(&msk->pm.lock);
> }
>
> -static void mptcp_pm_ops_init(struct mptcp_sock *msk,
> - struct mptcp_pm_ops *pm_ops)
> +static void mptcp_pm_ops_init(struct mptcp_sock *msk, const char
> *pm_name)
> {
> + struct mptcp_pm_ops *pm_ops;
> +
> + rcu_read_lock();
> + pm_ops = mptcp_pm_find(pm_name);
> if (!pm_ops || !bpf_try_module_get(pm_ops, pm_ops->owner)) {
> - pr_warn_once("pm %s fails, fallback to default pm",
> - pm_ops->name);
> + pr_warn_once("pm %s fails, fallback to default pm",
> pm_name);
> pm_ops = &mptcp_pm_kernel;
> }
>
> @@ -1151,6 +1153,7 @@ static void mptcp_pm_ops_init(struct mptcp_sock
> *msk,
> msk->pm.ops->init(msk);
>
> pr_debug("pm %s initialized\n", pm_ops->name);
nit: Here we can also print "pm_name" to keep it consistent with
pr_warn_once().
> + rcu_read_unlock();
Then this unlock line can be moved ahead of the previous pr_debug()
line.
> }
>
> static void mptcp_pm_ops_release(struct mptcp_sock *msk)
> @@ -1161,9 +1164,9 @@ static void mptcp_pm_ops_release(struct
> mptcp_sock *msk)
> if (pm_ops->release)
> pm_ops->release(msk);
>
> - bpf_module_put(pm_ops, pm_ops->owner);
> -
> pr_debug("pm %s released\n", pm_ops->name);
> +
> + bpf_module_put(pm_ops, pm_ops->owner);
> }
>
> void mptcp_pm_destroy(struct mptcp_sock *msk)
> @@ -1184,9 +1187,7 @@ void mptcp_pm_data_reset(struct mptcp_sock
> *msk)
> pm->rm_list_rx.nr = 0;
> WRITE_ONCE(pm->pm_type, pm_type);
>
> - rcu_read_lock();
> - mptcp_pm_ops_init(msk, mptcp_pm_find(pm_name));
> - rcu_read_unlock();
Or we leave rcu_read_lock() and rcu_read_unlock() where they are,
without moving them into mptcp_pm_ops_init().
Thanks,
-Geliang
> + mptcp_pm_ops_init(msk, pm_name);
> }
>
> void mptcp_pm_data_init(struct mptcp_sock *msk)
July 2, 2026 at 10:15 AM, "Geliang Tang" <geliang@kernel.org mailto:geliang@kernel.org?to=%22Geliang%20Tang%22%20%3Cgeliang%40kernel.org%3E > wrote:
Hi Geliang,
Thanks for your review.
>
> Hi Gang,
>
> Thanks for this v2.
>
> On Wed, 2026-07-01 at 15:37 +0800, Gang Yan wrote:
>
> >
> > From: Gang Yan <yangang@kylinos.cn>
> >
> > This patch removes the need to handle a NULL pm_ops in the fallback
> > path, and moves the rcu_read_lock\unlock into mptcp_pm_ops_init.
> >
> > Following sashiko's comments, it also moves the pr_debug() before
> > bpf_module_put() in mptcp_pm_ops_release.
> >
> > Signed-off-by: Gang Yan <yangang@kylinos.cn>
> > ---
> > net/mptcp/pm.c | 19 ++++++++++---------
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> > index 9dc7b41fb562..5fe4276cf895 100644
> > --- a/net/mptcp/pm.c
> > +++ b/net/mptcp/pm.c
> > @@ -1137,12 +1137,14 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
> > spin_unlock_bh(&msk->pm.lock);
> > }
> >
> > -static void mptcp_pm_ops_init(struct mptcp_sock *msk,
> > - struct mptcp_pm_ops *pm_ops)
> > +static void mptcp_pm_ops_init(struct mptcp_sock *msk, const char
> > *pm_name)
> > {
> > + struct mptcp_pm_ops *pm_ops;
> > +
> > + rcu_read_lock();
> > + pm_ops = mptcp_pm_find(pm_name);
> > if (!pm_ops || !bpf_try_module_get(pm_ops, pm_ops->owner)) {
> > - pr_warn_once("pm %s fails, fallback to default pm",
> > - pm_ops->name);
> > + pr_warn_once("pm %s fails, fallback to default pm",
> > pm_name);
> > pm_ops = &mptcp_pm_kernel;
> > }
> >
> > @@ -1151,6 +1153,7 @@ static void mptcp_pm_ops_init(struct mptcp_sock
> > *msk,
> > msk->pm.ops->init(msk);
> >
> > pr_debug("pm %s initialized\n", pm_ops->name);
> >
> nit: Here we can also print "pm_name" to keep it consistent with
> pr_warn_once().
Here is the print of the name of initialized pm, for example, if we fallback
to kernel_PM it will print like:
'''
pm bpf_pm fails, fallback to default pm
pm kernel initialized
'''
So I think it should use 'pm_ops->name' here.
WDYT?
>
> >
> > + rcu_read_unlock();
> >
> Then this unlock line can be moved ahead of the previous pr_debug()
> line.
>
> >
> > }
> >
> > static void mptcp_pm_ops_release(struct mptcp_sock *msk)
> > @@ -1161,9 +1164,9 @@ static void mptcp_pm_ops_release(struct
> > mptcp_sock *msk)
> > if (pm_ops->release)
> > pm_ops->release(msk);
> >
> > - bpf_module_put(pm_ops, pm_ops->owner);
> > -
> > pr_debug("pm %s released\n", pm_ops->name);
> > +
> > + bpf_module_put(pm_ops, pm_ops->owner);
> > }
> >
> > void mptcp_pm_destroy(struct mptcp_sock *msk)
> > @@ -1184,9 +1187,7 @@ void mptcp_pm_data_reset(struct mptcp_sock
> > *msk)
> > pm->rm_list_rx.nr = 0;
> > WRITE_ONCE(pm->pm_type, pm_type);
> >
> > - rcu_read_lock();
> > - mptcp_pm_ops_init(msk, mptcp_pm_find(pm_name));
> > - rcu_read_unlock();
> >
> Or we leave rcu_read_lock() and rcu_read_unlock() where they are,
> without moving them into mptcp_pm_ops_init().
>
LGTM, I'll apply the reset suggestions when sending v3.
> Thanks,
> -Geliang
>
> >
Thanks
Gang
> > + mptcp_pm_ops_init(msk, pm_name);
> > }
> >
> > void mptcp_pm_data_init(struct mptcp_sock *msk)
> >
>
Hi Gang,
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): Unstable: 1 failed test(s): packetdrill_fastopen ⚠️
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- 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/28503396809
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/dac4cb248a7d
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1119502
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)
© 2016 - 2026 Red Hat, Inc.