| Message ID | 20220203072508.3072309-8-kishen.maloor@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | mptcp: fixes and enhancements related to path management | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 120 lines checked |
| matttbe/build | success | Build and static analysis OK |
| matttbe/KVM_Validation__normal | warning | Unstable: 1 failed test(s): selftest_mptcp_join |
| matttbe/KVM_Validation__debug | warning | Unstable: 2 failed test(s): selftest_diag selftest_mptcp_join |
Kishen Maloor <kishen.maloor@intel.com> 于2022年2月3日周四 15:25写道： > > When ADD_ADDR announcements use the port associated with an > active subflow, this change ensures that a listening socket is bound > to the announced addr+port in the kernel for subsequently receiving > MP_JOINs. But if a listening socket for this address is already held > by the application then no action is taken. > > A listening socket is created (when there isn't a listener) > just prior to the addr advertisement. If it is desired to not create > a listening socket in the kernel for an address, then this can be > requested by including the MPTCP_PM_ADDR_FLAG_NO_LISTEN flag > with the address. > > When a listening socket is created, it is stored in > struct mptcp_pm_add_entry and released accordingly. > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/203 > Signed-off-by: Kishen Maloor <kishen.maloor@intel.com> > --- > v2: fixed formatting > v3: added new addr flag MPTCP_PM_ADDR_FLAG_NO_LISTEN to skip creating a > listening socket in the kernel during an ADD_ADDR request, use this flag > along the in-kernel PM flow for ADD_ADDR requests (Note: listening sockets > are always created for port-based endpoints as before), use the > lsk_list_find_or_create() helper > v4: call lsk_list_find() after a failed lsk_list_find_or_create() > for a chance to retrieve a recently created lsk by a simultaneous > call > --- > include/uapi/linux/mptcp.h | 1 + > net/mptcp/pm_netlink.c | 46 ++++++++++++++++++++++++++++++++++++-- > 2 files changed, 45 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h > index f106a3941cdf..265cabc0d7aa 100644 > --- a/include/uapi/linux/mptcp.h > +++ b/include/uapi/linux/mptcp.h > @@ -81,6 +81,7 @@ enum { > #define MPTCP_PM_ADDR_FLAG_SUBFLOW (1 << 1) > #define MPTCP_PM_ADDR_FLAG_BACKUP (1 << 2) > #define MPTCP_PM_ADDR_FLAG_FULLMESH (1 << 3) > +#define MPTCP_PM_ADDR_FLAG_NO_LISTEN (1 << 4) > > enum { > MPTCP_PM_CMD_UNSPEC, > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index a4fb9acbba51..9b3d871d3712 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -43,6 +43,7 @@ struct mptcp_pm_add_entry { > struct mptcp_addr_info addr; > struct timer_list add_timer; > struct mptcp_sock *sock; > + struct mptcp_local_lsk *lsk_ref; > u8 retrans_times; > }; > > @@ -469,7 +470,8 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk, > } > > static bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk, > - struct mptcp_pm_addr_entry *entry) > + struct mptcp_pm_addr_entry *entry, > + struct mptcp_local_lsk *lsk_ref) > { > struct mptcp_pm_add_entry *add_entry = NULL; > struct sock *sk = (struct sock *)msk; > @@ -489,6 +491,10 @@ static bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk, > add_entry->addr = entry->addr; > add_entry->sock = msk; > add_entry->retrans_times = 0; > + add_entry->lsk_ref = lsk_ref; > + > + if (lsk_ref) > + lsk_list_add_ref(lsk_ref); > > timer_setup(&add_entry->add_timer, mptcp_pm_add_timer, 0); > sk_reset_timer(sk, &add_entry->add_timer, > @@ -501,8 +507,11 @@ void mptcp_pm_free_anno_list(struct mptcp_sock *msk) > { > struct mptcp_pm_add_entry *entry, *tmp; > struct sock *sk = (struct sock *)msk; > + struct pm_nl_pernet *pernet; > LIST_HEAD(free_list); > > + pernet = net_generic(sock_net(sk), pm_nl_pernet_id); > + > pr_debug("msk=%p", msk); > > spin_lock_bh(&msk->pm.lock); > @@ -511,6 +520,8 @@ void mptcp_pm_free_anno_list(struct mptcp_sock *msk) > > list_for_each_entry_safe(entry, tmp, &free_list, list) { > sk_stop_timer_sync(sk, &entry->add_timer); > + if (entry->lsk_ref) > + lsk_list_release(pernet, entry->lsk_ref); > kfree(entry); > } > } > @@ -615,7 +626,9 @@ lookup_id_by_addr(struct pm_nl_pernet *pernet, const struct mptcp_addr_info *add > } > > static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) > + __must_hold(&msk->pm.lock) > { > + struct mptcp_local_lsk *lsk_ref = NULL; > struct sock *sk = (struct sock *)msk; > struct mptcp_pm_addr_entry *local; > unsigned int add_addr_signal_max; > @@ -652,12 +665,34 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) > local = select_signal_address(pernet, msk); > > if (local) { > - if (mptcp_pm_alloc_anno_list(msk, local)) { > + if (!(local->flags & MPTCP_PM_ADDR_FLAG_NO_LISTEN) && > + !local->addr.port) { > + local->addr.port = > + ((struct inet_sock *)inet_sk > + ((struct sock *)msk))->inet_sport; How about putting them in one line: local->addr.port = ((struct inet_sock *)inet_sk(sk))->inet_sport; > + > + spin_unlock_bh(&msk->pm.lock); > + > + lsk_ref = lsk_list_find_or_create(sock_net(sk), pernet, > + local, NULL); > + > + spin_lock_bh(&msk->pm.lock); > + > + if (!lsk_ref) > + lsk_ref = lsk_list_find(pernet, &local->addr); > + > + local->addr.port = 0; > + } > + > + if (mptcp_pm_alloc_anno_list(msk, local, lsk_ref)) { > __clear_bit(local->addr.id, msk->pm.id_avail_bitmap); > msk->pm.add_addr_signaled++; > mptcp_pm_announce_addr(msk, &local->addr, false); > mptcp_pm_nl_addr_send_ack(msk); > } > + > + if (lsk_ref) > + lsk_list_release(pernet, lsk_ref); > } > } > > @@ -749,6 +784,7 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk, > } > > static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) > + __must_hold(&msk->pm.lock) > { > struct mptcp_addr_info addrs[MPTCP_PM_ADDR_MAX]; > struct sock *sk = (struct sock *)msk; > @@ -1389,11 +1425,17 @@ int mptcp_pm_get_flags_and_ifindex_by_id(struct net *net, unsigned int id, > static bool remove_anno_list_by_saddr(struct mptcp_sock *msk, > struct mptcp_addr_info *addr) > { > + struct sock *sk = (struct sock *)msk; > struct mptcp_pm_add_entry *entry; > + struct pm_nl_pernet *pernet; > + > + pernet = net_generic(sock_net(sk), pm_nl_pernet_id); > > entry = mptcp_pm_del_add_timer(msk, addr, false); > if (entry) { > list_del(&entry->list); > + if (entry->lsk_ref) > + lsk_list_release(pernet, entry->lsk_ref); > kfree(entry); > return true; > } > -- > 2.31.1 > >
diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h index f106a3941cdf..265cabc0d7aa 100644 --- a/include/uapi/linux/mptcp.h +++ b/include/uapi/linux/mptcp.h @@ -81,6 +81,7 @@ enum { #define MPTCP_PM_ADDR_FLAG_SUBFLOW (1 << 1) #define MPTCP_PM_ADDR_FLAG_BACKUP (1 << 2) #define MPTCP_PM_ADDR_FLAG_FULLMESH (1 << 3) +#define MPTCP_PM_ADDR_FLAG_NO_LISTEN (1 << 4) enum { MPTCP_PM_CMD_UNSPEC, diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index a4fb9acbba51..9b3d871d3712 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -43,6 +43,7 @@ struct mptcp_pm_add_entry { struct mptcp_addr_info addr; struct timer_list add_timer; struct mptcp_sock *sock; + struct mptcp_local_lsk *lsk_ref; u8 retrans_times; }; @@ -469,7 +470,8 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk, } static bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk, - struct mptcp_pm_addr_entry *entry) + struct mptcp_pm_addr_entry *entry, + struct mptcp_local_lsk *lsk_ref) { struct mptcp_pm_add_entry *add_entry = NULL; struct sock *sk = (struct sock *)msk; @@ -489,6 +491,10 @@ static bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk, add_entry->addr = entry->addr; add_entry->sock = msk; add_entry->retrans_times = 0; + add_entry->lsk_ref = lsk_ref; + + if (lsk_ref) + lsk_list_add_ref(lsk_ref); timer_setup(&add_entry->add_timer, mptcp_pm_add_timer, 0); sk_reset_timer(sk, &add_entry->add_timer, @@ -501,8 +507,11 @@ void mptcp_pm_free_anno_list(struct mptcp_sock *msk) { struct mptcp_pm_add_entry *entry, *tmp; struct sock *sk = (struct sock *)msk; + struct pm_nl_pernet *pernet; LIST_HEAD(free_list); + pernet = net_generic(sock_net(sk), pm_nl_pernet_id); + pr_debug("msk=%p", msk); spin_lock_bh(&msk->pm.lock); @@ -511,6 +520,8 @@ void mptcp_pm_free_anno_list(struct mptcp_sock *msk) list_for_each_entry_safe(entry, tmp, &free_list, list) { sk_stop_timer_sync(sk, &entry->add_timer); + if (entry->lsk_ref) + lsk_list_release(pernet, entry->lsk_ref); kfree(entry); } } @@ -615,7 +626,9 @@ lookup_id_by_addr(struct pm_nl_pernet *pernet, const struct mptcp_addr_info *add } static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) + __must_hold(&msk->pm.lock) { + struct mptcp_local_lsk *lsk_ref = NULL; struct sock *sk = (struct sock *)msk; struct mptcp_pm_addr_entry *local; unsigned int add_addr_signal_max; @@ -652,12 +665,34 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk) local = select_signal_address(pernet, msk); if (local) { - if (mptcp_pm_alloc_anno_list(msk, local)) { + if (!(local->flags & MPTCP_PM_ADDR_FLAG_NO_LISTEN) && + !local->addr.port) { + local->addr.port = + ((struct inet_sock *)inet_sk + ((struct sock *)msk))->inet_sport; + + spin_unlock_bh(&msk->pm.lock); + + lsk_ref = lsk_list_find_or_create(sock_net(sk), pernet, + local, NULL); + + spin_lock_bh(&msk->pm.lock); + + if (!lsk_ref) + lsk_ref = lsk_list_find(pernet, &local->addr); + + local->addr.port = 0; + } + + if (mptcp_pm_alloc_anno_list(msk, local, lsk_ref)) { __clear_bit(local->addr.id, msk->pm.id_avail_bitmap); msk->pm.add_addr_signaled++; mptcp_pm_announce_addr(msk, &local->addr, false); mptcp_pm_nl_addr_send_ack(msk); } + + if (lsk_ref) + lsk_list_release(pernet, lsk_ref); } } @@ -749,6 +784,7 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk, } static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk) + __must_hold(&msk->pm.lock) { struct mptcp_addr_info addrs[MPTCP_PM_ADDR_MAX]; struct sock *sk = (struct sock *)msk; @@ -1389,11 +1425,17 @@ int mptcp_pm_get_flags_and_ifindex_by_id(struct net *net, unsigned int id, static bool remove_anno_list_by_saddr(struct mptcp_sock *msk, struct mptcp_addr_info *addr) { + struct sock *sk = (struct sock *)msk; struct mptcp_pm_add_entry *entry; + struct pm_nl_pernet *pernet; + + pernet = net_generic(sock_net(sk), pm_nl_pernet_id); entry = mptcp_pm_del_add_timer(msk, addr, false); if (entry) { list_del(&entry->list); + if (entry->lsk_ref) + lsk_list_release(pernet, entry->lsk_ref); kfree(entry); return true; }
When ADD_ADDR announcements use the port associated with an active subflow, this change ensures that a listening socket is bound to the announced addr+port in the kernel for subsequently receiving MP_JOINs. But if a listening socket for this address is already held by the application then no action is taken. A listening socket is created (when there isn't a listener) just prior to the addr advertisement. If it is desired to not create a listening socket in the kernel for an address, then this can be requested by including the MPTCP_PM_ADDR_FLAG_NO_LISTEN flag with the address. When a listening socket is created, it is stored in struct mptcp_pm_add_entry and released accordingly. Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/203 Signed-off-by: Kishen Maloor <kishen.maloor@intel.com> --- v2: fixed formatting v3: added new addr flag MPTCP_PM_ADDR_FLAG_NO_LISTEN to skip creating a listening socket in the kernel during an ADD_ADDR request, use this flag along the in-kernel PM flow for ADD_ADDR requests (Note: listening sockets are always created for port-based endpoints as before), use the lsk_list_find_or_create() helper v4: call lsk_list_find() after a failed lsk_list_find_or_create() for a chance to retrieve a recently created lsk by a simultaneous call --- include/uapi/linux/mptcp.h | 1 + net/mptcp/pm_netlink.c | 46 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-)