Hi Geliang,
On 22/10/2024 11:14, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> mptcp_pm_remove_addrs() actually only deletes one address, which does
> not match its name. This patch renames it to mptcp_pm_remove_addr_entry()
> and changes the parameter "rm_list" to "entry".
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> net/mptcp/pm_netlink.c | 24 ++++++++++--------------
> net/mptcp/pm_userspace.c | 2 +-
> net/mptcp/protocol.h | 3 ++-
> 3 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 06f0c7df575e..852706e43f13 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1615,26 +1615,22 @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
> }
>
> /* Called from the userspace PM only */
(out of curiosity: after the modifications you did, could this helper
not be "easily" moved to pm_userspace.c?)
> -void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct list_head *rm_list)
> +void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
> + struct mptcp_pm_addr_entry *entry)
> {
> struct mptcp_rm_list alist = { .nr = 0 };
> - struct mptcp_pm_addr_entry *entry;
> int anno_nr = 0;
>
> - list_for_each_entry(entry, rm_list, list) {
> - if (alist.nr >= MPTCP_RM_IDS_MAX)
> - break;
> -
> - /* only delete if either announced or matching a subflow */
> - if (remove_anno_list_by_saddr(msk, &entry->addr))
> - anno_nr++;
> - else if (!lookup_subflow_by_saddr(&msk->conn_list,
> - &entry->addr))
> - continue;
> + /* only delete if either announced or matching a subflow */
> + if (remove_anno_list_by_saddr(msk, &entry->addr))
> + anno_nr++;
> + else if (!lookup_subflow_by_saddr(&msk->conn_list,
> + &entry->addr))
(maybe this can move to the previous line if it is under 80 chars?)
> + goto out;
>
> - alist.ids[alist.nr++] = entry->addr.id;
> - }
> + alist.ids[alist.nr++] = entry->addr.id;
>
> +out:
> if (alist.nr) {
> spin_lock_bh(&msk->pm.lock);
> msk->pm.add_addr_signaled -= anno_nr;
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 3ad10d8062d9..21b2a3b02cfe 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -322,7 +322,7 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
> list_move(&match->list, &free_list);
Do you still need to do this move? Do you still need "free_list"? Can
you just remove the entry from the list?
> spin_unlock_bh(&msk->pm.lock);
>
> - mptcp_pm_remove_addrs(msk, &free_list);
> + mptcp_pm_remove_addr_entry(msk, match);
>
> release_sock(sk);
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 9dc01140af23..13cd97f9e0f7 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -1051,7 +1051,8 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk,
> const struct mptcp_addr_info *addr,
> bool echo);
> int mptcp_pm_remove_addr(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_addr_entry(struct mptcp_sock *msk,
> + struct mptcp_pm_addr_entry *entry);
>
> void mptcp_free_local_addr_list(struct mptcp_sock *msk);
>
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.