* [MPTCP] Re: [MPTCP][PATCH v3 mptcp-next 1/4] mptcp: drop *_max fields in mptcp_pm_data
@ 2021-01-06 1:39 Mat Martineau
0 siblings, 0 replies; 2+ messages in thread
From: Mat Martineau @ 2021-01-06 1:39 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 9637 bytes --]
On Tue, 5 Jan 2021, Geliang Tang wrote:
> This patch dropped the per-msk values add_addr_signal_max, add_addr_accept_max,
> local_addr_max and subflows_max fields in struct mptcp_pm_data, used the pernet
> *_max values instead. And added four new helpers to get the pernet *_max values
> separately.
>
> Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
> ---
> net/mptcp/pm.c | 9 +++--
> net/mptcp/pm_netlink.c | 87 ++++++++++++++++++++++++++++++------------
> net/mptcp/protocol.h | 5 +--
> 3 files changed, 69 insertions(+), 32 deletions(-)
>
Looks like there are some changes to mptcp_diag.c that didn't get in to
the commit - it's still looking for the values removed from struct
mptcp_pm_data and won't build.
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 0a6ebd0642ec..01a846b25771 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -78,10 +78,13 @@ void mptcp_pm_new_connection(struct mptcp_sock *msk, int server_side)
> bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
> {
> struct mptcp_pm_data *pm = &msk->pm;
> + unsigned int subflows_max;
> int ret = 0;
>
> + subflows_max = mptcp_pm_get_subflows_max(msk);
> +
> pr_debug("msk=%p subflows=%d max=%d allow=%d", msk, pm->subflows,
> - pm->subflows_max, READ_ONCE(pm->accept_subflow));
> + subflows_max, READ_ONCE(pm->accept_subflow));
>
> /* try to avoid acquiring the lock below */
> if (!READ_ONCE(pm->accept_subflow))
> @@ -89,8 +92,8 @@ bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
>
> spin_lock_bh(&pm->lock);
> if (READ_ONCE(pm->accept_subflow)) {
> - ret = pm->subflows < pm->subflows_max;
> - if (ret && ++pm->subflows == pm->subflows_max)
> + ret = pm->subflows < subflows_max;
> + if (ret && ++pm->subflows == subflows_max)
> WRITE_ONCE(pm->accept_subflow, false);
> }
> spin_unlock_bh(&pm->lock);
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index cc99410aca89..c65bd6dafd5b 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -197,11 +197,43 @@ select_signal_address(struct pm_nl_pernet *pernet, unsigned int pos)
> return ret;
> }
>
> +static unsigned int mptcp_pm_get_add_addr_signal_max(struct mptcp_sock *msk)
> +{
> + struct pm_nl_pernet *pernet;
> +
> + pernet = net_generic(sock_net((struct sock *)msk), pm_nl_pernet_id);
> + return READ_ONCE(pernet->add_addr_signal_max);
> +}
> +
> +static unsigned int mptcp_pm_get_add_addr_accept_max(struct mptcp_sock *msk)
> +{
> + struct pm_nl_pernet *pernet;
> +
> + pernet = net_generic(sock_net((struct sock *)msk), pm_nl_pernet_id);
> + return READ_ONCE(pernet->add_addr_accept_max);
> +}
> +
> +static unsigned int mptcp_pm_get_local_addr_max(struct mptcp_sock *msk)
> +{
> + struct pm_nl_pernet *pernet;
> +
> + pernet = net_generic(sock_net((struct sock *)msk), pm_nl_pernet_id);
> + return READ_ONCE(pernet->local_addr_max);
> +}
> +
> +unsigned int mptcp_pm_get_subflows_max(struct mptcp_sock *msk)
> +{
> + struct pm_nl_pernet *pernet;
> +
> + pernet = net_generic(sock_net((struct sock *)msk), pm_nl_pernet_id);
> + return READ_ONCE(pernet->subflows_max);
> +}
> +
These helpers all look good, but the places in existing code that write
these pernet values are not all using WRITE_ONCE(). Can you add a patch
before this to use WRITE_ONCE() on the values the 4 functions above access
with READ_ONCE()?
I think the rest of the patches are about ready for mptcp-next once the
pernet accesses are all handled.
Thanks!
Mat
> static void check_work_pending(struct mptcp_sock *msk)
> {
> - if (msk->pm.add_addr_signaled == msk->pm.add_addr_signal_max &&
> - (msk->pm.local_addr_used == msk->pm.local_addr_max ||
> - msk->pm.subflows == msk->pm.subflows_max))
> + if (msk->pm.add_addr_signaled == mptcp_pm_get_add_addr_signal_max(msk) &&
> + (msk->pm.local_addr_used == mptcp_pm_get_local_addr_max(msk) ||
> + msk->pm.subflows == mptcp_pm_get_subflows_max(msk)))
> WRITE_ONCE(msk->pm.work_pending, false);
> }
>
> @@ -329,17 +361,24 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
> struct mptcp_addr_info remote = { 0 };
> struct sock *sk = (struct sock *)msk;
> struct mptcp_pm_addr_entry *local;
> + unsigned int add_addr_signal_max;
> + unsigned int local_addr_max;
> struct pm_nl_pernet *pernet;
> + unsigned int subflows_max;
>
> pernet = net_generic(sock_net(sk), pm_nl_pernet_id);
>
> + add_addr_signal_max = mptcp_pm_get_add_addr_signal_max(msk);
> + local_addr_max = mptcp_pm_get_local_addr_max(msk);
> + subflows_max = mptcp_pm_get_subflows_max(msk);
> +
> pr_debug("local %d:%d signal %d:%d subflows %d:%d\n",
> - msk->pm.local_addr_used, msk->pm.local_addr_max,
> - msk->pm.add_addr_signaled, msk->pm.add_addr_signal_max,
> - msk->pm.subflows, msk->pm.subflows_max);
> + msk->pm.local_addr_used, local_addr_max,
> + msk->pm.add_addr_signaled, add_addr_signal_max,
> + msk->pm.subflows, subflows_max);
>
> /* check first for announce */
> - if (msk->pm.add_addr_signaled < msk->pm.add_addr_signal_max) {
> + if (msk->pm.add_addr_signaled < add_addr_signal_max) {
> local = select_signal_address(pernet,
> msk->pm.add_addr_signaled);
>
> @@ -351,15 +390,15 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
> }
> } else {
> /* pick failed, avoid fourther attempts later */
> - msk->pm.local_addr_used = msk->pm.add_addr_signal_max;
> + msk->pm.local_addr_used = add_addr_signal_max;
> }
>
> check_work_pending(msk);
> }
>
> /* check if should create a new subflow */
> - if (msk->pm.local_addr_used < msk->pm.local_addr_max &&
> - msk->pm.subflows < msk->pm.subflows_max) {
> + if (msk->pm.local_addr_used < local_addr_max &&
> + msk->pm.subflows < subflows_max) {
> remote_address((struct sock_common *)sk, &remote);
>
> local = select_local_address(pernet, msk);
> @@ -374,7 +413,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
> }
>
> /* lookup failed, avoid fourther attempts later */
> - msk->pm.local_addr_used = msk->pm.local_addr_max;
> + msk->pm.local_addr_used = local_addr_max;
> check_work_pending(msk);
> }
> }
> @@ -392,17 +431,22 @@ void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk)
> void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
> {
> struct sock *sk = (struct sock *)msk;
> + unsigned int add_addr_accept_max;
> struct mptcp_addr_info remote;
> struct mptcp_addr_info local;
> + unsigned int subflows_max;
> bool use_port = false;
>
> + add_addr_accept_max = mptcp_pm_get_add_addr_accept_max(msk);
> + subflows_max = mptcp_pm_get_subflows_max(msk);
> +
> pr_debug("accepted %d:%d remote family %d",
> - msk->pm.add_addr_accepted, msk->pm.add_addr_accept_max,
> + msk->pm.add_addr_accepted, add_addr_accept_max,
> msk->pm.remote.family);
> msk->pm.add_addr_accepted++;
> msk->pm.subflows++;
> - if (msk->pm.add_addr_accepted >= msk->pm.add_addr_accept_max ||
> - msk->pm.subflows >= msk->pm.subflows_max)
> + if (msk->pm.add_addr_accepted >= add_addr_accept_max ||
> + msk->pm.subflows >= subflows_max)
> WRITE_ONCE(msk->pm.accept_addr, false);
>
> /* connect to the specified remote address, using whatever
> @@ -683,19 +727,12 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
> void mptcp_pm_nl_data_init(struct mptcp_sock *msk)
> {
> struct mptcp_pm_data *pm = &msk->pm;
> - struct pm_nl_pernet *pernet;
> bool subflows;
>
> - pernet = net_generic(sock_net((struct sock *)msk), pm_nl_pernet_id);
> -
> - pm->add_addr_signal_max = READ_ONCE(pernet->add_addr_signal_max);
> - pm->add_addr_accept_max = READ_ONCE(pernet->add_addr_accept_max);
> - pm->local_addr_max = READ_ONCE(pernet->local_addr_max);
> - pm->subflows_max = READ_ONCE(pernet->subflows_max);
> - subflows = !!pm->subflows_max;
> - WRITE_ONCE(pm->work_pending, (!!pm->local_addr_max && subflows) ||
> - !!pm->add_addr_signal_max);
> - WRITE_ONCE(pm->accept_addr, !!pm->add_addr_accept_max && subflows);
> + subflows = !!mptcp_pm_get_subflows_max(msk);
> + WRITE_ONCE(pm->work_pending, (!!mptcp_pm_get_local_addr_max(msk) && subflows) ||
> + !!mptcp_pm_get_add_addr_signal_max(msk));
> + WRITE_ONCE(pm->accept_addr, !!mptcp_pm_get_add_addr_accept_max(msk) && subflows);
> WRITE_ONCE(pm->accept_subflow, subflows);
> }
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index d6400ad2d615..f6ed8dc0a073 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -202,10 +202,6 @@ struct mptcp_pm_data {
> u8 add_addr_accepted;
> u8 local_addr_used;
> u8 subflows;
> - u8 add_addr_signal_max;
> - u8 add_addr_accept_max;
> - u8 local_addr_max;
> - u8 subflows_max;
> u8 status;
> u8 rm_id;
> };
> @@ -626,6 +622,7 @@ void mptcp_pm_nl_add_addr_send_ack(struct mptcp_sock *msk);
> void mptcp_pm_nl_rm_addr_received(struct mptcp_sock *msk);
> void mptcp_pm_nl_rm_subflow_received(struct mptcp_sock *msk, u8 rm_id);
> int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
> +unsigned int mptcp_pm_get_subflows_max(struct mptcp_sock *msk);
>
> static inline struct mptcp_ext *mptcp_get_ext(struct sk_buff *skb)
> {
> --
> 2.29.2
> _______________________________________________
> mptcp mailing list -- mptcp(a)lists.01.org
> To unsubscribe send an email to mptcp-leave(a)lists.01.org
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 2+ messages in thread
* [MPTCP] Re: [MPTCP][PATCH v3 mptcp-next 1/4] mptcp: drop *_max fields in mptcp_pm_data
@ 2021-01-05 4:18 Geliang Tang
0 siblings, 0 replies; 2+ messages in thread
From: Geliang Tang @ 2021-01-05 4:18 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 907 bytes --]
Geliang Tang <geliangtang(a)gmail.com> 于2021年1月5日周二 下午12:05写道：
>
> This patch dropped the per-msk values add_addr_signal_max, add_addr_accept_max,
> local_addr_max and subflows_max fields in struct mptcp_pm_data, used the pernet
> *_max values instead. And added four new helpers to get the pernet *_max values
> separately.
>
Sorry, there's a checkpatch.pl warning here:
WARNING: Possible unwrapped commit description (prefer a maximum 75
chars per line)
#13:
local_addr_max and subflows_max fields in struct mptcp_pm_data, used the pernet
Please update the commit message:
---
This patch dropped the per-msk values add_addr_signal_max,
add_addr_accept_max, local_addr_max and subflows_max fields in struct
mptcp_pm_data, used the pernet *_max values instead. And added four new
helpers to get the pernet *_max values separately.
---
Thanks.
-Geliang
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-01-06 1:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-06 1:39 [MPTCP] Re: [MPTCP][PATCH v3 mptcp-next 1/4] mptcp: drop *_max fields in mptcp_pm_data Mat Martineau
-- strict thread matches above, loose matches on Subject: below --
2021-01-05 4:18 Geliang Tang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.