| 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
| ||
|
Message-ID: <m2o6wnt8to.fsf@gmail.com>
Date: Wed, 23 Apr 2025 12:29:23 +0100
From: Donald Hunter <donald.hunter@...il.com>
To: chia-yu.chang@...ia-bell-labs.com
Cc: xandfury@...il.com, netdev@...r.kernel.org, dave.taht@...il.com,
pabeni@...hat.com, jhs@...atatu.com, kuba@...nel.org,
stephen@...workplumber.org, xiyou.wangcong@...il.com, jiri@...nulli.us,
davem@...emloft.net, edumazet@...gle.com, horms@...nel.org,
andrew+netdev@...n.ch, ast@...erby.net, liuhangbin@...il.com,
shuah@...nel.org, linux-kselftest@...r.kernel.org, ij@...nel.org,
ncardwell@...gle.com, 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
Subject: Re: [PATCH v12 net-next 1/5] Documentation: netlink: specs: tc: Add
DualPI2 specification
chia-yu.chang@...ia-bell-labs.com writes:
> From: Chia-Yu Chang <chia-yu.chang@...ia-bell-labs.com>
>
> Introduce the specification of tc qdisc DualPI2 stats and attributes,
> which is the reference implementation of IETF RFC9332 DualQ Coupled AQM
> (https://datatracker.ietf.org/doc/html/rfc9332) providing two different
> queues: low latency queue (L-queue) and classic queue (C-queue).
>
> Signed-off-by: Chia-Yu Chang <chia-yu.chang@...ia-bell-labs.com>
> ---
> Documentation/netlink/specs/tc.yaml | 144 ++++++++++++++++++++++++++++
> 1 file changed, 144 insertions(+)
The syntax is not valid so this doesn't pass the schema check and
presumably hasn't been tested. Please validate YNL .yaml additions e.g.
./tools/net/ynl/pyynl/cli.py \
--spec Documentation/netlink/specs/tc.yaml \
--list-ops
...
jsonschema.exceptions.ValidationError: Additional properties are not allowed ('entries' was unexpected)
...
On instance['attribute-sets'][30]['attributes'][14]:
{'name': 'gso_split',
'type': 'flags',
'doc': 'Split aggregated skb or not',
'entries': ['split_gso', 'no_split_gso']}
>
> diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml
> index aacccea5dfe4..08255bba81c4 100644
> --- a/Documentation/netlink/specs/tc.yaml
> +++ b/Documentation/netlink/specs/tc.yaml
> @@ -816,6 +816,58 @@ definitions:
> -
> name: drop-overmemory
> type: u32
> + -
> + name: tc-dualpi2-xstats
> + type: struct
> + members:
> + -
> + name: prob
> + type: uint
> + doc: Current probability
> + -
> + name: delay_c
Please use dashes in member names, e.g. "delay-c", to follow YNL
conventions. Same for all member and attribute names below.
> + type: uint
> + doc: Current C-queue delay in microseconds
> + -
> + name: delay_l
> + type: uint
> + doc: Current L-queue delay in microseconds
> + -
> + name: pkts_in_c
> + type: uint
> + doc: Number of packets enqueued in the C-queue
> + -
> + name: pkts_in_l
> + type: uint
> + doc: Number of packets enqueued in the L-queue
> + -
> + name: maxq
> + type: uint
> + doc: Maximum number of packets seen by the DualPI2
> + -
> + name: ecn_mark
> + type: uint
> + doc: All packets marked with ecn
> + -
> + name: step_mark
> + type: uint
> + doc: Only packets marked with ecn due to L-queue step AQM
> + -
> + name: credit
> + type: int
> + doc: Current credit value for WRR
> + -
> + name: memory_used
> + type: uint
> + doc: Memory used in bytes by the DualPI2
> + -
> + name: max_memory_used
> + type: uint
> + doc: Maximum memory used in bytes by the DualPI2
> + -
> + name: memory_limit
> + type: uint
> + doc: Memory limit in bytes
> -
> name: tc-fq-pie-xstats
> type: struct
> @@ -2299,6 +2351,92 @@ attribute-sets:
> -
> name: quantum
> type: u32
> + -
> + name: tc-dualpi2-attrs
> + attributes:
> + -
> + name: limit
> + type: uint
> + doc: Limit of total number of packets in queue
> + -
> + name: memlimit
> + type: uint
> + doc: Memory limit of total number of packets in queue
> + -
> + name: target
> + type: uint
> + doc: Classic target delay in microseconds
> + -
> + name: tupdate
> + type: uint
> + doc: Drop probability update interval time in microseconds
> + -
> + name: alpha
> + type: uint
> + doc: Integral gain factor in Hz for PI controller
> + -
> + name: beta
> + type: uint
> + doc: Proportional gain factor in Hz for PI controller
> + -
> + name: step_thresh
> + type: uint
> + doc: L4S step marking threshold in microseconds or in packet (see step_packets)
> + -
> + name: step_packets
> + type: flags
> + doc: L4S Step marking threshold unit
> + entries:
> + - microseconds
> + - packets
This is not valid syntax. Enumerations and sets of flags need to be
defined separately. For example, look at the definition of tc-cls-flags
and its usage.
BUT step_packets is defined as a boolean in the implementation so could
be implemented as a boolean flag in the API. If it needs to be
extensible in future then it should be declared as an enum in uAPI and
defined in this spec as an enum. Either way, the parsing and policy in
patch 2 should be made more robust.
> + -
> + name: min_qlen_step
> + type: uint
> + doc: Packets enqueued to the L-queue can apply the step threshold when the queue length of L-queue is larger than this value. (0 is recommended)
> + -
> + name: coupling_factor
> + type: uint
> + doc: Probability coupling factor between Classic and L4S (2 is recommended)
> + -
> + name: drop_overload
> + type: flags
> + doc: Control the overload strategy (drop to preserve latency or let the queue overflow)
> + entries:
> + - drop_on_overload
> + - overflow
Not valid syntax. Use a boolean flag or define an enum.
> + -
> + name: drop_early
> + type: flags
> + doc: Decide where the Classic packets are PI-based dropped or marked
> + entries:
> + - drop_enqueue
> + - drop_dequeue
Not valid syntax. Use a boolean flag or define an enum.
> + -
> + name: classic_protection
> + type: uint
> + doc: Classic WRR weight in percentage (from 0 to 100)
> + -
> + name: ecn_mask
> + type: flags
> + doc: Configure the L-queue ECN classifier
> + entries:
> + - l4s_ect
> + - any_ect
Not valid syntax. Type should probably match implementation, unless you
want to enumerate the valid values by definining an enum.
> + -
> + name: gso_split
> + type: flags
> + doc: Split aggregated skb or not
> + entries:
> + - split_gso
> + - no_split_gso
Not valid syntax. Use a boolean flag or define an enum.
> + -
> + name: max_rtt
> + type: uint
> + doc: The maximum expected RTT of the traffic that is controlled by DualPI2 in usec
> + -
> + name: typical_rtt
> + type: uint
> + doc: The typical base RTT of the traffic that is controlled by DualPI2 in usec
> -
> name: tc-ematch-attrs
> attributes:
> @@ -3679,6 +3817,9 @@ sub-messages:
> -
> value: drr
> attribute-set: tc-drr-attrs
> + -
> + value: dualpi2
> + attribute-set: tc-dualpi2-attrs
> -
> value: etf
> attribute-set: tc-etf-attrs
> @@ -3846,6 +3987,9 @@ sub-messages:
> -
> value: codel
> fixed-header: tc-codel-xstats
> + -
> + value: dualpi2
> + fixed-header: tc-dualpi2-xstats
> -
> value: fq
> fixed-header: tc-fq-qd-stats
Powered by blists - more mailing lists