Page MenuHomeVyOS Platform
Create Task
Maniphest T4989

QoS Policy Limiter - classes for marked traffic do not work
Closed, ResolvedPublicBUG
Actions

Description

How to reproduce the problem:

set qos policy limiter 1G-in class 100 bandwidth '20gbit'
set qos policy limiter 1G-in class 100 burst '3760k'
set qos policy limiter 1G-in class 100 match INTERNAL mark '100'
set qos policy limiter 1G-in class 100 priority '20'
set qos policy limiter 1G-in default bandwidth '1gbit'
set qos policy limiter 1G-in default burst '125000000b'
set qos interface eth0 ingress 1G-in
show qos
interface eth0 {
    ingress 1G-in
}
policy {
    limiter 1G-in {
        class 100 {
            bandwidth 20gbit
            burst 3760k
            match INTERNAL {
                mark 100
            }
            priority 20
        }
        default {
            bandwidth 1gbit
            burst 125000000b
        }
    }
}

Check tc, missing filter with priority 20:

#tc filter show dev eth0 ingress
filter parent ffff: protocol all pref 255 basic chain 0 
filter parent ffff: protocol all pref 255 basic chain 0 handle 0x1 flowid ffff:65 
	action order 1:  police 0x1 rate 1Gbit burst 125000000b mtu 2Kb action drop overhead 0b 
	ref 1 bind 1

tc filters when configured correctly:

filter parent ffff: protocol all pref 20 fw chain 0 
filter parent ffff: protocol all pref 20 fw chain 0 handle 0x64 classid ffff:64 

	action order 1:  police 0xac rate 20Gbit burst 3847500b mtu 2Kb action drop overhead 0b 
	ref 1 bind 1

filter parent ffff: protocol all pref 255 basic chain 0 
filter parent ffff: protocol all pref 255 basic chain 0 handle 0x1 flowid ffff:65 
	action order 1:  police 0xab rate 1Gbit burst 125000000b mtu 2Kb action drop overhead 0b 
	ref 1 bind 1

Details

Difficulty level
Normal (likely a few hours)
Version
1.4-rolling-202302080317
Why the issue appeared?
Design mistake
Is it a breaking change?
Perfectly compatible
Issue type
Bug (incorrect behavior)

Related Objects
Search...

Event Timeline

daniil created this task.Feb 9 2023, 9:20 AM
pasik added a subscriber: pasik.Feb 9 2023, 11:14 AM
c-po claimed this task.Feb 27 2023, 7:30 PM
c-po changed the task status from Open to Needs testing.Mar 1 2023, 7:08 PM
c-po triaged this task as Normal priority.
c-po changed Why the issue appeared? from Will be filled on close to Design mistake.
c-po changed Is it a breaking change? from Behavior change to Perfectly compatible.
MartB added a subscriber: MartB.Mar 9 2023, 1:23 AM

@c-po Isnt this implementation wrong for "shaper" anyway?
The speed should only be taken from the interface as an last resort, if the default bandwidth is configured in a no percentage unit it should be used instead, no?

e.g

set qos policy shaper test bandwidth 100mbit
set qos policy shaper test class 10 bandwidth 50%

It wont be used right now if I add a class that uses a percentage, it will always use the interface speed.
The base __rate_convert should include some check if the "base" speed was already set.

daniil added a comment.Mar 9 2023, 6:35 PM

QoS Policy Limiter now works correctly.
But the shaper classes for tagged traffic don't work.

qos {
interface br100 {
    egress 1G-out
   }
   shaper 1G-out {
         bandwidth 20gbit
         class 100 {
             bandwidth 20gbit
             burst 3760k
             match INTERNAL {
                 mark 100
             }
             queue-type fq-codel
         }
         default {
             bandwidth 1gbit
             burst 188k
             queue-type fq-codel
         }
     }
 }

Marked traffic does not match in class 100. I am not an expert in Linux tc (Traffic Control), but I have seen it work in older versions.
Old version of vyos with correctly working shaper;

