Re: [PATCH mptcp-next v1 4/6] mptcp: pm: in-kernel: drop is_userspace in remove_id_zero

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 27/02/2025 07:43, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> There're duplicate operations in mptcp_nl_remove_subflow_and_signal_addr()
> and mptcp_nl_remove_id_zero_address(), both of which traverse all mptcp
> sockets in the net namespace. This patch drops the traversal operation in
> the latter and reuse the traversal loop of the former to do the removal of
> id zero address.

I'm hesitating here. It "feels" wrong to deal with an endpoint having 0
for the ID in the in-kernel manager code. Currently, the code is
separated from the beginning, and even if there is bit of duplication,
it looks clearer to deal with this special case.

(Also, I don't really see a use-case to delete only the first subflows
of all connections, but that's a different topic).

> An additional benefit is that the check for mptcp_pm_is_userspace() in
> mptcp_nl_remove_id_zero_address() is dropped, which reduces the path
> manager's reliance on mptcp_pm_is_userspace() and mptcp_pm_is_kernel()
> helpers.

I think these checks here make sense: we are iterating over all
connections, but we just need the ones handled by the in-kernel PM.

When the pm->ops will be there, can you not simply replace all these
checks under a mptcp_token_iter_next() by something like:

  if (&msk->pm.ops != &mptcp_pm_netlink)
      continue;

or:

  if (mptcp_pm_ops(msk) != mptcp_pm_netlink)

or with a macro for the iterator:

  mptcp_pm_for_each_msk(net, &s_slot, &s_num, &mptcp_pm_netlink) {

WDYT?

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