diff options
| -rw-r--r-- | python/vyos/qos/base.py | 5 | ||||
| -rwxr-xr-x | smoketest/scripts/cli/test_qos.py | 39 | ||||
| -rwxr-xr-x | src/conf_mode/qos.py | 23 |
3 files changed, 66 insertions, 1 deletions
diff --git a/python/vyos/qos/base.py b/python/vyos/qos/base.py index f9366c6b1..4173a1a43 100644 --- a/python/vyos/qos/base.py +++ b/python/vyos/qos/base.py @@ -246,6 +246,7 @@ class QoSBase: filter_cmd_base += ' protocol all' if 'match' in cls_config: + is_filtered = False for index, (match, match_config) in enumerate(cls_config['match'].items(), start=1): filter_cmd = filter_cmd_base if self.qostype == 'shaper' and 'prio ' not in filter_cmd: @@ -330,11 +331,13 @@ class QoSBase: cls = int(cls) filter_cmd += f' flowid {self._parent:x}:{cls:x}' self._cmd(filter_cmd) + is_filtered = True vlan_expression = "match.*.vif" match_vlan = jmespath.search(vlan_expression, cls_config) - if any(tmp in ['exceed', 'bandwidth', 'burst'] for tmp in cls_config): + if any(tmp in ['exceed', 'bandwidth', 'burst'] for tmp in cls_config) \ + and is_filtered: # For "vif" "basic match" is used instead of "action police" T5961 if not match_vlan: filter_cmd += f' action police' diff --git a/smoketest/scripts/cli/test_qos.py b/smoketest/scripts/cli/test_qos.py index 4f41e36cd..fef1ff23a 100755 --- a/smoketest/scripts/cli/test_qos.py +++ b/smoketest/scripts/cli/test_qos.py @@ -697,6 +697,45 @@ class TestQoS(VyOSUnitTestSHIM.TestCase): for config_entry in config_entries: self.assertIn(config_entry, output) + def test_13_shaper_delete_only_rule(self): + default_bandwidth = 100 + default_burst = 100 + interface = self._interfaces[0] + class_bandwidth = 50 + class_ceiling = 5 + src_address = '10.1.1.0/24' + + shaper_name = f'qos-shaper-{interface}' + self.cli_set(base_path + ['interface', interface, 'egress', shaper_name]) + self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'bandwidth', f'10mbit']) + self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'default', 'bandwidth', f'{default_bandwidth}mbit']) + self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'default', 'burst', f'{default_burst}']) + + self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'class', '30', 'bandwidth', f'{class_bandwidth}mbit']) + self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'class', '30', 'ceiling', f'{class_ceiling}mbit']) + self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'class', '30', 'match', 'ADDRESS30', 'ip', 'source', 'address', src_address]) + self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'class', '30', 'match', 'ADDRESS30', 'description', 'smoketest']) + self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'class', '30', 'priority', '5']) + self.cli_set(base_path + ['policy', 'shaper', shaper_name, 'class', '30', 'queue-type', 'fair-queue']) + + # commit changes + self.cli_commit() + # check root htb config + output = cmd(f'tc class show dev {interface}') + + config_entries = ( + f'prio 5 rate {class_bandwidth}Mbit ceil {class_ceiling}Mbit burst 15Kb', # specified class + f'prio 7 rate {default_bandwidth}Mbit ceil 100Mbit burst {default_burst}b', # default class + ) + for config_entry in config_entries: + self.assertIn(config_entry, output) + + self.assertTrue('' != cmd(f'tc filter show dev {interface}')) + # self.cli_delete(base_path + ['policy', 'shaper', shaper_name, 'class', '30', 'match', 'ADDRESS30']) + self.cli_delete(base_path + ['policy', 'shaper', shaper_name, 'class', '30', 'match', 'ADDRESS30', 'ip', 'source', 'address', src_address]) + self.cli_commit() + self.assertEqual('', cmd(f'tc filter show dev {interface}')) + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/src/conf_mode/qos.py b/src/conf_mode/qos.py index 3dfb4bab8..ccfc8f6b8 100755 --- a/src/conf_mode/qos.py +++ b/src/conf_mode/qos.py @@ -70,6 +70,22 @@ def get_shaper(qos, interface_config, direction): return (map_vyops_tc[shaper_type], shaper_config) + +def _clean_conf_dict(conf): + """ + Delete empty nodes from config e.g. + match ADDRESS30 { + ip { + source {} + } + } + """ + if isinstance(conf, dict): + return {node: _clean_conf_dict(val) for node, val in conf.items() if val != {} and _clean_conf_dict(val) != {}} + else: + return conf + + def get_config(config=None): if config: conf = config @@ -120,6 +136,13 @@ def get_config(config=None): if 'queue_limit' not in qos['policy'][policy][p_name]['precedence'][precedence]: qos['policy'][policy][p_name]['precedence'][precedence]['queue_limit'] = \ str(int(4 * max_thr)) + # cleanup empty match config + if 'class' in p_config: + for cls, cls_config in p_config['class'].items(): + if 'match' in cls_config: + cls_config['match'] = _clean_conf_dict(cls_config['match']) + if cls_config['match'] == {}: + del cls_config['match'] return qos |