Skip to content

Commit

Permalink
T6790: QoS: Improve CAKE Policy
Browse files Browse the repository at this point in the history 
- Fixed handling of flow isolation parameters.
- Corrected support for `nat` and `nonat` in flow isolation.
- Extended RTT values to cover the full range supported by `tc`.
  • Loading branch information
@HollyGurza
HollyGurza committed Oct 24, 2024
1 parent ceb64f3 commit 36296d0
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 82 deletions.
111 changes: 50 additions & 61 deletions interface-definitions/qos.xml.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,78 +85,67 @@
<children>
#include <include/generic-description.xml.i>
#include <include/qos/bandwidth.xml.i>
<node name="flow-isolation">
<leafNode name="flow-isolation">
<properties>
<help>Flow isolation settings</help>
<completionHelp>
<list>blind src-host dst-host host flow dual-src-host dual-dst-host triple-isolate</list>
</completionHelp>
<valueHelp>
<format>blind</format>
<description>Disables flow isolation, all traffic passes through a single queue</description>
</valueHelp>
<valueHelp>
<format>src-host</format>
<description>Flows are defined only by source address</description>
</valueHelp>
<valueHelp>
<format>dst-host</format>
<description>Flows are defined only by destination address</description>
</valueHelp>
<valueHelp>
<format>host</format>
<description>Flows are defined by source-destination host pairs</description>
</valueHelp>
<valueHelp>
<format>flow</format>
<description>Flows are defined by the entire 5-tuple</description>
</valueHelp>
<valueHelp>
<format>dual-src-host</format>
<description>Flows are defined by the 5-tuple, fairness is applied first over source addresses, then over individual flows</description>
</valueHelp>
<valueHelp>
<format>dual-dst-host</format>
<description>Flows are defined by the 5-tuple, fairness is applied first over destination addresses, then over individual flows</description>
</valueHelp>
<valueHelp>
<format>triple-isolate</format>
<description>Flows are defined by the 5-tuple, fairness is applied over source and destination addresses and also over individual flows (default)</description>
</valueHelp>
<constraint>
<regex>(blind|src-host|dst-host|host|flow|dual-src-host|dual-dst-host|triple-isolate)</regex>
</constraint>
</properties>
<children>
<leafNode name="blind">
<properties>
<help>Disables flow isolation, all traffic passes through a single queue</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="src-host">
<properties>
<help>Flows are defined only by source address</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="dst-host">
<properties>
<help>Flows are defined only by destination address</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="host">
<properties>
<help>Flows are defined by source-destination host pairs</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="flow">
<properties>
<help>Flows are defined by the entire 5-tuple</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="dual-src-host">
<properties>
<help>Flows are defined by the 5-tuple, fairness is applied first over source addresses, then over individual flows</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="dual-dst-host">
<properties>
<help>Flows are defined by the 5-tuple, fairness is applied first over destination addresses, then over individual flows</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="triple-isolate">
<properties>
<help>Flows are defined by the 5-tuple, fairness is applied over source and destination addresses and also over individual flows (default)</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="nat">
<properties>
<help>Perform NAT lookup before applying flow-isolation rules</help>
<valueless/>
</properties>
</leafNode>
</children>
</node>
<defaultValue>triple-isolate</defaultValue>
</leafNode>
<leafNode name="flow-isolation-nat">
<properties>
<help>Perform NAT lookup before applying flow-isolation rules</help>
<valueless/>
</properties>
</leafNode>
<leafNode name="rtt">
<properties>
<help>Round-Trip-Time for Active Queue Management (AQM)</help>
<valueHelp>
<format>u32:1-3600000</format>
<format>u32:1-1000000000</format>
<description>RTT in ms</description>
</valueHelp>
<constraint>
<validator name="numeric" argument="--range 1-3600000"/>
<validator name="numeric" argument="--range 1-1000000000"/>
</constraint>
<constraintErrorMessage>RTT must be in range 1 to 3600000 milli-seconds</constraintErrorMessage>
<constraintErrorMessage>RTT must be in range 1 to 1000000000 milli-seconds</constraintErrorMessage>
</properties>
<defaultValue>100</defaultValue>
</leafNode>
Expand Down
47 changes: 26 additions & 21 deletions python/vyos/qos/cake.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,25 @@

from vyos.qos.base import QoSBase


class CAKE(QoSBase):
"""
https://man7.org/linux/man-pages/man8/tc-cake.8.html
"""

