userspace pm: only remove entry from `local_addr_list` when sending a `REMOVE_ADDR` #403

matttbe · 2023-05-17T15:21:16Z

Linked to #379, local addresses are being tracked on the kernel side to be able to send REMOVE_ADDR for subflows that have been created (MP_JOIN (and MP_CAPABLE)) but not announced (ADD_ADDR) previously.

The entries are added via mptcp_userspace_pm_append_new_local_addr(). To be able to re-use IDs, an explicit REMOVE_ADDR should be sent, we should not remove entries when a subflow is deleted by the userspace (see mptcp_userspace_pm_delete_local_addr()).

So, if I'm not mistaken:

mptcp_pm_nl_subflow_create_doit() should not call mptcp_userspace_pm_delete_local_addr() if the entry is added in the list after the creation of the subflow (or only remove the entry if it has just been created before).
mptcp_pm_nl_subflow_destroy_doit() should not call mptcp_userspace_pm_delete_local_addr()
mptcp_pm_nl_remove_doit() should call mptcp_userspace_pm_delete_local_addr() (which might need to be modified)
We should replace this comment above mptcp_userspace_pm_delete_local_addr()...

/* 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.
 */

... by something like:

/* Only remove entries from the local addr list if the address has been
 * explicitly removed via a REMOVE_ADDR. Removing a subflow doesn't mean the
 * address became unavailable.
 */

We should remove the TODO about the refcount in mptcp_userspace_pm_delete_local_addr()
And close this ticket

The text was updated successfully, but these errors were encountered:

Cited commit converted the neighbour code to use the standard RCU variant instead of the RCU-bh variant, but the VRF code still uses rcu_read_lock_bh() / rcu_read_unlock_bh() around the neighbour lookup code in its IPv4 and IPv6 output paths, resulting in lockdep splats [1][2]. Can be reproduced using [3]. Fix by switching to rcu_read_lock() / rcu_read_unlock(). [1] ============================= WARNING: suspicious RCU usage 6.5.0-rc1-custom-g9c099e6dbf98 #403 Not tainted ----------------------------- include/net/neighbour.h:302 suspicious rcu_dereference_check() usage! other info that might help us debug this: rcu_scheduler_active = 2, debug_locks = 1 2 locks held by ping/183: #0: ffff888105ea1d80 (sk_lock-AF_INET){+.+.}-{0:0}, at: raw_sendmsg+0xc6c/0x33c0 #1: ffffffff85b46820 (rcu_read_lock_bh){....}-{1:2}, at: vrf_output+0x2e3/0x2030 stack backtrace: CPU: 0 PID: 183 Comm: ping Not tainted 6.5.0-rc1-custom-g9c099e6dbf98 #403 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0xc1/0xf0 lockdep_rcu_suspicious+0x211/0x3b0 vrf_output+0x1380/0x2030 ip_push_pending_frames+0x125/0x2a0 raw_sendmsg+0x200d/0x33c0 inet_sendmsg+0xa2/0xe0 __sys_sendto+0x2aa/0x420 __x64_sys_sendto+0xe5/0x1c0 do_syscall_64+0x38/0x80 entry_SYSCALL_64_after_hwframe+0x63/0xcd [2] ============================= WARNING: suspicious RCU usage 6.5.0-rc1-custom-g9c099e6dbf98 #403 Not tainted ----------------------------- include/net/neighbour.h:302 suspicious rcu_dereference_check() usage! other info that might help us debug this: rcu_scheduler_active = 2, debug_locks = 1 2 locks held by ping6/182: #0: ffff888114b63000 (sk_lock-AF_INET6){+.+.}-{0:0}, at: rawv6_sendmsg+0x1602/0x3e50 #1: ffffffff85b46820 (rcu_read_lock_bh){....}-{1:2}, at: vrf_output6+0xe9/0x1310 stack backtrace: CPU: 0 PID: 182 Comm: ping6 Not tainted 6.5.0-rc1-custom-g9c099e6dbf98 #403 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0xc1/0xf0 lockdep_rcu_suspicious+0x211/0x3b0 vrf_output6+0xd32/0x1310 ip6_local_out+0xb4/0x1a0 ip6_send_skb+0xbc/0x340 ip6_push_pending_frames+0xe5/0x110 rawv6_sendmsg+0x2e6e/0x3e50 inet_sendmsg+0xa2/0xe0 __sys_sendto+0x2aa/0x420 __x64_sys_sendto+0xe5/0x1c0 do_syscall_64+0x38/0x80 entry_SYSCALL_64_after_hwframe+0x63/0xcd [3] #!/bin/bash ip link add name vrf-red up numtxqueues 2 type vrf table 10 ip link add name swp1 up master vrf-red type dummy ip address add 192.0.2.1/24 dev swp1 ip address add 2001:db8:1::1/64 dev swp1 ip neigh add 192.0.2.2 lladdr 00:11:22:33:44:55 nud perm dev swp1 ip neigh add 2001:db8:1::2 lladdr 00:11:22:33:44:55 nud perm dev swp1 ip vrf exec vrf-red ping 192.0.2.2 -c 1 &> /dev/null ip vrf exec vrf-red ping6 2001:db8:1::2 -c 1 &> /dev/null Fixes: 09eed11 ("neighbour: switch to standard rcu, instead of rcu_bh") Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> Link: https://lore.kernel.org/netdev/CA+G9fYtEr-=GbcXNDYo3XOkwR+uYgehVoDjsP0pFLUpZ_AZcyg@mail.gmail.com/ Signed-off-by: Ido Schimmel <idosch@nvidia.com> Reviewed-by: David Ahern <dsahern@kernel.org> Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://lore.kernel.org/r/20230715153605.4068066-1-idosch@nvidia.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>

