Re: [PATCH mptcp-next 7/8] mptcp: add check_id for lookup_anno_list_by_saddr

[Matthieu Baerts <matttbe@kernel.org>]

On 21/02/2024 7:31 am, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Add a new parameter check_id for mptcp_lookup_anno_list_by_saddr(), and
> pass it to mptcp_addresses_equal(). Then in mptcp_pm_del_add_timer(),
> the input parameter check_id can be passed as the new parameter into
> mptcp_lookup_anno_list_by_saddr(). After this, this condition:
> 
>         (!check_id || entry->addr.id == addr->id)
> 
> can be dropped, only test if 'entry' is NULL is enough.

(...)

> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 1bf5c3440f84..4519dfa79775 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c

(...)

> @@ -329,12 +330,12 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
>  	struct sock *sk = (struct sock *)msk;
>  
>  	spin_lock_bh(&msk->pm.lock);
> -	entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
> -	if (entry && (!check_id || entry->addr.id == addr->id))
> +	entry = mptcp_lookup_anno_list_by_saddr(msk, addr, check_id);
> +	if (entry)
>  		entry->retrans_times = ADD_ADDR_RETRANS_MAX;
>  	spin_unlock_bh(&msk->pm.lock);
>  
> -	if (entry && (!check_id || entry->addr.id == addr->id))
> +	if (entry)

Instead of all these modifications in mptcp_addresses_equal() and
mptcp_lookup_anno_list_by_saddr() just for this specific case here, can
we not use an extra variable?

  bool stop_retrans;

  (...)

  spin_lock_bh(&msk->pm.lock);
  entry = mptcp_lookup_anno_list_by_saddr(msk, addr);
  stop_retrans = entry && (!check_id || entry->addr.id == addr->id);

  if (stop_retrans)  // or in one line, merged with ↑ ?
      entry->retrans_times = ADD_ADDR_RETRANS_MAX;
  spin_unlock_bh(&msk->pm.lock);

  if (stop_retrans)
      sk_stop_timer_sync(sk, &entry->add_timer);

>  		sk_stop_timer_sync(sk, &entry->add_timer);
>  
>  	return entry;
(...)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.