From b832ab05d16876f66bc6bf4837ae87d941455171 Mon Sep 17 00:00:00 2001 From: Henning Surmeier Date: Thu, 31 Mar 2022 14:12:02 +0200 Subject: Revert "Revert "backport: T4515: T4219: policy local-route6 and inbound-interface support"" This reverts commit 45a2a7d0adc7e9d27d6c7aee1ccbd9b64a1437ad. --- src/conf_mode/policy-local-route.py | 183 +++++++++++++++++++++++++++++------- 1 file changed, 149 insertions(+), 34 deletions(-) (limited to 'src/conf_mode/policy-local-route.py') diff --git a/src/conf_mode/policy-local-route.py b/src/conf_mode/policy-local-route.py index 013f22665..0a4597869 100755 --- a/src/conf_mode/policy-local-route.py +++ b/src/conf_mode/policy-local-route.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# Copyright (C) 2020 VyOS maintainers and contributors +# Copyright (C) 2020-2021 VyOS maintainers and contributors # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License version 2 or later as @@ -18,6 +18,7 @@ import os from sys import exit +from netifaces import interfaces from vyos.config import Config from vyos.configdict import dict_merge from vyos.configdict import node_changed @@ -35,27 +36,96 @@ def get_config(config=None): conf = config else: conf = Config() - base = ['policy', 'local-route'] + base = ['policy'] + pbr = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True) - # delete policy local-route - dict = {} - tmp = node_changed(conf, ['policy', 'local-route', 'rule'], key_mangling=('-', '_')) - if tmp: - for rule in (tmp or []): - src = leaf_node_changed(conf, ['policy', 'local-route', 'rule', rule, 'source']) - if src: - dict = dict_merge({'rule_remove' : {rule : {'source' : src}}}, dict) + for route in ['local_route', 'local_route6']: + dict_id = 'rule_remove' if route == 'local_route' else 'rule6_remove' + route_key = 'local-route' if route == 'local_route' else 'local-route6' + base_rule = base + [route_key, 'rule'] + + # delete policy local-route + dict = {} + tmp = node_changed(conf, base_rule, key_mangling=('-', '_')) + if tmp: + for rule in (tmp or []): + src = leaf_node_changed(conf, base_rule + [rule, 'source']) + fwmk = leaf_node_changed(conf, base_rule + [rule, 'fwmark']) + iif = leaf_node_changed(conf, base_rule + [rule, 'inbound-interface']) + dst = leaf_node_changed(conf, base_rule + [rule, 'destination']) + rule_def = {} + if src: + rule_def = dict_merge({'source' : src}, rule_def) + if fwmk: + rule_def = dict_merge({'fwmark' : fwmk}, rule_def) + if iif: + rule_def = dict_merge({'inbound_interface' : iif}, rule_def) + if dst: + rule_def = dict_merge({'destination' : dst}, rule_def) + dict = dict_merge({dict_id : {rule : rule_def}}, dict) pbr.update(dict) - - # delete policy local-route rule x source x.x.x.x - if 'rule' in pbr: - for rule in pbr['rule']: - src = leaf_node_changed(conf, ['policy', 'local-route', 'rule', rule, 'source']) - if src: - dict = dict_merge({'rule_remove' : {rule : {'source' : src}}}, dict) + if fwmk: + dict = dict_merge({'rule_remove' : {rule : {'fwmark' : fwmk}}}, dict) pbr.update(dict) + if not route in pbr: + continue + + # delete policy local-route rule x source x.x.x.x + # delete policy local-route rule x fwmark x + # delete policy local-route rule x destination x.x.x.x + if 'rule' in pbr[route]: + for rule, rule_config in pbr[route]['rule'].items(): + src = leaf_node_changed(conf, base_rule + [rule, 'source']) + fwmk = leaf_node_changed(conf, base_rule + [rule, 'fwmark']) + iif = leaf_node_changed(conf, base_rule + [rule, 'inbound-interface']) + dst = leaf_node_changed(conf, base_rule + [rule, 'destination']) + # keep track of changes in configuration + # otherwise we might remove an existing node although nothing else has changed + changed = False + + rule_def = {} + # src is None if there are no changes to src + if src is None: + # if src hasn't changed, include it in the removal selector + # if a new selector is added, we have to remove all previous rules without this selector + # to make sure we remove all previous rules with this source(s), it will be included + if 'source' in rule_config: + rule_def = dict_merge({'source': rule_config['source']}, rule_def) + else: + # if src is not None, it's previous content will be returned + # this can be an empty array if it's just being set, or the previous value + # either way, something has to be changed and we only want to remove previous values + changed = True + # set the old value for removal if it's not empty + if len(src) > 0: + rule_def = dict_merge({'source' : src}, rule_def) + if fwmk is None: + if 'fwmark' in rule_config: + rule_def = dict_merge({'fwmark': rule_config['fwmark']}, rule_def) + else: + changed = True + if len(fwmk) > 0: + rule_def = dict_merge({'fwmark' : fwmk}, rule_def) + if iif is None: + if 'inbound_interface' in rule_config: + rule_def = dict_merge({'inbound_interface': rule_config['inbound_interface']}, rule_def) + else: + changed = True + if len(iif) > 0: + rule_def = dict_merge({'inbound_interface' : iif}, rule_def) + if dst is None: + if 'destination' in rule_config: + rule_def = dict_merge({'destination': rule_config['destination']}, rule_def) + else: + changed = True + if len(dst) > 0: + rule_def = dict_merge({'destination' : dst}, rule_def) + if changed: + dict = dict_merge({dict_id : {rule : rule_def}}, dict) + pbr.update(dict) + return pbr def verify(pbr): @@ -63,13 +133,25 @@ def verify(pbr): if not pbr: return None - if 'rule' in pbr: - for rule in pbr['rule']: - if 'source' not in pbr['rule'][rule]: - raise ConfigError('Source address required!') - else: - if 'set' not in pbr['rule'][rule] or 'table' not in pbr['rule'][rule]['set']: - raise ConfigError('Table set is required!') + for route in ['local_route', 'local_route6']: + if not route in pbr: + continue + + pbr_route = pbr[route] + if 'rule' in pbr_route: + for rule in pbr_route['rule']: + if 'source' not in pbr_route['rule'][rule] \ + and 'destination' not in pbr_route['rule'][rule] \ + and 'fwmark' not in pbr_route['rule'][rule] \ + and 'inbound_interface' not in pbr_route['rule'][rule]: + raise ConfigError('Source or destination address or fwmark or inbound-interface is required!') + else: + if 'set' not in pbr_route['rule'][rule] or 'table' not in pbr_route['rule'][rule]['set']: + raise ConfigError('Table set is required!') + if 'inbound_interface' in pbr_route['rule'][rule]: + interface = pbr_route['rule'][rule]['inbound_interface'] + if interface not in interfaces(): + raise ConfigError(f'Interface "{interface}" does not exist') return None @@ -84,18 +166,51 @@ def apply(pbr): return None # Delete old rule if needed - if 'rule_remove' in pbr: - for rule in pbr['rule_remove']: - for src in pbr['rule_remove'][rule]['source']: - call(f'ip rule del prio {rule} from {src}') + for rule_rm in ['rule_remove', 'rule6_remove']: + if rule_rm in pbr: + v6 = " -6" if rule_rm == 'rule6_remove' else "" + for rule, rule_config in pbr[rule_rm].items(): + rule_config['source'] = rule_config['source'] if 'source' in rule_config else [''] + for src in rule_config['source']: + f_src = '' if src == '' else f' from {src} ' + rule_config['destination'] = rule_config['destination'] if 'destination' in rule_config else [''] + for dst in rule_config['destination']: + f_dst = '' if dst == '' else f' to {dst} ' + rule_config['fwmark'] = rule_config['fwmark'] if 'fwmark' in rule_config else [''] + for fwmk in rule_config['fwmark']: + f_fwmk = '' if fwmk == '' else f' fwmark {fwmk} ' + rule_config['inbound_interface'] = rule_config['inbound_interface'] if 'inbound_interface' in rule_config else [''] + for iif in rule_config['inbound_interface']: + f_iif = '' if iif == '' else f' iif {iif} ' + call(f'ip{v6} rule del prio {rule} {f_src}{f_dst}{f_fwmk}{f_iif}') # Generate new config - if 'rule' in pbr: - for rule in pbr['rule']: - table = pbr['rule'][rule]['set']['table'] - if pbr['rule'][rule]['source']: - for src in pbr['rule'][rule]['source']: - call(f'ip rule add prio {rule} from {src} lookup {table}') + for route in ['local_route', 'local_route6']: + if not route in pbr: + continue + + v6 = " -6" if route == 'local_route6' else "" + + pbr_route = pbr[route] + if 'rule' in pbr_route: + for rule, rule_config in pbr_route['rule'].items(): + table = rule_config['set']['table'] + + rule_config['source'] = rule_config['source'] if 'source' in rule_config else ['all'] + for src in rule_config['source'] or ['all']: + f_src = '' if src == '' else f' from {src} ' + rule_config['destination'] = rule_config['destination'] if 'destination' in rule_config else ['all'] + for dst in rule_config['destination']: + f_dst = '' if dst == '' else f' to {dst} ' + f_fwmk = '' + if 'fwmark' in rule_config: + fwmk = rule_config['fwmark'] + f_fwmk = f' fwmark {fwmk} ' + f_iif = '' + if 'inbound_interface' in rule_config: + iif = rule_config['inbound_interface'] + f_iif = f' iif {iif} ' + call(f'ip{v6} rule add prio {rule} {f_src}{f_dst}{f_fwmk}{f_iif} lookup {table}') return None -- cgit v1.2.3 From 9fc6dba84177702e24fc6b8e6668f638ef156995 Mon Sep 17 00:00:00 2001 From: Henning Surmeier Date: Thu, 31 Mar 2022 13:43:50 +0200 Subject: policy: T4151: remove left over fwmk check --- src/conf_mode/policy-local-route.py | 3 --- 1 file changed, 3 deletions(-) (limited to 'src/conf_mode/policy-local-route.py') diff --git a/src/conf_mode/policy-local-route.py b/src/conf_mode/policy-local-route.py index 0a4597869..3f834f55c 100755 --- a/src/conf_mode/policy-local-route.py +++ b/src/conf_mode/policy-local-route.py @@ -65,9 +65,6 @@ def get_config(config=None): rule_def = dict_merge({'destination' : dst}, rule_def) dict = dict_merge({dict_id : {rule : rule_def}}, dict) pbr.update(dict) - if fwmk: - dict = dict_merge({'rule_remove' : {rule : {'fwmark' : fwmk}}}, dict) - pbr.update(dict) if not route in pbr: continue -- cgit v1.2.3 From 45734d25f6b4f930fbc572be7ab247a377e179bf Mon Sep 17 00:00:00 2001 From: Henning Surmeier Date: Wed, 6 Apr 2022 13:25:35 +0200 Subject: correctly set fwmk from current config on removals, fix smoketest --- smoketest/scripts/cli/test_policy.py | 55 +++++++++++++++++++++++++++--------- src/conf_mode/policy-local-route.py | 4 +-- 2 files changed, 44 insertions(+), 15 deletions(-) (limited to 'src/conf_mode/policy-local-route.py') diff --git a/smoketest/scripts/cli/test_policy.py b/smoketest/scripts/cli/test_policy.py index 50f2d7b43..a636a8097 100755 --- a/smoketest/scripts/cli/test_policy.py +++ b/smoketest/scripts/cli/test_policy.py @@ -1030,13 +1030,6 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase): self.assertEqual(sort_ip(tmp), sort_ip(original_second)) - -def sort_ip(output): - o = '\n'.join([' '.join(line.strip().split()) for line in output.strip().splitlines()]) - o = o.splitlines() - o.sort() - return o - # Test set table for fwmark def test_fwmark_table_id(self): path = base_path + ['local-route'] @@ -1050,9 +1043,6 @@ def sort_ip(output): self.cli_commit() - # Check generated configuration - - # Expected values original = """ 101: from all fwmark 0x18 lookup 154 """ @@ -1077,9 +1067,6 @@ def sort_ip(output): self.cli_commit() - # Check generated configuration - - # Expected values original = """ 100: from 203.0.113.11 fwmark 0x17 lookup 150 100: from 203.0.113.12 fwmark 0x17 lookup 150 @@ -1090,5 +1077,47 @@ def sort_ip(output): self.assertEqual(tmp, original) + # Test remove fwmark for sources with fwmark + def test_source_fwmk_remove(self): + path = base_path + ['local-route'] + + src = '203.0.113.11' + dst = '203.0.113.0/24' + fwmk = '23' + rule = '100' + table = '150' + self.cli_set(path + ['rule', rule, 'set', 'table', table]) + self.cli_set(path + ['rule', rule, 'source', src]) + self.cli_set(path + ['rule', rule, 'destination', dst]) + self.cli_set(path + ['rule', rule, 'fwmark', fwmk]) + + self.cli_commit() + + original = """ + 100: from 203.0.113.11 to 203.0.113.0/24 fwmark 0x17 lookup 150 + """ + tmp = cmd('ip rule show prio 100') + original = original.split() + tmp = tmp.split() + + self.assertEqual(tmp, original) + + self.cli_delete(path + ['rule', rule, 'source', src]) + self.cli_commit() + + original = """ + 100: from all to 203.0.113.0/24 fwmark 0x17 lookup 150 + """ + tmp = cmd('ip rule show prio 100') + original = original.split() + tmp = tmp.split() + self.assertEqual(tmp, original) + +def sort_ip(output): + o = '\n'.join([' '.join(line.strip().split()) for line in output.strip().splitlines()]) + o = o.splitlines() + o.sort() + return o + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/src/conf_mode/policy-local-route.py b/src/conf_mode/policy-local-route.py index 3f834f55c..c32463d04 100755 --- a/src/conf_mode/policy-local-route.py +++ b/src/conf_mode/policy-local-route.py @@ -100,14 +100,14 @@ def get_config(config=None): rule_def = dict_merge({'source' : src}, rule_def) if fwmk is None: if 'fwmark' in rule_config: - rule_def = dict_merge({'fwmark': rule_config['fwmark']}, rule_def) + rule_def = dict_merge({'fwmark': [rule_config['fwmark']]}, rule_def) else: changed = True if len(fwmk) > 0: rule_def = dict_merge({'fwmark' : fwmk}, rule_def) if iif is None: if 'inbound_interface' in rule_config: - rule_def = dict_merge({'inbound_interface': rule_config['inbound_interface']}, rule_def) + rule_def = dict_merge({'inbound_interface': [rule_config['inbound_interface']]}, rule_def) else: changed = True if len(iif) > 0: -- cgit v1.2.3 From 19e85acabcbc2eb839a2624d5e5e422ae675c7da Mon Sep 17 00:00:00 2001 From: Henning Surmeier Date: Sat, 9 Apr 2022 13:21:26 +0200 Subject: respect table changes for remove_rule --- smoketest/scripts/cli/test_policy.py | 52 ++++++++++++++++++++++++------------ src/conf_mode/policy-local-route.py | 16 ++++++++++- 2 files changed, 50 insertions(+), 18 deletions(-) (limited to 'src/conf_mode/policy-local-route.py') diff --git a/smoketest/scripts/cli/test_policy.py b/smoketest/scripts/cli/test_policy.py index a636a8097..ab63cbcf7 100755 --- a/smoketest/scripts/cli/test_policy.py +++ b/smoketest/scripts/cli/test_policy.py @@ -902,8 +902,6 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase): self.cli_commit() - # Check generated configuration - # Expected values original = """ 102: from 2001:db8:1338::/126 iif lo lookup 150 102: from 2001:db8:1339::/126 iif lo lookup 150 @@ -1047,10 +1045,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase): 101: from all fwmark 0x18 lookup 154 """ tmp = cmd('ip rule show prio 101') - original = original.split() - tmp = tmp.split() - - self.assertEqual(tmp, original) + self.assertEqual(sort_ip(tmp), sort_ip(original)) # Test set table for sources with fwmark def test_fwmark_sources_table_id(self): @@ -1072,10 +1067,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase): 100: from 203.0.113.12 fwmark 0x17 lookup 150 """ tmp = cmd('ip rule show prio 100') - original = original.split() - tmp = tmp.split() - - self.assertEqual(tmp, original) + self.assertEqual(sort_ip(tmp), sort_ip(original)) # Test remove fwmark for sources with fwmark def test_source_fwmk_remove(self): @@ -1097,10 +1089,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase): 100: from 203.0.113.11 to 203.0.113.0/24 fwmark 0x17 lookup 150 """ tmp = cmd('ip rule show prio 100') - original = original.split() - tmp = tmp.split() - - self.assertEqual(tmp, original) + self.assertEqual(sort_ip(tmp), sort_ip(original)) self.cli_delete(path + ['rule', rule, 'source', src]) self.cli_commit() @@ -1109,9 +1098,38 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase): 100: from all to 203.0.113.0/24 fwmark 0x17 lookup 150 """ tmp = cmd('ip rule show prio 100') - original = original.split() - tmp = tmp.split() - self.assertEqual(tmp, original) + self.assertEqual(sort_ip(tmp), sort_ip(original)) + + # Test change table for sources with fwmark + def test_source_change_table(self): + path = base_path + ['local-route'] + + src = '203.0.113.11' + dst = '203.0.113.0/24' + fwmk = '23' + rule = '100' + table = '150' + self.cli_set(path + ['rule', rule, 'set', 'table', table]) + self.cli_set(path + ['rule', rule, 'source', src]) + self.cli_set(path + ['rule', rule, 'destination', dst]) + self.cli_set(path + ['rule', rule, 'fwmark', fwmk]) + + self.cli_commit() + + original = """ + 100: from 203.0.113.11 to 203.0.113.0/24 fwmark 0x17 lookup 150 + """ + tmp = cmd('ip rule show prio 100') + self.assertEqual(sort_ip(tmp), sort_ip(original)) + + self.cli_set(path + ['rule', rule, 'set', 'table', '151']) + self.cli_commit() + + original = """ + 100: from 203.0.113.11 to 203.0.113.0/24 fwmark 0x17 lookup 151 + """ + tmp = cmd('ip rule show prio 100') + self.assertEqual(sort_ip(tmp), sort_ip(original)) def sort_ip(output): o = '\n'.join([' '.join(line.strip().split()) for line in output.strip().splitlines()]) diff --git a/src/conf_mode/policy-local-route.py b/src/conf_mode/policy-local-route.py index c32463d04..8a92bbc76 100755 --- a/src/conf_mode/policy-local-route.py +++ b/src/conf_mode/policy-local-route.py @@ -54,6 +54,7 @@ def get_config(config=None): fwmk = leaf_node_changed(conf, base_rule + [rule, 'fwmark']) iif = leaf_node_changed(conf, base_rule + [rule, 'inbound-interface']) dst = leaf_node_changed(conf, base_rule + [rule, 'destination']) + table = leaf_node_changed(conf, base_rule + [rule, 'set', 'table']) rule_def = {} if src: rule_def = dict_merge({'source' : src}, rule_def) @@ -63,6 +64,8 @@ def get_config(config=None): rule_def = dict_merge({'inbound_interface' : iif}, rule_def) if dst: rule_def = dict_merge({'destination' : dst}, rule_def) + if table: + rule_def = dict_merge({'table' : table}, rule_def) dict = dict_merge({dict_id : {rule : rule_def}}, dict) pbr.update(dict) @@ -78,6 +81,7 @@ def get_config(config=None): fwmk = leaf_node_changed(conf, base_rule + [rule, 'fwmark']) iif = leaf_node_changed(conf, base_rule + [rule, 'inbound-interface']) dst = leaf_node_changed(conf, base_rule + [rule, 'destination']) + table = leaf_node_changed(conf, base_rule + [rule, 'set', 'table']) # keep track of changes in configuration # otherwise we might remove an existing node although nothing else has changed changed = False @@ -119,6 +123,13 @@ def get_config(config=None): changed = True if len(dst) > 0: rule_def = dict_merge({'destination' : dst}, rule_def) + if table is None: + if 'set' in rule_config and 'table' in rule_config['set']: + rule_def = dict_merge({'table': [rule_config['set']['table']]}, rule_def) + else: + changed = True + if len(table) > 0: + rule_def = dict_merge({'table' : table}, rule_def) if changed: dict = dict_merge({dict_id : {rule : rule_def}}, dict) pbr.update(dict) @@ -179,7 +190,10 @@ def apply(pbr): rule_config['inbound_interface'] = rule_config['inbound_interface'] if 'inbound_interface' in rule_config else [''] for iif in rule_config['inbound_interface']: f_iif = '' if iif == '' else f' iif {iif} ' - call(f'ip{v6} rule del prio {rule} {f_src}{f_dst}{f_fwmk}{f_iif}') + rule_config['table'] = rule_config['table'] if 'table' in rule_config else [''] + for table in rule_config['table']: + f_table = '' if table == '' else f' lookup {table} ' + call(f'ip{v6} rule del prio {rule} {f_src}{f_dst}{f_fwmk}{f_iif}{f_table}') # Generate new config for route in ['local_route', 'local_route6']: -- cgit v1.2.3