Re: [PATCH net-next 2/4] mptcp: unify pm get_local_id interfaces

[Larysa Zaremba]

On Thu, Jun 08, 2023 at 03:20:50PM +0200, Matthieu Baerts wrote:
> From: Geliang Tang <geliang.tang@xxxxxxxx>
> 
> This patch unifies the three PM get_local_id() interfaces:
> 
> mptcp_pm_nl_get_local_id() in mptcp/pm_netlink.c for the in-kernel PM and
> mptcp_userspace_pm_get_local_id() in mptcp/pm_userspace.c for the
> userspace PM.
> 
> They'll be switched in the common PM infterface mptcp_pm_get_local_id()
> in mptcp/pm.c based on whether mptcp_pm_is_userspace() or not.
> 
> Also put together the declarations of these three functions in protocol.h.
> 
> Signed-off-by: Geliang Tang <geliang.tang@xxxxxxxx>
> Reviewed-by: Matthieu Baerts <matthieu.baerts@xxxxxxxxxxxx>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@xxxxxxxxxxxx>

Reviewed-by: Larysa Zaremba <larysa.zaremba@xxxxxxxxx>

> ---
>  net/mptcp/pm.c         | 18 +++++++++++++++++-
>  net/mptcp/pm_netlink.c | 22 +++-------------------
>  net/mptcp/protocol.h   |  2 +-
>  3 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 7d03b5fd8200..5a027a46196c 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -400,7 +400,23 @@ bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
>  
>  int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
>  {
> -	return mptcp_pm_nl_get_local_id(msk, skc);
> +	struct mptcp_addr_info skc_local;
> +	struct mptcp_addr_info msk_local;
> +
> +	if (WARN_ON_ONCE(!msk))
> +		return -1;
> +
> +	/* The 0 ID mapping is defined by the first subflow, copied into the msk
> +	 * addr
> +	 */
> +	mptcp_local_address((struct sock_common *)msk, &msk_local);
> +	mptcp_local_address((struct sock_common *)skc, &skc_local);
> +	if (mptcp_addresses_equal(&msk_local, &skc_local, false))
> +		return 0;
> +
> +	if (mptcp_pm_is_userspace(msk))
> +		return mptcp_userspace_pm_get_local_id(msk, &skc_local);
> +	return mptcp_pm_nl_get_local_id(msk, &skc_local);
>  }
>  
>  void mptcp_pm_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index c55ed3dda0d8..315ad669eb3c 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1055,33 +1055,17 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
>  	return 0;
>  }
>  
> -int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
> +int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
>  {
>  	struct mptcp_pm_addr_entry *entry;
> -	struct mptcp_addr_info skc_local;
> -	struct mptcp_addr_info msk_local;
>  	struct pm_nl_pernet *pernet;
>  	int ret = -1;
>  
> -	if (WARN_ON_ONCE(!msk))
> -		return -1;
> -
> -	/* The 0 ID mapping is defined by the first subflow, copied into the msk
> -	 * addr
> -	 */
> -	mptcp_local_address((struct sock_common *)msk, &msk_local);
> -	mptcp_local_address((struct sock_common *)skc, &skc_local);
> -	if (mptcp_addresses_equal(&msk_local, &skc_local, false))
> -		return 0;
> -
> -	if (mptcp_pm_is_userspace(msk))
> -		return mptcp_userspace_pm_get_local_id(msk, &skc_local);
> -
>  	pernet = pm_nl_get_pernet_from_msk(msk);
>  
>  	rcu_read_lock();
>  	list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
> -		if (mptcp_addresses_equal(&entry->addr, &skc_local, entry->addr.port)) {
> +		if (mptcp_addresses_equal(&entry->addr, skc, entry->addr.port)) {
>  			ret = entry->addr.id;
>  			break;
>  		}
> @@ -1095,7 +1079,7 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
>  	if (!entry)
>  		return -ENOMEM;
>  
> -	entry->addr = skc_local;
> +	entry->addr = *skc;
>  	entry->addr.id = 0;
>  	entry->addr.port = 0;
>  	entry->ifindex = 0;
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 6e6cffc04ced..8a2e01d10582 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -916,13 +916,13 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb,
>  bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
>  			     struct mptcp_rm_list *rm_list);
>  int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
> +int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
>  int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
>  
>  void __init mptcp_pm_nl_init(void);
>  void mptcp_pm_nl_work(struct mptcp_sock *msk);
>  void mptcp_pm_nl_rm_subflow_received(struct mptcp_sock *msk,
>  				     const struct mptcp_rm_list *rm_list);
> -int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
>  unsigned int mptcp_pm_get_add_addr_signal_max(const struct mptcp_sock *msk);
>  unsigned int mptcp_pm_get_add_addr_accept_max(const struct mptcp_sock *msk);
>  unsigned int mptcp_pm_get_subflows_max(const struct mptcp_sock *msk);
> 
> -- 
> 2.40.1
> 
>