Hi Anil,
On 12/10/2026 19:17, Anil Kaushik wrote:
> With the in-kernel PM, a subflow closed after an error is never
> re-created, even once the path is back (#440). For example, with fiber
> as main link and Starlink as backup, connections never return to the
> fiber after an outage.
Thank you for looking at that. Here is a quick review (based on the
preview you sent earlier). I might not have the opportunity to do more
before November.
> Patch 1 fixes a selftest flake: write() can return EAGAIN after POLLOUT
> when a subflow closes.
When did you see this? With the other patches applied or before? Just
wonder if this is worth backporting this, but I don't think I saw the
issue before (or maybe it was very rare). Maybe we would need a Fixes tag?
(The "Assisted-by" tag is only used on this patch, did you want to add
it on the other ones as well?)
> Patch 2 adds an opt-in sysctl, net.mptcp.subflow_retry_timeout, to
> re-create such subflows with an exponential backoff.
> Patch 3 adds the selftests.
>
> Questions:
> - sysctl, or an endpoint flag?
I still need to think if this should be a per-endpoint setting, or a per
netns one. I guess people might want to use it by default, and disable
it in some cases. Possibly this would need to be disabled per endpoint
with a flag, I don't know. We might not want to dynamically react when
the flag is changed, (except if this is really needed): that caused
enough issues in the past.
With a sysctl, it is a bit "strange" because we have in-kernel PM
options that are all configurable via netlink (ip mptcp). An alternative
is to have general settings per netns configurable via netlink, similar
to how the limits are configured. (In fact, that can also be an extra
limit in fact). Maybe later, we could have a flag to disable retries per
endpoint.
> - the re-created initial subflow counts in the subflows limit, OK?
The "subflows" limit is for the extra subflows. So yes, if you have 2
paths with 1 extra one and the initial one is recreated, it now counts
as an extra one.
> - should retries stop after some attempts?
I guess this should be similar to what TCP is doing with the "retrans"
parameters: set a maximum number of attempts. No?
We usually try to reduce the amount of data stored/copied here. I'm not
a fan of these new entries: here, a re-connect will already retry a few
times before giving up (depending on syn_retries). We will need to deal
with two ongoing timers, that doesn't look good. Can we not instead only
deal with subflow sockets and their timers? If an attempt fails (and the
socket creation timer is stopped), retry later with the same socket?
Now, regarding the code, if it is specific to the in-kernel one, it
should go exclusively to pm_kernel.c: pm.c is for shared code between PMs.
For the selftests, maybe better to use netfilter to block links instead
of TC? Maybe can you have a single test where there is a failure on the
extra subflow, then on the initial one? Try to have a "quick" test, it
is not clear how long this one takes, but it would feel better to run a
very long connection in the background, test what you need to test with
the 'wait_*' functions as you did, then stop it, similar to what you can
see in many tests from "endpoint_tests".
Regarding the tests, just in case, there is a docker image to help you
there:
https://github.com/multipath-tcp/mptcp-upstream-virtme-docker
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.