This patch adds userspace PM address entry refcount. Add a new filed 'refcnt' in struct mptcp_pm_addr_entry, initiated to 1. Increase this counter in mptcp_nl_cmd_sf_create(), and decrease it in mptcp_userspace_pm_delete_local_addr() according the subflows value. Closes: multipath-tcp/mptcp_net-next#403 Fixes: 24430f8 ("mptcp: add address into userspace pm list") Signed-off-by: Geliang Tang <geliang.tang@suse.com>

This patch adds the refcount of addree entry in userspace PM. Add a new counter 'refcnt' in struct mptcp_pm_addr_entry, initiated to 1. Increase this counter when an address is announced or a subflow is created in mptcp_pm_nl_announce_doit() and mptcp_pm_nl_subflow_create_doit(). And decrease it when an address is removed or a subflow is closed in mptcp_pm_nl_remove_doit() and mptcp_userspace_pm_delete_local_addr(). If the counter reaches to 1, free this entry. Closes: multipath-tcp/mptcp_net-next#403 Fixes: 24430f8 ("mptcp: add address into userspace pm list") Signed-off-by: Geliang Tang <geliang.tang@suse.com>

This patch adds the refcount of addree entry in userspace PM. Add a new counter 'refcnt' in struct mptcp_pm_addr_entry, initiated to 1. Increase this counter when an address is announced or a subflow is created in mptcp_pm_nl_announce_doit() and mptcp_pm_nl_subflow_create_doit(). And decrease it when an address is removed or a subflow is closed in mptcp_pm_nl_remove_doit() and mptcp_userspace_pm_delete_local_addr(). If the counter reaches to 1, free this entry. Closes: multipath-tcp#403 Fixes: 24430f8 ("mptcp: add address into userspace pm list") Signed-off-by: Geliang Tang <geliang.tang@linux.dev>

matttbe added enhancement pm path-manager labels May 17, 2023

geliangtang self-assigned this May 24, 2023

matttbe changed the title ~~userspace pm: local_addr_list: use a refcount per entry~~ userspace pm: only remove entry from local_addr_list when sending a REMOVE_ADDR Feb 2, 2024

matttbe added bug and removed enhancement labels Feb 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

userspace pm: only remove entry from `local_addr_list` when sending a `REMOVE_ADDR` #403

userspace pm: only remove entry from `local_addr_list` when sending a `REMOVE_ADDR` #403

matttbe commented May 17, 2023 •

edited

Loading

userspace pm: only remove entry from local_addr_list when sending a REMOVE_ADDR #403

userspace pm: only remove entry from local_addr_list when sending a REMOVE_ADDR #403

Comments

matttbe commented May 17, 2023 • edited Loading

userspace pm: only remove entry from `local_addr_list` when sending a `REMOVE_ADDR` #403

userspace pm: only remove entry from `local_addr_list` when sending a `REMOVE_ADDR` #403

matttbe commented May 17, 2023 •

edited

Loading