# tc filter show dev br100
filter parent 1: protocol all pref 1 fw chain 0 
filter parent 1: protocol all pref 1 fw chain 0 handle 0x64 classid 1:64

tc qdisc show dev br100
qdisc htb 1: root refcnt 2 r2q 12500 default 0x65 direct_packets_stat 0 direct_qlen 1000
qdisc fq_codel 8031: parent 1:65 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb drop_batch 64 
qdisc fq_codel 8032: parent 1:64 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb drop_batch 64 
qdisc ingress ffff: parent ffff:fff1 ----------------

Last rolling version with an incorrectly working shaper;

# tc filter show dev br100
filter parent 1: protocol all pref 255 basic chain 0 
filter parent 1: protocol all pref 255 basic chain 0 handle 0x1 flowid 1:65 
	action order 1:  police 0x2 rate 1Gbit burst 192375b mtu 2Kb action reclassify overhead 0b 
	ref 1 bind 1 

filter parent 1: protocol all pref 49152 fw chain 0 
filter parent 1: protocol all pref 49152 fw chain 0 handle 0x64 classid 1:64 

	action order 1:  police 0x1 rate 20Gbit burst 3847500b mtu 2Kb action reclassify overhead 0b 
	ref 1 bind 1 

# tc qdisc show dev br100
qdisc htb 1: root refcnt 2 r2q 12500 default 0x65 direct_packets_stat 0 direct_qlen 1000
qdisc fq_codel 8003: parent 1:64 limit 10240p flows 1024 quantum 1514 target 4us interval 99us memory_limit 32Mb drop_batch 64 
qdisc fq_codel 8004: parent 1:65 limit 10240p flows 1024 quantum 1514 target 4us interval 99us memory_limit 32Mb drop_batch 64
Viacheslav added a parent task: T5048: QoS doesn't work correctly root task.Jun 17 2023, 12:48 PM
Viacheslav added a subscriber: Viacheslav.Jun 17 2023, 3:57 PM
In T4989#144426, @MartB wrote:
set qos policy shaper test bandwidth 100mbit
set qos policy shaper test class 10 bandwidth 50%

It wont be used right now if I add a class that uses a percentage, it will always use the interface speed.
The base __rate_convert should include some check if the "base" speed was already set.

@MartB it will be fixed in https://github.com/vyos/vyos-1x/pull/2046

Viacheslav added a comment.Jun 19 2023, 9:52 AM

in the VyOS 1.4-rolling-202306190317

set qos interface eth0 egress 'test'
set qos policy shaper test bandwidth '300bit'
set qos policy shaper test class 100 bandwidth '150mbit'
set qos policy shaper test class 100 burst '3760k'
set qos policy shaper test class 100 match internal mark '100'
set qos policy shaper test class 100 queue-type 'fq-codel'
set qos policy shaper test default bandwidth '20mbit'
set qos policy shaper test default burst '188k'
set qos policy shaper test default queue-type 'fq-codel'

comimt

DEBUG/QoS: tc qdisc replace dev eth0 root handle 1: htb r2q 1 default 65
DEBUG/QoS: tc class replace dev eth0 parent 1: classid 1:1 htb rate 300
DEBUG/QoS: tc class replace dev eth0 parent 1:1 classid 1:64 htb rate 150000000 burst 3760k quantum 1514
DEBUG/QoS: tc qdisc replace dev eth0 parent 1:64 sfq
DEBUG/QoS: tc class replace dev eth0 parent 1:1 classid 1:65 htb rate 20000000 burst 188k quantum 1514 prio 20
DEBUG/QoS: tc qdisc replace dev eth0 parent 1:65 sfq
DEBUG/QoS: tc qdisc replace dev eth0 parent 1:64 fq_codel quantum 1514 flows 1024 interval 100 interval 100 target 5 noecn
DEBUG/QoS: tc filter replace dev eth0 parent 1: protocol all handle 100 fw flowid 1:64

Is it correct?

