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

[Matthieu Baerts <matttbe@kernel.org>]

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"?

> Together they separate "the guard held the penalty back" (PenalCandidate
> high, CwndPenalized ~0) from "the trigger never fired" (both ~0), which is
> what the receive-window-limited case needs to be read correctly.
> 
> Co-developed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
>  net/mptcp/mib.c      | 2 ++
>  net/mptcp/mib.h      | 2 ++
>  net/mptcp/protocol.c | 8 ++++++++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
> index d9bd4f4afcc0..1299613e183b 100644
> --- a/net/mptcp/mib.c
> +++ b/net/mptcp/mib.c
> @@ -88,6 +88,8 @@ static const struct snmp_mib mptcp_snmp_list[] = {
>  	SNMP_MIB_ITEM("BacklogDrop", MPTCP_MIB_BACKLOGDROP),
>  	SNMP_MIB_ITEM("RcvPruned", MPTCP_MIB_RCVPRUNED),
>  	SNMP_MIB_ITEM("OfoPruned", MPTCP_MIB_OFO_PRUNED),
> +	SNMP_MIB_ITEM("CwndPenalized", MPTCP_MIB_CWNDPENALIZED),
> +	SNMP_MIB_ITEM("PenalCandidate", MPTCP_MIB_PENALCAND),
>  };
>  
>  /* mptcp_mib_alloc - allocate percpu mib counters
> diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
> index 18f35f7e0a2d..93b3a7e9584f 100644
> --- a/net/mptcp/mib.h
> +++ b/net/mptcp/mib.h
> @@ -91,6 +91,8 @@ enum linux_mptcp_mib_field {
>  	MPTCP_MIB_BACKLOGDROP,		/* Backlog over memory limit */
>  	MPTCP_MIB_RCVPRUNED,		/* Dropped due to memory constrains */
>  	MPTCP_MIB_OFO_PRUNED,		/* MPTCP-level OoO queue pruned */
> +	MPTCP_MIB_CWNDPENALIZED,	/* DEBUG: subflow cwnd halved by the scheduler (#345) */
> +	MPTCP_MIB_PENALCAND,		/* DEBUG: picker chose a slow (low-rate) subflow */
>  	__MPTCP_MIB_MAX
>  };
>  
> 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.

> +		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_PENALCAND);
>  	subflow->penalise = fastest && ssk != fastest &&
>  			    (u64)subflow->avg_pacing_rate * MPTCP_PENALISE_RATE_RATIO < max_pace &&
>  			    inet_csk(ssk)->icsk_ca_state == TCP_CA_Open &&
> 

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