Re: [PATCH mptcp-next v2 7/8] mptcp: add mptcp_pm_addr_id_bitmap_t type

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 13/12/2024 08:35, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Similar to defining types such as nodemask_t, dma_cap_mask_t and
> cpumask_t to simplify the use of bitmap, a new type for MPTCP
> userspace pm id bitmap, mptcp_pm_addr_id_bitmap_t is defined to
> easily modify dump_addr() interface of the path managers to accept
> an mptcp_pm_addr_id_bitmap_t type parameter. It also allows this
> parameter of dump_addr() can be modified by BPF program when
> implementing this interface of a BFP path manager. Because a
> dump_addr() interface that accepts an 'unsigned long *bitmap'
> or 'unsigned long bitmap[]' parameter is difficult to implement
> in BPF program.
> 
> In addition, this also makes it easier for us to implement similar
> logic to mptcp_userspace_pm_append_new_local_addr() in BPF path
> manager, because there's no way to use DECLARE_BITMAP macro in BPF
> program, and it's not easy to reimplement it in BPF.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  include/net/mptcp.h      |  7 +++++++
>  net/mptcp/pm_userspace.c | 14 ++++++--------
>  net/mptcp/protocol.h     |  3 ---
>  3 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 814b5f2e3ed5..220b1f60e8c1 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -120,6 +120,13 @@ struct mptcp_sched_ops {
>  	void (*release)(struct mptcp_sock *msk);
>  } ____cacheline_aligned_in_smp;
>  
> +/* max value of mptcp_addr_info.id */
> +#define MPTCP_PM_MAX_ADDR_ID		U8_MAX
> +
> +typedef struct {
> +	DECLARE_BITMAP(map, MPTCP_PM_MAX_ADDR_ID + 1);
> +} mptcp_pm_addr_id_bitmap_t;

Why do you need to declare it here and not in protocol.h? Is it for the
future BPF PM?

Maybe you don't really need this bitmap if it is only needed for the
dump interfaces that are probably not needed with BPF?

Then maybe there is no need to change anything here?

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