Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T5963: Fix QoS shaper rate calculations and set default 1Gbit #2855

Merged
merged 1 commit into from Jan 19, 2024

Conversation

sever-sever
Copy link
Member

@sever-sever sever-sever commented Jan 19, 2024

Change Summary

It is impossible to detect interface speed for some devices for exmaple virtio interfaces:

vyos@r4:~$ cat /sys/class/net/eth1/speed
-1

It causes wrong negative calcultaions like:

  • bandwidth: -1000000
  • 4% of bandwidth: -40000

tc class replace dev eth1 parent 1: classid 1:1 htb rate -1000000
tc class replace dev eth1 parent 1:1 classid 1:a htb rate -40000

Fix this by checking negative value.
Add default interface speed to 1000 Mbit if we cannot detect the interface speed, the current default value 10 Mbit is too low for nowadays.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

Component(s) name

qos, shaper

Proposed changes

How to test

VyOS configuration:

touch /tmp/vyos.qos.debug

set qos policy shaper SHAPER class 10 bandwidth '4%'
set qos policy shaper SHAPER class 10 match lowdelay ip dscp 'lowdelay'
set qos policy shaper SHAPER default bandwidth '2mbit'

set qos interface eth1 egress SHAPER
commit

Before the fix we have negative values/rates for bandwidth

WARNING: Interface speed cannot be determined (assuming 10 Mbit/s)

DEBUG/QoS: tc qdisc replace dev eth1 root handle 1: htb r2q 1 default b
DEBUG/QoS: tc class replace dev eth1 parent 1: classid 1:1 htb rate -1000000
DEBUG/QoS: tc class replace dev eth1 parent 1:1 classid 1:a htb rate -40000 burst 15k quantum 1514
DEBUG/QoS: tc class replace dev eth1 parent 1:1 classid 1:b htb rate 2000000 burst 15k quantum 1514 prio 20

DEBUG/QoS: tc qdisc replace dev eth1 parent 1:a sfq
DEBUG/QoS: tc qdisc replace dev eth1 parent 1:b sfq
DEBUG/QoS: tc qdisc replace dev eth1 parent 1:a fq_codel quantum 1514 flows 1024 interval 100 interval 100 target 5 noecn
DEBUG/QoS: tc qdisc replace dev eth1 parent 1:b fq_codel quantum 1514 flows 1024 interval 100 interval 100 target 5 noecn

DEBUG/QoS: tc filter add dev eth1 parent 1: protocol all prio 1 u32 match ip dsfield 16 0xff flowid 1:a
DEBUG/QoS: tc filter add dev eth1 parent 1: protocol all prio 1 u32 match ip dsfield 16 0xff flowid 1:a action police rate -1000000 burst 15k flowid 1:a

And incorrect values from TC (before the fix) 18446744Tbit:

vyos@r4# sudo tc class show dev eth1
class htb 1:1 root rate 18446744Tbit ceil 18446744Tbit burst 0b cburst 0b
class htb 1:a parent 1:1 leaf 8003: prio 0 rate 18446744Tbit ceil 18446744Tbit burst 0b cburst 0b
class htb 1:b parent 1:1 leaf 8004: prio 7 rate 2Mbit ceil 2Mbit burst 15Kb cburst 1600b
[edit]
vyos@r4#

After the fix, all looks good:

DEBUG/QoS: tc class replace dev eth1 parent 1:1 classid 1:a htb rate 40000000 burst 15k quantum 1514
DEBUG/QoS: tc qdisc replace dev eth1 parent 1:a sfq
DEBUG/QoS: tc class replace dev eth1 parent 1:1 classid 1:b htb rate 2000000 burst 15k quantum 1514 prio 20
DEBUG/QoS: tc qdisc replace dev eth1 parent 1:b sfq
DEBUG/QoS: tc qdisc replace dev eth1 parent 1:a fq_codel quantum 1514 flows 1024 interval 100 interval 100 target 5 noecn
DEBUG/QoS: tc filter add dev eth1 parent 1: protocol all prio 1 u32 match ip dsfield 16 0xff flowid 1:a
DEBUG/QoS: tc filter add dev eth1 parent 1: protocol all prio 1 u32 match ip dsfield 16 0xff flowid 1:a action police rate 40000000 burst 15k flowid 1:a
DEBUG/QoS: tc qdisc replace dev eth1 parent 1:b fq_codel quantum 1514 flows 1024 interval 100 interval 100 target 5 noecn

TC after the fix

vyos@r4# sudo tc class show dev eth1
class htb 1:1 root rate 1Gbit ceil 1Gbit burst 1375b cburst 1375b
class htb 1:a parent 1:1 leaf 8159: prio 0 rate 40Mbit ceil 40Mbit burst 15Kb cburst 1600b
class htb 1:b parent 1:1 leaf 815a: prio 7 rate 2Mbit ceil 2Mbit burst 15Kb cburst 1600b
[edit]
vyos@r4# 

Smoketest result

vyos@r4:~$ /usr/libexec/vyos/tests/smoke/cli/test_qos.py
test_01_cake (__main__.TestQoS.test_01_cake) ... ok
test_02_drop_tail (__main__.TestQoS.test_02_drop_tail) ... ok
test_03_fair_queue (__main__.TestQoS.test_03_fair_queue) ... ok
test_04_fq_codel (__main__.TestQoS.test_04_fq_codel) ... ok
test_05_limiter (__main__.TestQoS.test_05_limiter) ... ok
test_06_network_emulator (__main__.TestQoS.test_06_network_emulator) ... ok
test_07_priority_queue (__main__.TestQoS.test_07_priority_queue) ... ok
test_08_random_detect (__main__.TestQoS.test_08_random_detect) ... skipped 'tc returns invalid JSON here - needs iproute2 fix'
test_09_rate_control (__main__.TestQoS.test_09_rate_control) ... ok
test_10_round_robin (__main__.TestQoS.test_10_round_robin) ... ok
test_11_shaper (__main__.TestQoS.test_11_shaper) ... ok

----------------------------------------------------------------------
Ran 11 tests in 75.044s

OK (skipped=1)
vyos@r4:~$ 

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

It is impossible to detect interface speed for some devices
for exmaple virtio interfaces:
```
vyos@r4:~$ cat /sys/class/net/eth1/speed
-1
```

It causes wrong negative calcultaions like:
 - bandwidth: -1000000
 - 4% of bandwidth: -40000

tc class replace dev eth1 parent 1: classid 1:1 htb rate -1000000
tc class replace dev eth1 parent 1:1 classid 1:a htb rate -40000

Fix this with checking negative value.
Add default interface speed to 1000 Mbit if we cannot detect the
interface speed, the current default value 10 Mbit is too low
for nowadays
@vyosbot vyosbot requested a review from a team January 19, 2024 13:36
@vyosbot vyosbot requested review from dmbaturin, sarthurdev, zdc, jestabro and c-po and removed request for a team January 19, 2024 13:36
@sever-sever sever-sever changed the title T5963: Fix QoS shaper rate calculations and set defaul 1Gbit T5963: Fix QoS shaper rate calculations and set default 1Gbit Jan 19, 2024
Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, and indeed I haven't seen a 10mbit NIC in quite a while. ;)

@dmbaturin dmbaturin merged commit d52f804 into vyos:current Jan 19, 2024
8 checks passed
@sever-sever
Copy link
Member Author

@Mergifyio backport sagitta

Copy link

mergify bot commented Jan 20, 2024

backport sagitta

✅ Backports have been created

c-po added a commit that referenced this pull request Jan 20, 2024
T5963: Fix QoS shaper rate calculations and set default 1Gbit (backport #2855)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants