Re: [PATCH mptcp-next v2 0/8] BPF path manager, part 2

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 13/12/2024 08:35, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v2:
>  - split patch 7 of v1 into two.
>  - patches 1-6 have no code changes, only the commit logs have been
> updated.
>  - more commit log to explain why mptcp_pm_addr_id_bitmap_t needs to
> be defined.
>  - use mptcp_pm_addr_id_bitmap_t in userspace_pm_append_new_local_addr
> too.

Sorry for the delay: I was waiting for the CI to do the validation, but
somehow, it didn't even start. An issue on Patchew side apparently:

  https://patchew.org/MPTCP/cover.1734074788.git.tanggeliang@kylinos.cn/

The 6 first patches look good to me (just one small thing, and a rebase
might be needed).

It is unclear to me if we need the 2 last patches. Maybe patch 7/8 is
needed for the BPF path-manager? Not even sure, please see below.

Regarding patch 8/8, it looks like it introduces new issues, and solving
them increases the complexity, and I don't think it is worth saving a
bit of duplicated code there. WDYT?

> In order to implement BPF userspace path manager, it is necessary to
> unify the interfaces of the path manager. This set updates get_addr()
> and dump_addr() interfaces.
The modifications you are proposing here makes sense. Now, regarding the
BPF path-manager, to me, they don't really need to implement get_addr()
and dump_addr() because these interfaces are only useful for the userspace:

- with the in-kernel PM, it is useful to list the different endpoints to
be able to change the configuration if needed (or to check how endpoints
got configured).

- with the userspace one, it can be useful to retrieve addresses when
the daemon got restarted, to get info about previously created connections.

- with the BPF PM, I'm not sure if there is really a need to dump
addresses to the userspace. Or do you see a case where these interfaces
can be useful? I guess BPF PMs will be configured via BPF, and if there
is a need, the addresses can also be dumped via BPF, but I don't see the
point of exporting them via Netlink.

Maybe there is no need to do much around these interfaces, and the last
two patches can be dropped? Or maybe mptcp_pm_addr_id_bitmap_t can be
defined in protocol.h instead of in include/net/mptcp.h?

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