Re: [PATCH mptcp-next 6/8] mptcp: add use_id parameter for addresses_equal

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 21/02/2024 7:31 am, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds a new parameter use_id for mptcp_addresses_equal() to
> test the address ids, as well as the address. This can be used to test
> if the two given addresses are identically equal, they have both the
> same address and the same address id.

(...)

> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 7905783c95e4..1c32f579e1a1 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -670,7 +670,8 @@ void mptcp_set_owner_r(struct sk_buff *skb, struct sock *sk);
>  void mptcp_set_state(struct sock *sk, int state);
>  
>  bool mptcp_addresses_equal(const struct mptcp_addr_info *a,
> -			   const struct mptcp_addr_info *b, bool use_port);
> +			   const struct mptcp_addr_info *b,
> +			   bool use_port, bool use_id);

That's a lot of modifications just to introduce one new parameter. Would
it not be better to use a macro?

  #define mptcp_addresses_equal(a, b, use_port) \
    mptcp_addresses_equal_check_id(a, b, use_port, false)

Or maybe better with an "inline" function?

Or extending the function:

  bool mptcp_addresses_equal_check_id(...)
  {
      return mptcp_addresses_equal(...) ? a->id == b->id : false;
  }

=> Or only do this extra check in mptcp_lookup_anno_list_by_saddr()
where you need that, no?

I will check with Mat what he thinks about that.

>  void mptcp_local_address(const struct sock_common *skc, struct mptcp_addr_info *addr);
>  
>  /* called with sk socket lock held */

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