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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
 <PAXPR07MB7984195B7980F92B26571777A3412@PAXPR07MB7984.eurprd07.prod.outlook.com>
Date: Sat, 19 Oct 2024 15:59:37 +0000
From: "Chia-Yu Chang (Nokia)" <chia-yu.chang@...ia-bell-labs.com>
To: Stephen Hemminger <stephen@...workplumber.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"dsahern@...nel.org" <dsahern@...nel.org>, "ij@...nel.org" <ij@...nel.org>,
	"ncardwell@...gle.com" <ncardwell@...gle.com>, "Koen De Schepper (Nokia)"
	<koen.de_schepper@...ia-bell-labs.com>, "g.white@...leLabs.com"
	<g.white@...leLabs.com>, "ingemar.s.johansson@...csson.com"
	<ingemar.s.johansson@...csson.com>, "mirja.kuehlewind@...csson.com"
	<mirja.kuehlewind@...csson.com>, "cheshire@...le.com" <cheshire@...le.com>,
	"rs.ietf@....at" <rs.ietf@....at>, "Jason_Livingood@...cast.com"
	<Jason_Livingood@...cast.com>, "vidhi_goel@...le.com" <vidhi_goel@...le.com>,
	Olga Albisser <olga@...isser.org>, "Olivier Tilmans (Nokia)"
	<olivier.tilmans@...ia.com>, Henrik Steen <henrist@...rist.net>, Bob Briscoe
	<research@...briscoe.net>
Subject: RE: [PATCH v3 net-next 1/1] sched: Add dualpi2 qdisc

Hi Stephen,

	I will update patch to add blank line after each #define.

	I also prepared one corresponding patch for iprout2-next, and that patch will be submitted after this patch is in net-next.
	The detail of that corresponding patch is at https://github.com/L4STeam/iproute2-next/commit/c9b160b8a02c75a3284b3aa41bf4173060fa5ee1

Brs,
Chia-Yu

-----Original Message-----
From: Stephen Hemminger <stephen@...workplumber.org> 
Sent: Saturday, October 19, 2024 5:48 PM
To: Chia-Yu Chang (Nokia) <chia-yu.chang@...ia-bell-labs.com>
Cc: netdev@...r.kernel.org; davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com; dsahern@...nel.org; ij@...nel.org; ncardwell@...gle.com; Koen De Schepper (Nokia) <koen.de_schepper@...ia-bell-labs.com>; g.white@...leLabs.com; ingemar.s.johansson@...csson.com; mirja.kuehlewind@...csson.com; cheshire@...le.com; rs.ietf@....at; Jason_Livingood@...cast.com; vidhi_goel@...le.com; Olga Albisser <olga@...isser.org>; Olivier Tilmans (Nokia) <olivier.tilmans@...ia.com>; Henrik Steen <henrist@...rist.net>; Bob Briscoe <research@...briscoe.net>
Subject: Re: [PATCH v3 net-next 1/1] sched: Add dualpi2 qdisc

[You don't often get email from stephen@...workplumber.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

CAUTION: This is an external email. Please be very careful when clicking links or opening attachments. See the URL nok.it/ext for additional information.



On Sat, 19 Oct 2024 01:14:19 +0200
chia-yu.chang@...ia-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.