netdev - Re: [PATCH net-next v2] fq_codel: generalise ce_threshold marking for subset of traffic

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Oct 2021 23:16:32 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Eric Dumazet <eric.dumazet@...il.com>, davem@...emloft.net,
        kuba@...nel.org
Cc:     netdev@...r.kernel.org, edumazet@...gle.com
Subject: Re: [PATCH net-next v2] fq_codel: generalise ce_threshold marking
 for subset of traffic

Eric Dumazet <eric.dumazet@...il.com> writes:

> On 10/19/21 10:47 AM, Toke Høiland-Jørgensen wrote:
>> The commit in the Fixes tag expanded the ce_threshold feature of FQ-CoDel
>> so it can be applied to a subset of the traffic, using the ECT(1) bit of
>> the ECN field as the classifier. However, hard-coding ECT(1) as the only
>> classifier for this feature seems limiting, so let's expand it to be more
>> general.
>> 
>> To this end, change the parameter from a ce_threshold_ect1 boolean, to a
>> one-byte selector/mask pair (ce_threshold_{selector,mask}) which is applied
>> to the whole diffserv/ECN field in the IP header. This makes it possible to
>> classify packets by any value in either the ECN field or the diffserv
>> field. In particular, setting a selector of INET_ECN_ECT_1 and a mask of
>> INET_ECN_MASK corresponds to the functionality before this patch, and a
>> mask of ~INET_ECN_MASK allows using the selector as a straight-forward
>> match against a diffserv code point:
>> 
>>  # apply ce_threshold to ECT(1) traffic
>>  tc qdisc replace dev eth0 root fq_codel ce_threshold 1ms ce_threshold_selector 0x1/0x3
>> 
>>  # apply ce_threshold to ECN-capable traffic marked as diffserv AF22
>>  tc qdisc replace dev eth0 root fq_codel ce_threshold 1ms ce_threshold_selector 0x50/0xfc
>> 
>> Regardless of the selector chosen, the normal rules for ECN-marking of
>> packets still apply, i.e., the flow must still declare itself ECN-capable
>> by setting one of the bits in the ECN field to get marked at all.
>> 
>> v2:
>> - Add tc usage examples to patch description
>> 
>> Fixes: e72aeb9ee0e3 ("fq_codel: implement L4S style ce_threshold_ect1 marking")
>> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
>> ---
>
> Reviewed-by: Eric Dumazet <edumazet@...gle.com>

Thanks!

> BTW, the Fixes: tag seems not really needed, your patch is a
> followup/generalization.

Yeah, I included it because I don't know of any other way to express
"this is a follow-up commit to this other one, and the two should be
kept together" - for e.g., backports. And I figured that since this
changes the UAPI from your initial patch, this was important to express
in case someone does backport that.

Is there another way to express this that I'm not aware of?

-Toke