vyos@r14# tc filter show dev eth0
filter parent 1: protocol all pref 49152 fw chain 0 
filter parent 1: protocol all pref 49152 fw chain 0 handle 0x64 classid 1:64 
[edit]
vyos@r14# 
[edit]
vyos@r14# tc qdisc show dev eth0
qdisc htb 1: root refcnt 2 r2q 1 default 0x65 direct_packets_stat 0 direct_qlen 1000
qdisc sfq 800b: parent 1:65 limit 127p quantum 1514b depth 127 divisor 1024 
qdisc fq_codel 800c: parent 1:64 limit 10240p flows 1024 quantum 1514 target 4us interval 99us memory_limit 32Mb drop_batch 64 
[edit]
vyos@r14#
daniil added a comment.EditedJun 19 2023, 11:13 AM

Now the traffic limiter is broken.

 interface br100 {
     ingress 1G-in
 }
 policy {
     limiter 1G-in {
         default {
             bandwidth 1gbit
             burst 125000000b
         }
     }
}
tc filter show dev br100 ingress

is empty, upload traffic is not limited.

Viacheslav added a comment.EditedJun 19 2023, 1:33 PM
In T4989#150726, @daniil wrote:

Now the traffic limiter is broken.

 interface br100 {
     ingress 1G-in
 }
 policy {
     limiter 1G-in {
         default {
             bandwidth 1gbit
             burst 125000000b
         }
     }
}
tc filter show dev br100 ingress

is empty, upload traffic is not limited.

It seems after this PR https://github.com/vyos/vyos-1x/pull/2044
so it fixes shaper but broke policer
https://vyos.dev/T5295

Viacheslav added a comment.Jul 3 2023, 12:10 PM

@daniil could you re-check?

set qos interface eth0 ingress '1G-in'
set qos policy limiter 1G-in default bandwidth '1gbit'
set qos policy limiter 1G-in default burst '125000000b'

tc:

vyos@r14# tc filter show dev eth0 ingress
filter parent ffff: protocol all pref 255 basic chain 0 
filter parent ffff: protocol all pref 255 basic chain 0 handle 0x1 
	action order 1:  police 0x1 rate 1Gbit burst 125000000b mtu 2Kb action drop overhead 0b 
	ref 1 bind 1
daniil added a comment.Jul 4 2023, 1:36 PM
Version:          VyOS 1.4-rolling-202307040317
interface br100 {
    egress 1G-out
    ingress 1G-in
}
policy {
    limiter 1G-in {
        class 100 {
            bandwidth 20gbit
            burst 3760k
            match INTERNAL {
                mark 100
            }
            priority 20
        }
        default {
            bandwidth 1gbit
            burst 125000000b
        }
    }
    shaper 1G-out {
        bandwidth 20gbit
        class 100 {
            bandwidth 20gbit
            burst 3760k
            match INTERNAL {
                mark 100
            }
            queue-type fq-codel
        }
        default {
            bandwidth 1gbit
            burst 188k
            queue-type fq-codel
        }
    }
}
tc filter show dev br100 ingress
filter parent ffff: protocol all pref 255 basic chain 0 
filter parent ffff: protocol all pref 255 basic chain 0 handle 0x1 flowid ffff:65 
	action order 1:  police 0x1 rate 1Gbit burst 125000000b mtu 2Kb action drop overhead 0b 
	ref 1 bind 1 


tc qdisc show dev br100
qdisc htb 1: root refcnt 2 r2q 12500 default 0x65 direct_packets_stat 0 direct_qlen 1000
qdisc fq_codel 8003: parent 1:64 limit 10240p flows 1024 quantum 1514 target 4us interval 99us memory_limit 32Mb drop_batch 64 
qdisc sfq 8002: parent 1:65 limit 127p quantum 1514b depth 127 divisor 1024 
qdisc ingress ffff: parent ffff:fff1 ----------------

Default class works correctly, classes for marked traffic do not work.

Viacheslav added a comment.Aug 8 2023, 10:45 AM

PR https://github.com/vyos/vyos-1x/pull/2138

c-po closed this task as Resolved.Sep 30 2023, 6:36 AM