_direction = ['egress']

# https://man7.org/linux/man-pages/man8/tc-cake.8.html
flow_isolation_map = {
'blind': 'flowblind',
'src-host': 'srchost',
'dst-host': 'dsthost',
'dual-dst-host': 'dual-dsthost',
'dual-src-host': 'dual-srchost',
'triple-isolate': 'triple-isolate',
'flow': 'flows',
'host': 'hosts',
}

def update(self, config, direction):
tmp = f'tc qdisc add dev {self._interface} root handle 1: cake {direction}'
if 'bandwidth' in config:
Expand All @@ -30,26 +45,16 @@ def update(self, config, direction):
tmp += f' rtt {rtt}ms'

if 'flow_isolation' in config:
if 'blind' in config['flow_isolation']:
tmp += f' flowblind'
if 'dst_host' in config['flow_isolation']:
tmp += f' dsthost'
if 'dual_dst_host' in config['flow_isolation']:
tmp += f' dual-dsthost'
if 'dual_src_host' in config['flow_isolation']:
tmp += f' dual-srchost'
if 'triple_isolate' in config['flow_isolation']:
tmp += f' triple-isolate'
if 'flow' in config['flow_isolation']:
tmp += f' flows'
if 'host' in config['flow_isolation']:
tmp += f' hosts'
if 'nat' in config['flow_isolation']:
tmp += f' nat'
if 'src_host' in config['flow_isolation']:
tmp += f' srchost '
else:
tmp += f' nonat'
isolation_value = self.flow_isolation_map.get(config['flow_isolation'])

if isolation_value is not None:
tmp += f' {isolation_value}'
else:
raise ValueError(
f'Invalid flow isolation parameter: {config["flow_isolation"]}'
)

tmp += ' nat' if 'flow_isolation_nat' in config else ' nonat'

self._cmd(tmp)

Expand Down
63 changes: 63 additions & 0 deletions smoketest/scripts/cli/test_qos.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

from vyos.configsession import ConfigSessionError
from vyos.ifconfig import Section
from vyos.qos import CAKE
from vyos.utils.process import cmd

base_path = ['qos']
Expand Down Expand Up @@ -854,6 +855,68 @@ def test_16_wrong_traffic_match_group(self):
self.cli_set(['qos', 'traffic-match-group', '3', 'match-group', 'unexpected'])
self.cli_commit()

def test_17_cake_updates(self):
bandwidth = 1000000
rtt = 200
interface = self._interfaces[0]
policy_name = f'qos-policy-{interface}'

self.cli_set(base_path + ['interface', interface, 'egress', policy_name])
self.cli_set(
base_path + ['policy', 'cake', policy_name, 'bandwidth', str(bandwidth)]
)
self.cli_set(base_path + ['policy', 'cake', policy_name, 'rtt', str(rtt)])

# commit changes
self.cli_commit()

tmp = get_tc_qdisc_json(interface)

self.assertEqual('cake', tmp['kind'])
# TC store rates as a 32-bit unsigned integer in bps (Bytes per second)
self.assertEqual(int(bandwidth * 125), tmp['options']['bandwidth'])
# RTT internally is in us
self.assertEqual(int(rtt * 1000), tmp['options']['rtt'])
self.assertEqual('triple-isolate', tmp['options']['flowmode'])
self.assertFalse(tmp['options']['ingress'])
self.assertFalse(tmp['options']['nat'])
self.assertTrue(tmp['options']['raw'])

nat = True
for flow_isolation in [
'blind',
'src-host',
'dst-host',
'dual-dst-host',
'dual-src-host',
'triple-isolate',
'flow',
'host',
]:
self.cli_set(
base_path
+ ['policy', 'cake', policy_name, 'flow-isolation', flow_isolation]
)

if nat:
self.cli_set(
base_path + ['policy', 'cake', policy_name, 'flow-isolation-nat']
)
else:
self.cli_delete(
base_path + ['policy', 'cake', policy_name, 'flow-isolation-nat']
)

self.cli_commit()

tmp = get_tc_qdisc_json(interface)
self.assertEqual(
CAKE.flow_isolation_map.get(flow_isolation), tmp['options']['flowmode']
)

self.assertEqual(nat, tmp['options']['nat'])
nat = not nat


if __name__ == '__main__':
unittest.main(verbosity=2)

0 comments on commit 36296d0

Please sign in to comment.