Re: [PATCH mptcp-next 0/3] mptcp: sched: penalise a slow subflow (#345, first cut for your lab)

Hi Matt,

On Wed, 2026-07-29 at 13:48 +0200, Matthieu Baerts wrote:
> Hi Shardul,
> 
> On 26/07/2026 07:55, Shardul Bankar wrote:
> > 
> > 
> > - 2/3 is a guard that is in neither the fork nor what I described.
> > Without it,
> >   1/3 regresses badly (about 2x slower in my runs) when the
> > connection is
> >   receive-window-limited. In that case the fastest path is capped
> > by the same
> >   shared window, so it cannot absorb what the slow path gives up,
> > and halving
> >   just sheds the slow path's throughput. 2/3 skips the penalty
> > while the
> >   application has queued past the send-window edge (write_seq >
> > wnd_end), which
> >   is the sign that the receiver, not our congestion window, is the
> > bottleneck.
> >   I kept it a separate patch so you can test 1/3 on its own, or
> > drop or retune
> >   2/3 independently. The exact condition is the piece I would most
> > value your
> >   lab checking.
> 
> It feels to me that you require this because patch 1/3 doesn't check
> if
> the MPTCP connection was "send-buffer-limited", no? But you are doing
> something very similar, no? Without testing, it feels like this is
> required not to limit the penalisation to when it is really needed.
> 

Patch 1 does gate on load, with tcp_is_cwnd_limited(); though that is
not the send-buffer-limited check. The send-buffer-limited check never
fires at scheduler time in this tree, as the msk buffer has just
drained into the subflows. Patch 2 guards the receive-window-limited
case, where patch 1 without it can regress the transfer about 2x in my
tests. I have kept it as the explicit guard and am still characterizing
when patch 1 alone would suffice. Would you prefer we drop it?

> > 
> > 
> > I drove those regimes with a small simult_flows variant (receive-
> > window-
> > limited, send-buffer-limited, and autotuned cases). It is a helper,
> > not
> > selftest quality, so I did not fold it into the series; it is on a
> > branch of
> > my tree, in case it saves your lab time or you spot a case I
> > missed:
> > 
> > https://github.com/shardulsdk-mpiric/linux/blob/6926c4b7f583/tools/testing/selftests/net/mptcp/mptcp_sched_penalise.sh
> > 
> > Run it on a baseline and a patched kernel and compare (prefix with
> > MPTCP_LIB_IP_MPTCP=1 if pm_nl_ctl does not work in your setup):
> > 
> >   SCENARIO=suite                             
> > ./mptcp_sched_penalise.sh
> >   SCENARIO=unbounded                         
> > ./mptcp_sched_penalise.sh
> >   SCENARIO=rwnd    RCVBUF=262144             
> > ./mptcp_sched_penalise.sh
> >   SCENARIO=sndbuf  SNDBUF=65536              
> > ./mptcp_sched_penalise.sh
> >   SCENARIO=both    RCVBUF=262144 SNDBUF=65536
> > ./mptcp_sched_penalise.sh
> 
> Sounds good! Did you check with a fixed sndbuf higher than the rcv
> one?
> 

Yes (SNDBUF 256K, RCVBUF 128K). The guard correctly suppresses the
penalty there: the receiver is genuinely at a zero window (receive-
window-limited, not congestion-limited), and it is not slower than
baseline.

> Also, be careful that with netem, the limits you give to run_test()
> can
> influence a lot the bufferbloat. Did you monitor the RTTs during
> these
> transfers?
> 

I do now. The harness samples the subflows' srtt, and it confirms your
point: the netem queue length drives it (srtt max is about 40 ms with
the fast path alone, rising to several seconds on a bufferbloated
path).

> On the other hand, it would be good to validate this with one path
> having bufferbloat. These patches should also help to improve the
> situation. (And issue #332 should help even more)
> 

I added a bufferbloated-slow-path case, but it was too noisy to draw a
firm conclusion: the completion times swung widely, and the same swing
was on the baseline kernel, so my setup is not measuring the effect
cleanly. I would build a more controlled bufferbloat case (a moderate,
stable queue, and a latency metric rather than completion time). I
agree #332 is likely the bigger lever there.

> > For the rwnd/sndbuf/both scenarios the simult_flows pass/fail bound
> > is not
> > meaningful (it assumes both paths are fully used): read the printed
> > runtime
> > and out-of-order counts, not OK/FAIL. The "both" case also
> > occasionally fails
> > to bring up the second subflow with the very small SO_SNDBUF; just
> > rerun it if
> > you see a single-subflow run.
> 
> I see, yes. I think what is important here for #345, is that when the
> transfer is buffer limited, the slow subflow impact should be
> reduced.
> At least not to cause the transfer to be worse than without this slow
> subflow.
> 

Using that as the bar: when send-buffer-limited, the penalised two-path
transfer beats the fast path alone (about 11.3 s against 14.4 s), with
roughly 15 to 20% less out-of-order data, so the slow subflow helps.
When it is bufferbloated, it comes out about even with the fast path
alone.


I have all of these changes ready in my tree. I would rather settle
whether patch 2 stays (above) and the counters question on 3/3 before I
post v2, but I am glad to send v2 now if you would prefer to look at
the code directly.

Thanks,
Shardul