Re: [PATCH mptcp-next 3/3] DO-NOT-MERGE: mptcp: sched: penalise counters

Hi Matt,

On Wed, 2026-07-29 at 13:50 +0200, Matthieu Baerts wrote:
> Hi Shardul,
> 
> On 26/07/2026 07:55, Shardul Bankar wrote:
> > Instrumentation for validating the two preceding patches; not for
> > merge.
> > 
> > Adds two MPTcpExt SNMP counters:
> > - CwndPenalized: times a subflow cwnd was actually halved;
> > - PenalCandidate: times the rate trigger picked a slow subflow.
> 
> I think at least the first counter is interesting, probably the
> second
> one as well, no?
> 
> Generally, if you need counters during the development, they might be
> needed to debug issues. But here with the scheduler, maybe the
> tracing
> are better. Did you use "trace_mptcp_subflow_get_send"?
> 

I did not. Looking at it now, it fires before the penalise decision and
does not see the halving, so it would need a small extension (a new
field, or a tracepoint at the halving point) to show what these
counters show. Which would you prefer for v2: keep the DO-NOT-MERGE
counters, or add that tracepoint? I will go with your call.

> > 
> >  
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index 7cbc5aa17e22..80866f09831a 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -1607,6 +1607,7 @@ static void mptcp_penalise_cwnd(struct sock
> > *ssk)
> >         subflow->penalise = false;
> >         subflow->last_penalise = tcp_jiffies32;
> >         tcp_snd_cwnd_set(tp, max_t(u32, cwnd >> 1, 2));
> > +       MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_CWNDPENALIZED);
> >         if (cwnd >= tp->snd_ssthresh)
> >                 tp->snd_ssthresh = max_t(u32, tp->snd_ssthresh >>
> > 1, 2);
> >  }
> > @@ -1694,6 +1695,13 @@ struct sock *mptcp_subflow_get_send(struct
> > mptcp_sock *msk)
> >          * once per RTT.
> >          */
> >         subflow = mptcp_subflow_ctx(ssk);
> > +       /* DEBUG: count how often the trigger picks a slow path, so
> > a gated-off
> > +        * run (PenalCandidate high, CwndPenalized 0) is
> > distinguishable from one
> > +        * where the trigger never fired.
> > +        */
> > +       if (fastest && ssk != fastest &&
> > +           (u64)subflow->avg_pacing_rate *
> > MPTCP_PENALISE_RATE_RATIO < max_pace)
> 
> If this counter is interesting, you could have a variable for
> penal_cand
> and use it below.
> 

Got it, a single penal_cand local will now feed both the counter and
the penalise decision.

Thanks,
Shardul