Re: [PATCH net v5 1/2] mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint
Matthieu Baerts <[email protected]>
| Newsgroups | dev.linux.lists.mptcp |
|---|---|
| Organization | NGI0 Core |
| Message-ID | <[email protected]> |
Hi Kalpan,
On 07/08/2026 09:23, Kalpan Jani wrote:
> Sorry for the long silence on this.
Np, I was overloaded anyway (still am)
> The packetdrill test is now up at
> https://github.com/multipath-tcp/packetdrill/pull/201 - I rewrote it and this
> time validated it in the virtme-docker environment: 3 failing without the
> kernel fix, 3 passing with it, on export/20260622T173908. Your suggestion about
> the endpoint linked to the initial subflow is what unblocked it. Details and
> the limitations I hit are in the PR.
Thanks, I just send some minor fixes, then merged. I hope that's OK.
> Two things about v5 itself.
>
> First, patch 1/2 is unchanged from what you have. I said I would rework the
> addr argument because the anno_addr copy looked fragile for endpoints with a
> port, but after re-checking I no longer think that case is reachable: the MPC
> endpoint cannot be a ported endpoint, since both __lookup_addr() and
> mptcp_nl_add_subflow_or_signal_addr() compare the port when the endpoint has
> one, and that would require it to match the initial subflow's local port -
> which mptcp_pm_nl_create_listen_socket() binding the same address:port makes
> unreachable in practice. So the entry reachable from
> mptcp_nl_remove_id_zero_address() always has port 0 and clearing it is correct.
> Tell me if you see a case I have missed.
Fine by me.
> Second, and I should have raised this earlier: please drop patch 2/2. The
> selftest does not exercise the fixed path. I used addr_nr_ns1=-1, which deletes
> the endpoint by its real ID via pm_nl_del_endpoint ${listener_ns} $id and
> therefore goes through mptcp_nl_remove_subflow_and_signal_addr() - the path
> that was already correct. Only -9 reaches mptcp_nl_remove_id_zero_address(),
> and the endpoint is 10.0.2.1, so the address check there would not have matched
> even with -9. The subtest passes identically with and without patch 1/2, which
> is why CI was green everywhere, and it largely duplicates the existing "remove
> single address" subtest. Sorry for not catching that before sending v5.
>
> With the packetdrill test covering the same ground, I think patch 1/2 alone is
> the right thing to apply. Happy to send a corrected mptcp_join.sh subtest as
> well if you would rather have both.
Indeed, dropped.
Reviewed-by: Matthieu Baerts (NGI0) <[email protected]>
> From: Kalpan Jani <[email protected]>
BTW, do you mind avoiding top-posting please, it is not recommended on
most ML:
https://docs.kernel.org/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions
New patches for t/upstream-net and t/upstream:
- a6a9ee8fc58c: mptcp: pm: drop pending ADD_ADDR when removing id 0 endpoint
- Results: fb78b98bf563..4439056e6ccd (export-net)
- Results: 6f0836d81132..e8891858656d (export)
Tests are now in progress:
- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/420be436959ff2164ff3087f88e6a9756d485436/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/c808c1fcb72b4387c5d2193a4c69f387d5a7e881/checks
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.