Re: [PATCH v3 net-next 1/1] sched: Add dualpi2 qdisc

[Stephen Hemminger <stephen@networkplumber.org>]

On Sat, 19 Oct 2024 01:14:19 +0200
chia-yu.chang@nokia-bell-labs.com wrote:

> +config NET_SCH_DUALPI2
> +	tristate "Dual Queue Proportional Integral Controller Improved with a Square (DUALPI2) scheduler"
> +	help
> +	  Say Y here if you want to use the DualPI2 AQM.
> +	  This is a combination of the DUALQ Coupled-AQM with a PI2 base-AQM.
> +	  The PI2 AQM is in turn both an extension and a simplification of the
> +	  PIE AQM. PI2 makes quite some PIE heuristics unnecessary, while being
> +	  able to control scalable congestion controls like DCTCP and
> +	  TCP-Prague. With PI2, both Reno/Cubic can be used in parallel with
> +	  DCTCP, maintaining window fairness. DUALQ provides latency separation
> +	  between low latency DCTCP flows and Reno/Cubic flows that need a
> +	  bigger queue.
> +	  For more information, please see
> +	  https://datatracker.ietf.org/doc/html/rfc9332

The wording here is awkward and reads a little like a marketing statement.
Please keep it succinct.

> +
> +	  To compile this code as a module, choose M here: the module
> +	  will be called sch_dualpi2.
> +
> +	  If unsure, say N.
> +

> +/* 32b enable to support flows with windows up to ~8.6 * 1e9 packets
> + * i.e., twice the maximal snd_cwnd.
> + * MAX_PROB must be consistent with the RNG in dualpi2_roll().
> + */
> +#define MAX_PROB U32_MAX
> +/* alpha/beta values exchanged over netlink are in units of 256ns */
> +#define ALPHA_BETA_SHIFT 8
> +/* Scaled values of alpha/beta must fit in 32b to avoid overflow in later
> + * computations. Consequently (see and dualpi2_scale_alpha_beta()), their
> + * netlink-provided values can use at most 31b, i.e. be at most (2^23)-1
> + * (~4MHz) as those are given in 1/256th. This enable to tune alpha/beta to
> + * control flows whose maximal RTTs can be in usec up to few secs.
> + */
> +#define ALPHA_BETA_MAX ((1U << 31) - 1)
> +/* Internal alpha/beta are in units of 64ns.
> + * This enables to use all alpha/beta values in the allowed range without loss
> + * of precision due to rounding when scaling them internally, e.g.,
> + * scale_alpha_beta(1) will not round down to 0.
> + */
> +#define ALPHA_BETA_GRANULARITY 6
> +#define ALPHA_BETA_SCALING (ALPHA_BETA_SHIFT - ALPHA_BETA_GRANULARITY)
> +/* We express the weights (wc, wl) in %, i.e., wc + wl = 100 */
> +#define MAX_WC 100

For readability put a blank line after each #define please.

There are lots of parameters in this qdisc, and it would be good
to have some advice on best settings. Like RED the problem is that it is too
easy to get it wrong.

Also, need a patch to iproute-next to support this qdisc.