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

T5958: QoS add basic implementation of policy shaper-hfsc #2852

Merged
merged 1 commit into from
Jan 21, 2024

Conversation

sever-sever
Copy link
Member

Change Summary

QoS policy shaper-hfsc was not implemented after rewriting the traffic-policy to qos policy. We had CLI, but it does not use the correct class. Add a basic implementation of policy shaper-hfsc. Write the class TrafficShaperHFS

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

Proposed changes

How to test

set qos policy shaper-hfsc SHAPE bandwidth '400mbit'
set qos policy shaper-hfsc SHAPE class 10 linkshare m2 '200mbit'
set qos policy shaper-hfsc SHAPE class 10 match DST ip destination address '192.0.2.1/32'
set qos policy shaper-hfsc SHAPE default linkshare m2 '111mbit'

set qos interface eth1 egress SHAPE

commit:

DEBUG/QoS: tc qdisc replace dev eth1 root handle 1: hfsc default b
DEBUG/QoS: tc class replace dev eth1 parent 1: classid 1:1 hfsc sc rate 400000000 ul rate 400000000
DEBUG/QoS: tc class replace dev eth1 parent 1:1 classid 1:a hfsc ls m1 0bit m2 200000000 
DEBUG/QoS: tc qdisc replace dev eth1 parent 1:a sfq perturb 10
DEBUG/QoS: tc class replace dev eth1 parent 1:1 classid 1:b hfsc ls m1 0bit m2 111000000 
DEBUG/QoS: tc qdisc replace dev eth1 parent 1:b sfq perturb 10
{'bandwidth': '400mbit',
 'class': {'10': {'linkshare': {'m1': '0bit', 'm2': '200mbit'},
                  'match': {'DST': {'ip': {'destination': {'address': '192.0.2.1/32'}}}},
                  'realtime': {'m1': '0bit', 'm2': '100%'},
                  'upperlimit': {'m1': '0bit', 'm2': '100%'}}},
 'default': {'linkshare': {'m1': '0bit', 'm2': '111mbit'},
             'realtime': {'m1': '0bit', 'm2': '100%'},
             'upperlimit': {'m1': '0bit', 'm2': '100%'}}}
DEBUG/QoS: tc filter add dev eth1 parent 1: protocol all u32 match ip dst 192.0.2.1/32 flowid 1:a

[edit]
vyos@r4# 

Check TC:

vyos@r4# tc qdisc show dev eth1
qdisc hfsc 1: root refcnt 2 default b 
qdisc sfq 807b: parent 1:b limit 127p quantum 1514b depth 127 divisor 1024 perturb 10sec 
qdisc sfq 807a: parent 1:a limit 127p quantum 1514b depth 127 divisor 1024 perturb 10sec 
[edit]
vyos@r4# 
[edit]
vyos@r4# tc class show dev eth1
class hfsc 1: root 
class hfsc 1:1 parent 1: sc m1 0bit d 0us m2 400Mbit ul m1 0bit d 0us m2 400Mbit 
class hfsc 1:a parent 1:1 leaf 807a: ls m1 0bit d 0us m2 200Mbit 
class hfsc 1:b parent 1:1 leaf 807b: ls m1 0bit d 0us m2 111Mbit 
[edit]
vyos@r4# 
[edit]
vyos@r4# tc filter show dev eth1
filter parent 1: protocol all pref 49152 u32 chain 0 
filter parent 1: protocol all pref 49152 u32 chain 0 fh 800: ht divisor 1 
filter parent 1: protocol all pref 49152 u32 chain 0 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:a not_in_hw 
  match c0000201/ffffffff at 16
[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 71.759s

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

QoS policy shaper-hfsc was not implemented after rewriting the
traffic-policy to qos policy. We had CLI but it  does not use the
correct class. Add a basic implementation of policy shaper-hfsc.
Write the class `TrafficShaperHFS`
@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro and c-po and removed request for a team January 18, 2024 19:38
@sarthurdev
Copy link
Member

Only issue is that it causes a fair amount of duplicate code.

Should the base TrafficShaper class perhaps instead have an attribute set to htb which is used in the tc command strings. The HFSC child class could override this attribute with value hfsc and simply call the super().update(..) as before?

@sever-sever
Copy link
Member Author

sever-sever commented Jan 18, 2024

Only issue is that it causes a fair amount of duplicate code.

Should the base TrafficShaper class perhaps instead have an attribute set to htb which is used in the tc command strings. The HFSC child class could override this attribute with value hfsc and simply call the super().update(..) as before?

Yes, there is such a thing, it was safer and faster to create a new class with a copy the part of logic.
There are more nuances of config.
HFSC does not have options:

  • class bandwidth
  • r2q
  • default
  • burst
  • codel_quantum
  • priority
  • ceiling

and so on. I'd separate them it will be easier to fix/extend something only for shaper or only for shaper-hfsc and don't break another feature.

@sever-sever sever-sever merged commit 7c43d6c into vyos:current Jan 21, 2024
8 checks passed
@sever-sever
Copy link
Member Author

@Mergifyio backport sagitta

Copy link
Contributor

mergify bot commented Jan 22, 2024

backport sagitta

✅ Backports have been created

c-po added a commit that referenced this pull request Jan 22, 2024
T5958: QoS add basic implementation of policy shaper-hfsc (backport #2852)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants