Skip to content

Commit

Permalink
T6790: QoS: Improve CAKE Policy (#4173)
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`.
- Make migration script 2-to-3 qos
  • Loading branch information
@HollyGurza
HollyGurza authored Nov 20, 2024
1 parent 2419c2f commit 1c8321a
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 83 deletions.
2 changes: 1 addition & 1 deletion interface-definitions/include/version/qos-version.xml.i
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- include start from include/version/qos-version.xml.i -->
<syntaxVersion component='qos' version='2'></syntaxVersion>
<syntaxVersion component='qos' version='3'></syntaxVersion>
<!-- include end -->
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 @@ -871,6 +872,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

def test_20_round_robin_policy_default(self):
interface = self._interfaces[0]
policy_name = f'qos-policy-{interface}'
Expand Down
34 changes: 34 additions & 0 deletions src/migration-scripts/qos/2-to-3
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Copyright 2024 VyOS maintainers and contributors <[email protected]>
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
# License as published by the Free Software Foundation; either
# version 2.1 of the License, or (at your option) any later version.
#
# This library is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
# Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public License
# along with this library. If not, see <http://www.gnu.org/licenses/>.

from vyos.configtree import ConfigTree


def migrate(config: ConfigTree) -> None:
base = ['qos', 'policy', 'cake']
if config.exists(base):
for policy in config.list_nodes(base):
if config.exists(base + [policy, 'flow-isolation']):
isolation = None
for isol in config.list_nodes(base + [policy, 'flow-isolation']):
if isol == 'nat':
config.set(base + [policy, 'flow-isolation-nat'])
else:
isolation = isol

config.delete(base + [policy, 'flow-isolation'])

if isolation:
config.set(base + [policy, 'flow-isolation'], value=isolation)

0 comments on commit 1c8321a

Please sign in to comment.