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

T6806: Rework QoS Policy for HFSC Shaper #4181

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Conversation

HollyGurza
Copy link
Contributor

  • Removed default m1 and m2 values from interface definitions
  • Adjusted filter priorities for shapers
  • Fixed SFQ qdisc and HFSC class creation to fully support m1, d, and m2 parameters
  • Added validation logic similar to VyOS 1.3 to improve error handling and user experience

Change Summary

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

Proposed changes

How to test

minimal config:

conf
set qos policy shaper-hfsc test default realtime m2 100kbit
commit

check tc:

vyos@vyos# tc -d qdisc show dev eth1
qdisc hfsc 1: root refcnt 2 default 2 
qdisc sfq 85f7: parent 1:2 limit 127p quantum 1514b depth 127 flows 128 divisor 1024 perturb 10sec 
[edit]
vyos@vyos# tc -d class show dev eth1
class hfsc 1: root 
class hfsc 1:1 parent 1: sc m1 0bit d 0us m2 10Gbit ul m1 0bit d 0us m2 10Gbit 
class hfsc 1:2 parent 1:1 leaf 85f7: rt m1 0bit d 0us m2 100Kbit 
[edit]
vyos@vyos# tc -d filter show dev eth1
[edit]

example from initial pr:

conf
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
vyos@vyos# tc -d qdisc show dev eth1
qdisc hfsc 1: root refcnt 2 default b 
qdisc sfq 85f9: parent 1:b limit 127p quantum 1514b depth 127 flows 128 divisor 1024 perturb 10sec 
qdisc sfq 85f8: parent 1:a limit 127p quantum 1514b depth 127 flows 128 divisor 1024 perturb 10sec 
[edit]
vyos@vyos# tc -d 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 85f8: ls m1 0bit d 0us m2 200Mbit 
class hfsc 1:b parent 1:1 leaf 85f9: ls m1 0bit d 0us m2 111Mbit 
[edit]
vyos@vyos# tc -d filter show dev eth1
filter parent 1: protocol all pref 1 u32 chain 0 
filter parent 1: protocol all pref 1 u32 chain 0 fh 800: ht divisor 1 
filter parent 1: protocol all pref 1 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]

Smoketest result

vyos@vyos:~$ python3 /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) ... ok
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
test_12_shaper_with_red_queue (__main__.TestQoS.test_12_shaper_with_red_queue) ... ok
test_13_shaper_delete_only_rule (__main__.TestQoS.test_13_shaper_delete_only_rule) ... ok
test_14_policy_limiter_marked_traffic (__main__.TestQoS.test_14_policy_limiter_marked_traffic) ... ok
test_15_traffic_match_group (__main__.TestQoS.test_15_traffic_match_group) ... ok
test_16_wrong_traffic_match_group (__main__.TestQoS.test_16_wrong_traffic_match_group) ... ok
test_21_shaper_hfsc (__main__.TestQoS.test_21_shaper_hfsc) ... ok

----------------------------------------------------------------------
Ran 17 tests in 74.584s

OK

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

Copy link

github-actions bot commented Oct 30, 2024

👍
No issues in PR Title / Commit Title

Copy link

✅ No issues found in unused-imports check.. Please refer the workflow run

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link

Conflicts have been resolved. A maintainer will review the pull request shortly.

smoketest/scripts/cli/test_qos.py Outdated Show resolved Hide resolved
interface-definitions/include/qos/hfsc-m1.xml.i Outdated Show resolved Hide resolved
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link

Conflicts have been resolved. A maintainer will review the pull request shortly.

- Removed default `m1` and `m2` values from interface definitions
- Adjusted filter priorities for shapers
- Fixed SFQ qdisc and HFSC class creation to fully support `m1`, `d`, and `m2` parameters
- Added validation logic similar to VyOS 1.3 to improve error handling and user experience
Copy link

CI integration ❌ failed!

Details

CI logs

  • CLI Smoketests (no interfaces) ❌ failed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

@sever-sever sever-sever merged commit df0ef8a into vyos:current Nov 21, 2024
14 of 16 checks passed
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.

3 participants