v14: - drop mptcp_pm_remove_addrs helper in patch 1 - add two flags in patch 4, the address entry'll be removed from userspace_pm_local_addr_list only when both flags are set, by doing this, it's now independent of the order of the remove_subflows command and the remove_addrs command. v13: - move the RM_ADDR command after the destruction of the subflow in patch 2 and patch 5. - drop mptcp_pm_remove_anno_list_by_saddr in mptcp_nl_cmd_sf_destroy in patch 4. - update userspace_pm.sh too in patch 5. v12: - address Matt's commits in v11. v11: - #1-#5 part 1, address Matt's comments in v10. - #6-#9 part 2, update pm mptcp_info - #10-#12 part 3, some cleanups. v10: - fix userspace_pm.sh errors reported by CI. - fix the bug in mptcp_pm_remove_addrs in patch 1. - drop msk->pm.subflow == 1 in mptcp_userspace_pm_delete_local_addr in patch 3. - exchange the order of "pm_nl_ctl rem" and "pm_nl_ctl dsf" in patch 2 and 6. - update the commit logs. v9: - address Matt's commets in v8. v8: - address Matt's comments. - split into two series, pt 2 will send later. v7: - fix userspace_pm.sh errors reported by CI. - only remove addrs in mptcp_nl_cmd_remove(). v6: - send a RM ADDR from userspace. v5: - fix a memleak error reported by CI. - add more delay for userspace pm tests. v4: - add more patches - add selftests v3: - update local_addr_used and add_addr_signaled v2: - hold pm locks Geliang Tang (5): mptcp: only send RM_ADDR in nl_cmd_remove selftests: mptcp: update userspace pm addr tests mptcp: export remove_anno_list_by_saddr mptcp: add address into userspace pm list selftests: mptcp: update userspace pm subflow tests include/uapi/linux/mptcp.h | 2 + net/mptcp/pm_netlink.c | 8 +- net/mptcp/pm_userspace.c | 77 ++++++++++++++++++- net/mptcp/protocol.h | 2 + .../testing/selftests/net/mptcp/mptcp_join.sh | 11 ++- .../selftests/net/mptcp/userspace_pm.sh | 3 + 6 files changed, 94 insertions(+), 9 deletions(-) -- 2.35.3 The specifications from [1] about the "REMOVE" command say: Announce that an address has been lost to the peer It was then only supposed to send a RM_ADDR and not trying to delete associated subflows. mptcp_pm_remove_addr() is invoked to do just that, compared to mptcp_pm_remove_addrs_and_subflows() also removing subflows. To delete a subflow, the userspace daemon can use the "SUB_DESTROY" command, see mptcp_nl_cmd_sf_destroy(). Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE") Link: https://github.com/multipath-tcp/mptcp/blob/mptcp_v0.96/include/uapi/linux/mptcp.h [1] Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- net/mptcp/pm_userspace.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info) { struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN]; struct nlattr *id = info->attrs[MPTCP_PM_ATTR_LOC_ID]; + struct mptcp_rm_list rm_list = { .nr = 0 }; struct mptcp_pm_addr_entry *match = NULL; struct mptcp_pm_addr_entry *entry; struct mptcp_sock *msk; @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info) goto remove_err; } - list_move(&match->list, &free_list); - - mptcp_pm_remove_addrs_and_subflows(msk, &free_list); + rm_list.ids[rm_list.nr++] = match->addr.id; + spin_lock_bh(&msk->pm.lock); + mptcp_pm_remove_addr(msk, &rm_list); + spin_unlock_bh(&msk->pm.lock); release_sock((struct sock *)msk); + list_move(&match->list, &free_list); list_for_each_entry_safe(match, entry, &free_list, list) { sock_kfree_s((struct sock *)msk, match, sizeof(*match)); } -- 2.35.3 This patch is linked to the previous commit ("mptcp: only send RM_ADDR in nl_cmd_remove"). To align with what is done by the in-kernel PM, update userspace pm addr selftests, by sending a remove_subflows command together after the remove_addrs command. It's independent of the order of the remove_subflows command and the remove_addrs command. Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE") Fixes: 97040cf9806e ("selftests: mptcp: userspace pm address tests") Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index XXXXXXX..XXXXXXX 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -XXX,XX +XXX,XX @@ do_transfer() sed -n 's/.*\(token:\)\([[:digit:]]*\).*$/\2/p;q') ip netns exec ${listener_ns} ./pm_nl_ctl ann $addr token $tk id $id sleep 1 + sp=$(grep "type:10" "$evts_ns1" | + sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q') + da=$(grep "type:10" "$evts_ns1" | + sed -n 's/.*\(daddr6:\)\([0-9a-f:.]*\).*$/\2/p;q') + dp=$(grep "type:10" "$evts_ns1" | + sed -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q') ip netns exec ${listener_ns} ./pm_nl_ctl rem token $tk id $id + ip netns exec ${listener_ns} ./pm_nl_ctl dsf lip "::ffff:$addr" \ + lport $sp rip $da rport $dp token $tk fi counter=$((counter + 1)) -- 2.35.3 Rename remove_anno_list_by_saddr() with "mptcp_pm_" prefix and export it in protocol.h. This function will be re-used in the userspace PM code in the following commit. Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- net/mptcp/pm_netlink.c | 8 ++++---- net/mptcp/protocol.h | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -XXX,XX +XXX,XX @@ int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id return 0; } -static bool remove_anno_list_by_saddr(struct mptcp_sock *msk, - const struct mptcp_addr_info *addr) +bool mptcp_pm_remove_anno_list_by_saddr(struct mptcp_sock *msk, + const struct mptcp_addr_info *addr) { struct mptcp_pm_add_entry *entry; @@ -XXX,XX +XXX,XX @@ static bool mptcp_pm_remove_anno_addr(struct mptcp_sock *msk, list.ids[list.nr++] = addr->id; - ret = remove_anno_list_by_saddr(msk, addr); + ret = mptcp_pm_remove_anno_list_by_saddr(msk, addr); if (ret || force) { spin_lock_bh(&msk->pm.lock); mptcp_pm_remove_addr(msk, &list); @@ -XXX,XX +XXX,XX @@ void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk, slist.nr < MPTCP_RM_IDS_MAX) slist.ids[slist.nr++] = entry->addr.id; - if (remove_anno_list_by_saddr(msk, &entry->addr) && + if (mptcp_pm_remove_anno_list_by_saddr(msk, &entry->addr) && alist.nr < MPTCP_RM_IDS_MAX) alist.ids[alist.nr++] = entry->addr.id; } diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -XXX,XX +XXX,XX @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk, struct mptcp_pm_add_entry * mptcp_lookup_anno_list_by_saddr(const struct mptcp_sock *msk, const struct mptcp_addr_info *addr); +bool mptcp_pm_remove_anno_list_by_saddr(struct mptcp_sock *msk, + const struct mptcp_addr_info *addr); int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id, u8 *flags, int *ifindex); -- 2.35.3 Add the address into userspace_pm_local_addr_list when the subflow is created. Make sure it can be found in mptcp_nl_cmd_remove(). And delete it in the new helper mptcp_userspace_pm_delete_local_addr(). Add address into pm anno_list in mptcp_nl_cmd_sf_create(). Remove it when connecting fails. By doing this, the "REMOVE" command also works with subflows that have been created via the "SUB_CREATE" command instead of restricting to the addresses that have been announced via the "ANNOUNCE" command. Add two new removing flags for userspace PM: MPTCP_USER_PM_FLAG_RM_SIGNAL and MPTCP_USER_PM_FLAG_RM_SUBFLOW. They'll be set in mptcp_nl_cmd_remove() and mptcp_nl_cmd_sf_destroy(). The address entry'll be removed from userspace_pm_local_addr_list only when both flags are set. Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE") Link: https://github.com/multipath-tcp/mptcp_net-next/issues/379 Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- include/uapi/linux/mptcp.h | 2 ++ net/mptcp/pm_userspace.c | 72 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h index XXXXXXX..XXXXXXX 100644 --- a/include/uapi/linux/mptcp.h +++ b/include/uapi/linux/mptcp.h @@ -XXX,XX +XXX,XX @@ enum { #define MPTCP_PM_ADDR_FLAG_BACKUP (1 << 2) #define MPTCP_PM_ADDR_FLAG_FULLMESH (1 << 3) #define MPTCP_PM_ADDR_FLAG_IMPLICIT (1 << 4) +#define MPTCP_USER_PM_FLAG_RM_SIGNAL (1 << 5) +#define MPTCP_USER_PM_FLAG_RM_SUBFLOW (1 << 6) enum { MPTCP_PM_CMD_UNSPEC, diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -XXX,XX +XXX,XX @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk, return ret; } +static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, + struct mptcp_pm_addr_entry *addr) +{ + struct mptcp_pm_addr_entry *entry, *tmp; + + list_for_each_entry_safe(entry, tmp, &msk->pm.userspace_pm_local_addr_list, list) { + if (mptcp_addresses_equal(&entry->addr, &addr->addr, false)) { + if (entry->flags & MPTCP_USER_PM_FLAG_RM_SUBFLOW) + break; + entry->flags |= MPTCP_USER_PM_FLAG_RM_SUBFLOW; + if (entry->flags & MPTCP_USER_PM_FLAG_RM_SIGNAL) { + /* TODO: a refcount is needed because the entry can + * be used multiple times (e.g. fullmesh mode). + */ + list_del_rcu(&entry->list); + kfree(entry); + } + return 0; + } + } + + return -EINVAL; +} + int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id, u8 *flags, int *ifindex) @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info) goto remove_err; } + if (match->flags & MPTCP_USER_PM_FLAG_RM_SIGNAL) { + GENL_SET_ERR_MSG(info, "no need to remove this address"); + release_sock((struct sock *)msk); + goto remove_err; + } + rm_list.ids[rm_list.nr++] = match->addr.id; spin_lock_bh(&msk->pm.lock); mptcp_pm_remove_addr(msk, &rm_list); @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info) release_sock((struct sock *)msk); - list_move(&match->list, &free_list); - list_for_each_entry_safe(match, entry, &free_list, list) { - sock_kfree_s((struct sock *)msk, match, sizeof(*match)); + match->flags |= MPTCP_USER_PM_FLAG_RM_SIGNAL; + if (match->flags & MPTCP_USER_PM_FLAG_RM_SUBFLOW) { + list_move(&match->list, &free_list); + list_for_each_entry_safe(match, entry, &free_list, list) { + sock_kfree_s((struct sock *)msk, match, sizeof(*match)); + } } err = 0; @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_sf_create(struct sk_buff *skb, struct genl_info *info) struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN]; struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR]; + struct mptcp_pm_addr_entry local = { 0 }; struct mptcp_addr_info addr_r; struct mptcp_addr_info addr_l; struct mptcp_sock *msk; @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_sf_create(struct sk_buff *skb, struct genl_info *info) goto create_err; } + local.addr = addr_l; + err = mptcp_userspace_pm_append_new_local_addr(msk, &local); + if (err < 0) { + GENL_SET_ERR_MSG(info, "did not match address and id"); + goto create_err; + } + + spin_lock_bh(&msk->pm.lock); + if (!mptcp_pm_alloc_anno_list(msk, &local)) { + GENL_SET_ERR_MSG(info, "cannot alloc address list"); + err = -EINVAL; + goto anno_list_err; + } + spin_unlock_bh(&msk->pm.lock); + lock_sock(sk); err = __mptcp_subflow_connect(sk, &addr_l, &addr_r); release_sock(sk); + if (err) { + spin_lock_bh(&msk->pm.lock); + mptcp_pm_remove_anno_list_by_saddr(msk, &addr_l); +anno_list_err: + mptcp_userspace_pm_delete_local_addr(msk, &local); + spin_unlock_bh(&msk->pm.lock); + } + create_err: sock_put((struct sock *)msk); return err; @@ -XXX,XX +XXX,XX @@ static struct sock *mptcp_nl_find_ssk(struct mptcp_sock *msk, return NULL; } +/* If the subflow is closed from the other peer (not via a + * subflow destroy command then), we want to keep the entry + * not to assign the same ID to another address and to be + * able to send RM_ADDR after the removal of the subflow. + */ int mptcp_nl_cmd_sf_destroy(struct sk_buff *skb, struct genl_info *info) { struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_sf_destroy(struct sk_buff *skb, struct genl_info *info) ssk = mptcp_nl_find_ssk(msk, &addr_l, &addr_r); if (ssk) { struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk); + struct mptcp_pm_addr_entry entry = { .addr = addr_l }; + spin_lock_bh(&msk->pm.lock); + mptcp_userspace_pm_delete_local_addr(msk, &entry); + spin_unlock_bh(&msk->pm.lock); mptcp_subflow_shutdown(sk, ssk, RCV_SHUTDOWN | SEND_SHUTDOWN); mptcp_close_ssk(sk, ssk, subflow); MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RMSUBFLOW); -- 2.35.3 To align with what is done by the in-kernel PM, update userspace pm subflow selftests in mptcp_join.sh and userspace_pm.sh, by sending the a remove_addrs command together after the remove_subflows command. This will get a RM_ADDR in chk_rm_nr(). Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE") Fixes: 5e986ec46874 ("selftests: mptcp: userspace pm subflow tests") Link: https://github.com/multipath-tcp/mptcp_net-next/issues/379 Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 3 ++- tools/testing/selftests/net/mptcp/userspace_pm.sh | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index XXXXXXX..XXXXXXX 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -XXX,XX +XXX,XX @@ do_transfer() sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q') ip netns exec ${connector_ns} ./pm_nl_ctl dsf lip $addr lport $sp \ rip $da rport $dp token $tk + ip netns exec ${connector_ns} ./pm_nl_ctl rem token $tk id $id fi counter=$((counter + 1)) add_nr_ns2=$((add_nr_ns2 - 1)) @@ -XXX,XX +XXX,XX @@ userspace_tests() pm_nl_set_limits $ns1 0 1 run_tests $ns1 $ns2 10.0.1.1 0 0 userspace_1 slow chk_join_nr 1 1 1 - chk_rm_nr 0 1 + chk_rm_nr 1 1 kill_events_pids fi } diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh index XXXXXXX..XXXXXXX 100755 --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh @@ -XXX,XX +XXX,XX @@ test_subflows() :>"$server_evts" ip netns exec "$ns1" ./pm_nl_ctl dsf lip dead:beef:2::1 lport "$sport" rip\ dead:beef:2::2 rport "$client6_port" token "$server6_token" > /dev/null 2>&1 + ip netns exec "$ns1" ./pm_nl_ctl rem id 23 token "$server6_token" > /dev/null 2>&1 sleep 0.5 verify_subflow_events "$server_evts" "$SUB_CLOSED" "$server6_token" "$AF_INET6"\ "dead:beef:2::1" "dead:beef:2::2" "$client6_port" "23"\ @@ -XXX,XX +XXX,XX @@ test_subflows() :>"$server_evts" ip netns exec "$ns1" ./pm_nl_ctl dsf lip 10.0.2.1 lport "$sport" rip 10.0.2.2 rport\ $new4_port token "$server4_token" > /dev/null 2>&1 + ip netns exec "$ns1" ./pm_nl_ctl rem id 23 token "$server4_token" > /dev/null 2>&1 sleep 0.5 verify_subflow_events "$server_evts" "$SUB_CLOSED" "$server4_token" "$AF_INET" "10.0.2.1"\ "10.0.2.2" "$new4_port" "23" "$client_addr_id" "ns1" "ns2" @@ -XXX,XX +XXX,XX @@ test_subflows() :>"$client_evts" ip netns exec "$ns2" ./pm_nl_ctl dsf lip dead:beef:2::2 lport "$sport" rip\ dead:beef:2::1 rport $app6_port token "$client6_token" > /dev/null 2>&1 + ip netns exec "$ns2" ./pm_nl_ctl rem id 23 token "$client6_token" > /dev/null 2>&1 sleep 0.5 verify_subflow_events $client_evts $SUB_CLOSED $client6_token $AF_INET6 "dead:beef:2::2"\ "dead:beef:2::1" "$app6_port" "23" "$server_addr_id" "ns2" "ns1" -- 2.35.3 v15: - drop "mptcp: export remove_anno_list_by_saddr" - drop MPTCP_USER_PM_FLAG_RM_* flags - drop mptcp_pm_alloc_anno_list in mptcp_nl_cmd_sf_create. v14: - drop mptcp_pm_remove_addrs helper in patch 1 - add two flags in patch 4, the address entry'll be removed from userspace_pm_local_addr_list only when both flags are set, by doing this, it's now independent of the order of the remove_subflows command and the remove_addrs command. v13: - move the RM_ADDR command after the destruction of the subflow in patch 2 and patch 5. - drop mptcp_pm_remove_anno_list_by_saddr in mptcp_nl_cmd_sf_destroy in patch 4. - update userspace_pm.sh too in patch 5. v12: - address Matt's commits in v11. v11: - #1-#5 part 1, address Matt's comments in v10. - #6-#9 part 2, update pm mptcp_info - #10-#12 part 3, some cleanups. v10: - fix userspace_pm.sh errors reported by CI. - fix the bug in mptcp_pm_remove_addrs in patch 1. - drop msk->pm.subflow == 1 in mptcp_userspace_pm_delete_local_addr in patch 3. - exchange the order of "pm_nl_ctl rem" and "pm_nl_ctl dsf" in patch 2 and 6. - update the commit logs. v9: - address Matt's commets in v8. v8: - address Matt's comments. - split into two series, pt 2 will send later. v7: - fix userspace_pm.sh errors reported by CI. - only remove addrs in mptcp_nl_cmd_remove(). v6: - send a RM ADDR from userspace. v5: - fix a memleak error reported by CI. - add more delay for userspace pm tests. v4: - add more patches - add selftests v3: - update local_addr_used and add_addr_signaled v2: - hold pm locks Geliang Tang (5): mptcp: only send RM_ADDR in nl_cmd_remove selftests: mptcp: update userspace pm addr tests mptcp: add address into userspace pm list selftests: mptcp: update userspace pm subflow tests mptcp: update userspace pm infos net/mptcp/pm.c | 23 +++++++-- net/mptcp/pm_netlink.c | 18 +++++++ net/mptcp/pm_userspace.c | 48 ++++++++++++++++++- net/mptcp/protocol.h | 1 + .../testing/selftests/net/mptcp/mptcp_join.sh | 11 ++++- 5 files changed, 95 insertions(+), 6 deletions(-) -- 2.35.3 The specifications from [1] about the "REMOVE" command say: Announce that an address has been lost to the peer It was then only supposed to send a RM_ADDR and not trying to delete associated subflows. A new helper mptcp_pm_remove_addrs() is then introduced to do just that, compared to mptcp_pm_remove_addrs_and_subflows() also removing subflows. To delete a subflow, the userspace daemon can use the "SUB_DESTROY" command, see mptcp_nl_cmd_sf_destroy(). Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE") Link: https://github.com/multipath-tcp/mptcp/blob/mptcp_v0.96/include/uapi/linux/mptcp.h [1] Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- net/mptcp/pm_netlink.c | 18 ++++++++++++++++++ net/mptcp/pm_userspace.c | 2 +- net/mptcp/protocol.h | 1 + 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -XXX,XX +XXX,XX @@ static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info) return ret; } +void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct list_head *rm_list) +{ + struct mptcp_rm_list alist = { .nr = 0 }; + struct mptcp_pm_addr_entry *entry; + + list_for_each_entry(entry, rm_list, list) { + remove_anno_list_by_saddr(msk, &entry->addr); + if (alist.nr < MPTCP_RM_IDS_MAX) + alist.ids[alist.nr++] = entry->addr.id; + } + + if (alist.nr) { + spin_lock_bh(&msk->pm.lock); + mptcp_pm_remove_addr(msk, &alist); + spin_unlock_bh(&msk->pm.lock); + } +} + void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk, struct list_head *rm_list) { diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_remove(struct sk_buff *skb, struct genl_info *info) list_move(&match->list, &free_list); - mptcp_pm_remove_addrs_and_subflows(msk, &free_list); + mptcp_pm_remove_addrs(msk, &free_list); release_sock((struct sock *)msk); diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -XXX,XX +XXX,XX @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk, bool echo); int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_list); int mptcp_pm_remove_subflow(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_list); +void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct list_head *rm_list); void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk, struct list_head *rm_list); -- 2.35.3 This patch is linked to the previous commit ("mptcp: only send RM_ADDR in nl_cmd_remove"). To align with what is done by the in-kernel PM, update userspace pm addr selftests, by sending a remove_subflows command together after the remove_addrs command. Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE") Fixes: 97040cf9806e ("selftests: mptcp: userspace pm address tests") Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index XXXXXXX..XXXXXXX 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -XXX,XX +XXX,XX @@ do_transfer() sed -n 's/.*\(token:\)\([[:digit:]]*\).*$/\2/p;q') ip netns exec ${listener_ns} ./pm_nl_ctl ann $addr token $tk id $id sleep 1 + sp=$(grep "type:10" "$evts_ns1" | + sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q') + da=$(grep "type:10" "$evts_ns1" | + sed -n 's/.*\(daddr6:\)\([0-9a-f:.]*\).*$/\2/p;q') + dp=$(grep "type:10" "$evts_ns1" | + sed -n 's/.*\(dport:\)\([[:digit:]]*\).*$/\2/p;q') ip netns exec ${listener_ns} ./pm_nl_ctl rem token $tk id $id + ip netns exec ${listener_ns} ./pm_nl_ctl dsf lip "::ffff:$addr" \ + lport $sp rip $da rport $dp token $tk fi counter=$((counter + 1)) -- 2.35.3 Add the address into userspace_pm_local_addr_list when the subflow is created. Make sure it can be found in mptcp_nl_cmd_remove(). And delete it in the new helper mptcp_userspace_pm_delete_local_addr(). Add address into pm anno_list in mptcp_nl_cmd_sf_create(). Remove it when connecting fails. By doing this, the "REMOVE" command also works with subflows that have been created via the "SUB_CREATE" command instead of restricting to the addresses that have been announced via the "ANNOUNCE" command. Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE") Link: https://github.com/multipath-tcp/mptcp_net-next/issues/379 Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- net/mptcp/pm_userspace.c | 41 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -XXX,XX +XXX,XX @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk, return ret; } +/* If the subflow is closed from the other peer (not via a + * subflow destroy command then), we want to keep the entry + * not to assign the same ID to another address and to be + * able to send RM_ADDR after the removal of the subflow. + */ +static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, + struct mptcp_pm_addr_entry *addr) +{ + struct mptcp_pm_addr_entry *entry, *tmp; + + list_for_each_entry_safe(entry, tmp, &msk->pm.userspace_pm_local_addr_list, list) { + if (mptcp_addresses_equal(&entry->addr, &addr->addr, false)) { + /* TODO: a refcount is needed because the entry can + * be used multiple times (e.g. fullmesh mode). + */ + list_del_rcu(&entry->list); + kfree(entry); + return 0; + } + } + + return -EINVAL; +} + int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk, unsigned int id, u8 *flags, int *ifindex) @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_sf_create(struct sk_buff *skb, struct genl_info *info) struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE]; struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN]; struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR]; + struct mptcp_pm_addr_entry local = { 0 }; struct mptcp_addr_info addr_r; struct mptcp_addr_info addr_l; struct mptcp_sock *msk; @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_sf_create(struct sk_buff *skb, struct genl_info *info) goto create_err; } + local.addr = addr_l; + err = mptcp_userspace_pm_append_new_local_addr(msk, &local); + if (err < 0) { + GENL_SET_ERR_MSG(info, "did not match address and id"); + goto create_err; + } + lock_sock(sk); err = __mptcp_subflow_connect(sk, &addr_l, &addr_r); release_sock(sk); + spin_lock_bh(&msk->pm.lock); + if (err) + mptcp_userspace_pm_delete_local_addr(msk, &local); + spin_unlock_bh(&msk->pm.lock); + create_err: sock_put((struct sock *)msk); return err; @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_sf_destroy(struct sk_buff *skb, struct genl_info *info) ssk = mptcp_nl_find_ssk(msk, &addr_l, &addr_r); if (ssk) { struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk); + struct mptcp_pm_addr_entry entry = { .addr = addr_l }; + spin_lock_bh(&msk->pm.lock); + mptcp_userspace_pm_delete_local_addr(msk, &entry); + spin_unlock_bh(&msk->pm.lock); mptcp_subflow_shutdown(sk, ssk, RCV_SHUTDOWN | SEND_SHUTDOWN); mptcp_close_ssk(sk, ssk, subflow); MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RMSUBFLOW); -- 2.35.3 To align with what is done by the in-kernel PM, update userspace pm subflow selftests, by sending the a remove_addrs command together before the remove_subflows command. This will get a RM_ADDR in chk_rm_nr(). Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE") Fixes: 5e986ec46874 ("selftests: mptcp: userspace pm subflow tests") Link: https://github.com/multipath-tcp/mptcp_net-next/issues/379 Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index XXXXXXX..XXXXXXX 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -XXX,XX +XXX,XX @@ do_transfer() sleep 1 sp=$(grep "type:10" "$evts_ns2" | sed -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q') + ip netns exec ${connector_ns} ./pm_nl_ctl rem token $tk id $id ip netns exec ${connector_ns} ./pm_nl_ctl dsf lip $addr lport $sp \ rip $da rport $dp token $tk fi @@ -XXX,XX +XXX,XX @@ userspace_tests() pm_nl_set_limits $ns1 0 1 run_tests $ns1 $ns2 10.0.1.1 0 0 userspace_1 slow chk_join_nr 1 1 1 - chk_rm_nr 0 1 + chk_rm_nr 1 1 kill_events_pids fi } -- 2.35.3 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. Increase add_addr_signaled counter in mptcp_nl_cmd_announce() when the address is announced by userspace PM. This modification is similar to how the in-kernel PM is updating the counter: when additional subflows are created/removed. Fixes: 9ab4807c84a4 ("mptcp: netlink: Add MPTCP_PM_CMD_ANNOUNCE") Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment") Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/329 Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- net/mptcp/pm.c | 23 +++++++++++++++++++---- net/mptcp/pm_userspace.c | 5 +++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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); + update_subflows = subflow->request_join || subflow->mp_join; + if (mptcp_pm_is_userspace(msk)) { + if (update_subflows) { + spin_lock_bh(&pm->lock); + pm->subflows--; + spin_unlock_bh(&pm->lock); + } + return; + } + if (!READ_ONCE(pm->work_pending) && !update_subflows) return; diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c index XXXXXXX..XXXXXXX 100644 --- a/net/mptcp/pm_userspace.c +++ b/net/mptcp/pm_userspace.c @@ -XXX,XX +XXX,XX @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk, MPTCP_PM_MAX_ADDR_ID + 1, 1); list_add_tail_rcu(&e->list, &msk->pm.userspace_pm_local_addr_list); + msk->pm.local_addr_used++; ret = e->addr.id; } else if (match) { ret = entry->addr.id; @@ -XXX,XX +XXX,XX @@ static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk, list_for_each_entry_safe(entry, tmp, &msk->pm.userspace_pm_local_addr_list, list) { if (mptcp_addresses_equal(&entry->addr, &addr->addr, false)) { + msk->pm.local_addr_used--; /* TODO: a refcount is needed because the entry can * be used multiple times (e.g. fullmesh mode). */ @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_announce(struct sk_buff *skb, struct genl_info *info) spin_lock_bh(&msk->pm.lock); if (mptcp_pm_alloc_anno_list(msk, &addr_val)) { + msk->pm.add_addr_signaled++; mptcp_pm_announce_addr(msk, &addr_val.addr, false); mptcp_pm_nl_addr_send_ack(msk); } @@ -XXX,XX +XXX,XX @@ int mptcp_nl_cmd_sf_create(struct sk_buff *skb, struct genl_info *info) spin_lock_bh(&msk->pm.lock); if (err) mptcp_userspace_pm_delete_local_addr(msk, &local); + else + msk->pm.subflows++; spin_unlock_bh(&msk->pm.lock); create_err: -- 2.35.3