Hi Geliang,
On 14/03/2023 08:31, Geliang Tang wrote:
> Increase pm subflows counter on both server side and client side when
> userspace pm creates a new subflow, and decrease the counter when it
> closes a subflow.
I guess here as well, we can target -net and add a Fixes tag, no?
Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow
establishment")
(...)
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 4ed4d29d9c11..bb01f15d8e0a 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -87,8 +87,15 @@ bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
> unsigned int subflows_max;
> int ret = 0;
>
> - if (mptcp_pm_is_userspace(msk))
> - return mptcp_userspace_pm_active(msk);
> + if (mptcp_pm_is_userspace(msk)) {
> + if (mptcp_userspace_pm_active(msk)) {
> + spin_lock_bh(&pm->lock);
> + pm->subflows++;
> + spin_unlock_bh(&pm->lock);
> + return true;
> + }
> + return false;
> + }
>
> subflows_max = mptcp_pm_get_subflows_max(msk);
>
> @@ -181,8 +188,14 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk,
> struct mptcp_pm_data *pm = &msk->pm;
> bool update_subflows;
>
> - update_subflows = (subflow->request_join || subflow->mp_join) &&
> - mptcp_pm_is_kernel(msk);
> + if (mptcp_pm_is_userspace(msk)) {
> + spin_lock_bh(&pm->lock);
> + pm->subflows--;
> + spin_unlock_bh(&pm->lock);
> + return;
> + }
> +
> + update_subflows = (subflow->request_join || subflow->mp_join);
> if (!READ_ONCE(pm->work_pending) && !update_subflows)
> return;
I'm sorry, I'm not sure to understand why you do this here (and above)
and not just in mptcp_nl_cmd_sf_create() and _destroy() like you did in
the v3?
Cheers,
Matt
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 09b4b359d960..465928c59917 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -310,6 +310,7 @@ int mptcp_nl_cmd_sf_create(struct sk_buff *skb, struct genl_info *info)
>
> spin_lock_bh(&msk->pm.lock);
> msk->pm.local_addr_used++;
> + msk->pm.subflows++;
> spin_unlock_bh(&msk->pm.lock);
>
> lock_sock(sk);
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net