Re: [net-next,v8,09/15] quic: add congestion control

Simon Horman <[email protected]>
Newsgroups dev.linux.lists.kernel-tls-handshake,dev.linux.lists.quic,org.kernel.vger.linux-cifs,org.kernel.vger.netdev
Message-ID <[email protected]>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
quic: add congestion control

This patch introduces quic_cong for RTT measurement and congestion
control. It implements a congestion control state machine with slow
start, congestion avoidance, and recovery phases using New Reno.

> diff --git a/net/quic/cong.c b/net/quic/cong.c
> new file mode 100644
> index 000000000000..1a8b7f8db977
> --- /dev/null
> +++ b/net/quic/cong.c

[ ... ]

> +/* rfc9002#section-5: Estimating the Round-Trip Time */
> +void quic_cong_rtt_update(struct quic_cong *cong, u64 time, u32 ack_delay)
> +{
> +	u32 adjusted_rtt, rttvar_sample;

[ ... ]

> +	/* rfc9002#section-5.3:
> +	 *   adjusted_rtt = latest_rtt
> +	 *   if (latest_rtt >= min_rtt + ack_delay):
> +	 *     adjusted_rtt = latest_rtt - ack_delay
> +	 *   smoothed_rtt = 7/8 * smoothed_rtt + 1/8 * adjusted_rtt
> +	 *   rttvar_sample = abs(smoothed_rtt - adjusted_rtt)
> +	 *   rttvar = 3/4 * rttvar + 1/4 * rttvar_sample
> +	 */
> +	adjusted_rtt = cong->latest_rtt;
> +	if (cong->latest_rtt >= cong->min_rtt + ack_delay)
> +		adjusted_rtt = cong->latest_rtt - ack_delay;
> +
> +	cong->smoothed_rtt = (cong->smoothed_rtt * 7 + adjusted_rtt) / 8;
> +	if (cong->smoothed_rtt >= adjusted_rtt)
> +		rttvar_sample = cong->smoothed_rtt - adjusted_rtt;
> +	else
> +		rttvar_sample = adjusted_rtt - cong->smoothed_rtt;

The comment above references abs(smoothed_rtt - adjusted_rtt). Paolo Abeni
suggested using abs_diff() from linux/math.h for this calculation in v3
review, and it was acknowledged. Could abs_diff() be used here instead of
the manual if-else?

https://lore.kernel.org/netdev/[email protected]/

[ ... ]

> diff --git a/net/quic/cong.h b/net/quic/cong.h
> new file mode 100644
> index 000000000000..e6cfb0fa1b6c
> --- /dev/null
> +++ b/net/quic/cong.h

[ ... ]

> +struct quic_cong {
> +	/* RTT tracking */
> +	u32 max_ack_delay;	/* max_ack_delay from rfc9000#section-18.2 */
> +	u32 smoothed_rtt;	/* Smoothed RTT */
> +	u32 latest_rtt;		/* Latest RTT sample */
> +	u32 min_rtt;		/* Lowest observed RTT */
> +	u32 rttvar;		/* RTT variation */
> +	u32 pto;		/* Probe timeout */
> +
> +	/* Timing & pacing */
> +	u64 recovery_time;	/* Recovery period start timestamp */
> +	u64 pacing_rate;	/* Packet sending speed Bytes/sec */
> +	u64 pacing_time;	/* Next scheduled send timestamp (ns) */
> +	u64 time;		/* Cachedached current timestamp */
                                   ^^^^^^^^^^

There appears to be a typo here - "Cachedached" should be "Cached".
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.