frr: T6746: handle "system ip" and "system ipv6" with FRRender class

FRR 10.2 will use "[no] ip forwarding" and "[no] ipv6 forwarding" to enable or
disable IP(v6) forwarding. We no longer rely on sysctl as this was overridden
by FRR later on.

Remove code path for sysctl setting and solely rely on FRR.

7 files changed, 136 insertions, 135 deletions

diff --git a/data/templates/frr/zebra.route-map.frr.j2 b/data/templates/frr/zebra.route-map.frr.j2
index 669d58354..70a810f43 100644
--- a/data/templates/frr/zebra.route-map.frr.j2
+++ b/data/templates/frr/zebra.route-map.frr.j2
@@ -1,4 +1,6 @@
 !
+{{ 'no ' if disable_forwarding is vyos_defined }}{{ afi }} forwarding
+!
 {% if nht.no_resolve_via_default is vyos_defined %}
 no {{ afi }} nht resolve-via-default
 {% endif %}
diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index f5e84267e..9522d8fcc 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -775,6 +775,15 @@ def get_frrender_dict(conf, argv=None) -> dict:
     # At least one participating EVPN interface found, add to result dict
     if tmp: dict['interfaces'] = tmp
 
+    # Zebra prefix exchange for Kernel IP/IPv6 and routing protocols
+    for ip_version in ['ip', 'ipv6']:
+        ip_cli_path = ['system', ip_version]
+        ip_dict = conf.get_config_dict(ip_cli_path, key_mangling=('-', '_'),
+                                        get_first_key=True, with_recursive_defaults=True)
+        if ip_dict:
+            ip_dict['afi'] = ip_version
+            dict.update({ip_version : ip_dict})
+
     # Enable SNMP agentx support
     # SNMP AgentX support cannot be disabled once enabled
     if conf.exists(['service', 'snmp']):
diff --git a/python/vyos/frrender.py b/python/vyos/frrender.py
index f1bb39094..7a0b661a3 100644
--- a/python/vyos/frrender.py
+++ b/python/vyos/frrender.py
@@ -32,12 +32,16 @@ def debug(message):
         return
     print(message)
 
-pim_daemon = 'pimd'
-
 frr_protocols = ['babel', 'bfd', 'bgp', 'eigrp', 'isis', 'mpls', 'nhrp',
                  'openfabric', 'ospf', 'ospfv3', 'pim', 'pim6', 'rip',
                  'ripng', 'rpki', 'segment_routing', 'static']
 
+bgp_daemon = 'bgpd'
+isis_daemon = 'isisd'
+mgmt_daemon = 'mgmtd'
+pim_daemon = 'pimd'
+zebra_daemon = 'zebra'
+
 class FRRender:
     def __init__(self):
         self._frr_conf = '/run/frr/config/frr.conf'
@@ -100,6 +104,12 @@ class FRRender:
             if 'static' in config_dict and 'deleted' not in config_dict['static']:
                 output += render_to_string('frr/staticd.frr.j2', config_dict['static'])
                 output += '\n'
+            if 'ip' in config_dict and 'deleted' not in config_dict['ip']:
+                output += render_to_string('frr/zebra.route-map.frr.j2', config_dict['ip'])
+                output += '\n'
+            if 'ipv6' in config_dict and 'deleted' not in config_dict['ipv6']:
+                output += render_to_string('frr/zebra.route-map.frr.j2', config_dict['ipv6'])
+                output += '\n'
             return output
 
         debug('======< RENDERING CONFIG >======')
diff --git a/smoketest/scripts/cli/test_system_ip.py b/smoketest/scripts/cli/test_system_ip.py
index 4ab5e8181..7d730f7b2 100755
--- a/smoketest/scripts/cli/test_system_ip.py
+++ b/smoketest/scripts/cli/test_system_ip.py
@@ -18,12 +18,21 @@ import unittest
 from base_vyostest_shim import VyOSUnitTestSHIM
 
 from vyos.configsession import ConfigSessionError
-from vyos.utils.file import read_file
-from vyos.frr import mgmt_daemon
+from vyos.utils.system import sysctl_read
+from vyos.xml_ref import default_value
+from vyos.frrender import mgmt_daemon
+from vyos.frrender import zebra_daemon
 
 base_path = ['system', 'ip']
 
 class TestSystemIP(VyOSUnitTestSHIM.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        super(TestSystemIP, cls).setUpClass()
+        # ensure we can also run this test on a live system - so lets clean
+        # out the current configuration :)
+        cls.cli_delete(cls, base_path)
+
     def tearDown(self):
         self.cli_delete(base_path)
         self.cli_commit()
@@ -31,47 +40,45 @@ class TestSystemIP(VyOSUnitTestSHIM.TestCase):
     def test_system_ip_forwarding(self):
         # Test if IPv4 forwarding can be disabled globally, default is '1'
         # which means forwarding enabled
-        all_forwarding = '/proc/sys/net/ipv4/conf/all/forwarding'
-        self.assertEqual(read_file(all_forwarding), '1')
+        self.assertEqual(sysctl_read('net.ipv4.conf.all.forwarding'), '1')
 
         self.cli_set(base_path + ['disable-forwarding'])
         self.cli_commit()
+        self.assertEqual(sysctl_read('net.ipv4.conf.all.forwarding'), '0')
+        frrconfig = self.getFRRconfig('', end='', daemon=zebra_daemon)
+        self.assertIn('no ip forwarding', frrconfig)
 
-        self.assertEqual(read_file(all_forwarding), '0')
+        self.cli_delete(base_path + ['disable-forwarding'])
+        self.cli_commit()
+        self.assertEqual(sysctl_read('net.ipv4.conf.all.forwarding'), '1')
+        frrconfig = self.getFRRconfig('', end='', daemon=zebra_daemon)
+        self.assertNotIn('no ip forwarding', frrconfig)
 
     def test_system_ip_multipath(self):
         # Test IPv4 multipathing options, options default to off -> '0'
-        use_neigh = '/proc/sys/net/ipv4/fib_multipath_use_neigh'
-        hash_policy = '/proc/sys/net/ipv4/fib_multipath_hash_policy'
-
-        self.assertEqual(read_file(use_neigh), '0')
-        self.assertEqual(read_file(hash_policy), '0')
+        self.assertEqual(sysctl_read('net.ipv4.fib_multipath_use_neigh'), '0')
+        self.assertEqual(sysctl_read('net.ipv4.fib_multipath_hash_policy'), '0')
 
         self.cli_set(base_path + ['multipath', 'ignore-unreachable-nexthops'])
         self.cli_set(base_path + ['multipath', 'layer4-hashing'])
         self.cli_commit()
 
-        self.assertEqual(read_file(use_neigh), '1')
-        self.assertEqual(read_file(hash_policy), '1')
+        self.assertEqual(sysctl_read('net.ipv4.fib_multipath_use_neigh'), '1')
+        self.assertEqual(sysctl_read('net.ipv4.fib_multipath_hash_policy'), '1')
 
     def test_system_ip_arp_table_size(self):
-        # Maximum number of entries to keep in the ARP cache, the
-        # default is 8k
+        cli_default = int(default_value(base_path + ['arp', 'table-size']))
+        def _verify_gc_thres(table_size):
+            self.assertEqual(sysctl_read('net.ipv4.neigh.default.gc_thresh3'), str(table_size))
+            self.assertEqual(sysctl_read('net.ipv4.neigh.default.gc_thresh2'), str(table_size // 2))
+            self.assertEqual(sysctl_read('net.ipv4.neigh.default.gc_thresh1'), str(table_size // 8))
 
-        gc_thresh3 = '/proc/sys/net/ipv4/neigh/default/gc_thresh3'
-        gc_thresh2 = '/proc/sys/net/ipv4/neigh/default/gc_thresh2'
-        gc_thresh1 = '/proc/sys/net/ipv4/neigh/default/gc_thresh1'
-        self.assertEqual(read_file(gc_thresh3), '8192')
-        self.assertEqual(read_file(gc_thresh2), '4096')
-        self.assertEqual(read_file(gc_thresh1), '1024')
+        _verify_gc_thres(cli_default)
 
         for size in [1024, 2048, 4096, 8192, 16384, 32768]:
             self.cli_set(base_path + ['arp', 'table-size', str(size)])
             self.cli_commit()
-
-            self.assertEqual(read_file(gc_thresh3), str(size))
-            self.assertEqual(read_file(gc_thresh2), str(size // 2))
-            self.assertEqual(read_file(gc_thresh1), str(size // 8))
+            _verify_gc_thres(size)
 
     def test_system_ip_protocol_route_map(self):
         protocols = ['any', 'babel', 'bgp', 'connected', 'eigrp', 'isis',
diff --git a/smoketest/scripts/cli/test_system_ipv6.py b/smoketest/scripts/cli/test_system_ipv6.py
index 1c778a11d..be9751c4d 100755
--- a/smoketest/scripts/cli/test_system_ipv6.py
+++ b/smoketest/scripts/cli/test_system_ipv6.py
@@ -19,17 +19,21 @@ import unittest
 from base_vyostest_shim import VyOSUnitTestSHIM
 
 from vyos.configsession import ConfigSessionError
-from vyos.utils.file import read_file
-from vyos.frr import mgmt_daemon
+from vyos.utils.system import sysctl_read
+from vyos.xml_ref import default_value
+from vyos.frrender import mgmt_daemon
+from vyos.frrender import zebra_daemon
 
 base_path = ['system', 'ipv6']
 
-file_forwarding = '/proc/sys/net/ipv6/conf/all/forwarding'
-file_disable = '/proc/sys/net/ipv6/conf/all/disable_ipv6'
-file_dad = '/proc/sys/net/ipv6/conf/all/accept_dad'
-file_multipath = '/proc/sys/net/ipv6/fib_multipath_hash_policy'
-
 class TestSystemIPv6(VyOSUnitTestSHIM.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        super(TestSystemIPv6, cls).setUpClass()
+        # ensure we can also run this test on a live system - so lets clean
+        # out the current configuration :)
+        cls.cli_delete(cls, base_path)
+
     def tearDown(self):
         self.cli_delete(base_path)
         self.cli_commit()
@@ -37,16 +41,23 @@ class TestSystemIPv6(VyOSUnitTestSHIM.TestCase):
     def test_system_ipv6_forwarding(self):
         # Test if IPv6 forwarding can be disabled globally, default is '1'
         # which means forwearding enabled
-        self.assertEqual(read_file(file_forwarding), '1')
+        self.assertEqual(sysctl_read('net.ipv6.conf.all.forwarding'), '1')
 
         self.cli_set(base_path + ['disable-forwarding'])
         self.cli_commit()
+        self.assertEqual(sysctl_read('net.ipv6.conf.all.forwarding'), '0')
+        frrconfig = self.getFRRconfig('', end='', daemon=zebra_daemon)
+        self.assertIn('no ipv6 forwarding', frrconfig)
 
-        self.assertEqual(read_file(file_forwarding), '0')
+        self.cli_delete(base_path + ['disable-forwarding'])
+        self.cli_commit()
+        self.assertEqual(sysctl_read('net.ipv6.conf.all.forwarding'), '1')
+        frrconfig = self.getFRRconfig('', end='', daemon=zebra_daemon)
+        self.assertNotIn('no ipv6 forwarding', frrconfig)
 
     def test_system_ipv6_strict_dad(self):
         # This defaults to 1
-        self.assertEqual(read_file(file_dad), '1')
+        self.assertEqual(sysctl_read('net.ipv6.conf.all.accept_dad'), '1')
 
         # Do not assign any IPv6 address on interfaces, this requires a reboot
         # which can not be tested, but we can read the config file :)
@@ -54,11 +65,11 @@ class TestSystemIPv6(VyOSUnitTestSHIM.TestCase):
         self.cli_commit()
 
         # Verify configuration file
-        self.assertEqual(read_file(file_dad), '2')
+        self.assertEqual(sysctl_read('net.ipv6.conf.all.accept_dad'), '2')
 
     def test_system_ipv6_multipath(self):
         # This defaults to 0
-        self.assertEqual(read_file(file_multipath), '0')
+        self.assertEqual(sysctl_read('net.ipv6.fib_multipath_hash_policy'), '0')
 
         # Do not assign any IPv6 address on interfaces, this requires a reboot
         # which can not be tested, but we can read the config file :)
@@ -66,26 +77,24 @@ class TestSystemIPv6(VyOSUnitTestSHIM.TestCase):
         self.cli_commit()
 
         # Verify configuration file
-        self.assertEqual(read_file(file_multipath), '1')
+        self.assertEqual(sysctl_read('net.ipv6.fib_multipath_hash_policy'), '1')
 
     def test_system_ipv6_neighbor_table_size(self):
         # Maximum number of entries to keep in the ARP cache, the
         # default is 8192
+        cli_default = int(default_value(base_path + ['neighbor', 'table-size']))
 
-        gc_thresh3 = '/proc/sys/net/ipv6/neigh/default/gc_thresh3'
-        gc_thresh2 = '/proc/sys/net/ipv6/neigh/default/gc_thresh2'
-        gc_thresh1 = '/proc/sys/net/ipv6/neigh/default/gc_thresh1'
-        self.assertEqual(read_file(gc_thresh3), '8192')
-        self.assertEqual(read_file(gc_thresh2), '4096')
-        self.assertEqual(read_file(gc_thresh1), '1024')
+        def _verify_gc_thres(table_size):
+            self.assertEqual(sysctl_read('net.ipv6.neigh.default.gc_thresh3'), str(table_size))
+            self.assertEqual(sysctl_read('net.ipv6.neigh.default.gc_thresh2'), str(table_size // 2))
+            self.assertEqual(sysctl_read('net.ipv6.neigh.default.gc_thresh1'), str(table_size // 8))
+
+        _verify_gc_thres(cli_default)
 
         for size in [1024, 2048, 4096, 8192, 16384, 32768]:
             self.cli_set(base_path + ['neighbor', 'table-size', str(size)])
             self.cli_commit()
-
-            self.assertEqual(read_file(gc_thresh3), str(size))
-            self.assertEqual(read_file(gc_thresh2), str(size // 2))
-            self.assertEqual(read_file(gc_thresh1), str(size // 8))
+            _verify_gc_thres(size)
 
     def test_system_ipv6_protocol_route_map(self):
         protocols = ['any', 'babel', 'bgp', 'connected', 'isis',
@@ -143,4 +152,4 @@ class TestSystemIPv6(VyOSUnitTestSHIM.TestCase):
         self.assertNotIn(f'no ipv6 nht resolve-via-default', frrconfig)
 
 if __name__ == '__main__':
-    unittest.main(verbosity=2)
+    unittest.main(verbosity=2, failfast=True)
diff --git a/src/conf_mode/system_ip.py b/src/conf_mode/system_ip.py
index 5afb57404..374e6e611 100755
--- a/src/conf_mode/system_ip.py
+++ b/src/conf_mode/system_ip.py
@@ -17,17 +17,16 @@
 from sys import exit
 
 from vyos.config import Config
-from vyos.configdict import dict_merge
+from vyos.configdep import set_dependents
+from vyos.configdep import call_dependents
+from vyos.configdict import get_frrender_dict
+from vyos.configverify import has_frr_protocol_in_dict
 from vyos.configverify import verify_route_map
-from vyos.template import render_to_string
+from vyos.frrender import FRRender
 from vyos.utils.dict import dict_search
-from vyos.utils.file import write_file
 from vyos.utils.process import is_systemd_service_active
 from vyos.utils.system import sysctl_write
-from vyos.configdep import set_dependents
-from vyos.configdep import call_dependents
 from vyos import ConfigError
-from vyos import frr
 from vyos import airbag
 airbag.enable()
 
@@ -36,42 +35,36 @@ def get_config(config=None):
         conf = config
     else:
         conf = Config()
-    base = ['system', 'ip']
-
-    opt = conf.get_config_dict(base, key_mangling=('-', '_'),
-                               get_first_key=True,
-                               with_recursive_defaults=True)
-
-    # When working with FRR we need to know the corresponding address-family
-    opt['afi'] = 'ip'
-
-    # We also need the route-map information from the config
-    #
-    # XXX: one MUST always call this without the key_mangling() option! See
-    # vyos.configverify.verify_common_route_maps() for more information.
-    tmp = {'policy' : {'route-map' : conf.get_config_dict(['policy', 'route-map'],
-                                                          get_first_key=True)}}
-    # Merge policy dict into "regular" config dict
-    opt = dict_merge(tmp, opt)
 
     # If IPv4 ARP table size is set here and also manually in sysctl, the more
     # fine grained value from sysctl must win
     set_dependents('sysctl', conf)
+    return get_frrender_dict(conf)
 
-    return opt
+def verify(config_dict):
+    if not has_frr_protocol_in_dict(config_dict, 'ip'):
+        return None
+
+    opt = config_dict['ip']
+    opt['policy'] = config_dict['policy']
 
-def verify(opt):
     if 'protocol' in opt:
         for protocol, protocol_options in opt['protocol'].items():
             if 'route_map' in protocol_options:
                 verify_route_map(protocol_options['route_map'], opt)
     return
 
-def generate(opt):
-    opt['frr_zebra_config'] = render_to_string('frr/zebra.route-map.frr.j2', opt)
-    return
+def generate(config_dict):
+    if config_dict and 'frrender_cls' not in config_dict:
+        FRRender().generate(config_dict)
+    return None
+
+def apply(config_dict):
+    if not has_frr_protocol_in_dict(config_dict, 'ip'):
+
+        return None
+    opt = config_dict['ip']
 
-def apply(opt):
     # Apply ARP threshold values
     # table_size has a default value - thus the key always exists
     size = int(dict_search('arp.table_size', opt))
@@ -82,11 +75,6 @@ def apply(opt):
     # Minimum number of stored records is indicated which is not cleared
     sysctl_write('net.ipv4.neigh.default.gc_thresh1', size // 8)
 
-    # enable/disable IPv4 forwarding
-    tmp = dict_search('disable_forwarding', opt)
-    value = '0' if (tmp != None) else '1'
-    write_file('/proc/sys/net/ipv4/conf/all/forwarding', value)
-
     # configure multipath
     tmp = dict_search('multipath.ignore_unreachable_nexthops', opt)
     value = '1' if (tmp != None) else '0'
@@ -121,18 +109,11 @@ def apply(opt):
     # running when this script is called first. Skip this part and wait for initial
     # commit of the configuration to trigger this statement
     if is_systemd_service_active('frr.service'):
-        # Save original configuration prior to starting any commit actions
-        frr_cfg = frr.FRRConfig()
-
-        # The route-map used for the FIB (zebra) is part of the zebra daemon
-        frr_cfg.load_configuration(frr.mgmt_daemon)
-        frr_cfg.modify_section(r'no ip nht resolve-via-default')
-        frr_cfg.modify_section(r'ip protocol \w+ route-map [-a-zA-Z0-9.]+', stop_pattern='(\s|!)')
-        if 'frr_zebra_config' in opt:
-            frr_cfg.add_before(frr.default_add_before, opt['frr_zebra_config'])
-        frr_cfg.commit_configuration()
+        if config_dict and 'frrender_cls' not in config_dict:
+            FRRender().apply()
 
     call_dependents()
+    return None
 
 if __name__ == '__main__':
     try:
diff --git a/src/conf_mode/system_ipv6.py b/src/conf_mode/system_ipv6.py
index 90d5100d7..02c9a8201 100755
--- a/src/conf_mode/system_ipv6.py
+++ b/src/conf_mode/system_ipv6.py
@@ -18,17 +18,17 @@ import os
 
 from sys import exit
 from vyos.config import Config
-from vyos.configdict import dict_merge
+from vyos.configdep import set_dependents
+from vyos.configdep import call_dependents
+from vyos.configdict import get_frrender_dict
+from vyos.configverify import has_frr_protocol_in_dict
 from vyos.configverify import verify_route_map
-from vyos.template import render_to_string
+from vyos.frrender import FRRender
 from vyos.utils.dict import dict_search
 from vyos.utils.file import write_file
 from vyos.utils.process import is_systemd_service_active
 from vyos.utils.system import sysctl_write
-from vyos.configdep import set_dependents
-from vyos.configdep import call_dependents
 from vyos import ConfigError
-from vyos import frr
 from vyos import airbag
 airbag.enable()
 
@@ -37,42 +37,35 @@ def get_config(config=None):
         conf = config
     else:
         conf = Config()
-    base = ['system', 'ipv6']
-
-    opt = conf.get_config_dict(base, key_mangling=('-', '_'),
-                               get_first_key=True,
-                               with_recursive_defaults=True)
-
-    # When working with FRR we need to know the corresponding address-family
-    opt['afi'] = 'ipv6'
-
-    # We also need the route-map information from the config
-    #
-    # XXX: one MUST always call this without the key_mangling() option! See
-    # vyos.configverify.verify_common_route_maps() for more information.
-    tmp = {'policy' : {'route-map' : conf.get_config_dict(['policy', 'route-map'],
-                                                          get_first_key=True)}}
-    # Merge policy dict into "regular" config dict
-    opt = dict_merge(tmp, opt)
 
     # If IPv6 neighbor table size is set here and also manually in sysctl, the more
     # fine grained value from sysctl must win
     set_dependents('sysctl', conf)
+    return get_frrender_dict(conf)
+
+def verify(config_dict):
+    if not has_frr_protocol_in_dict(config_dict, 'ipv6'):
+        return None
 
-    return opt
+    opt = config_dict['ipv6']
+    opt['policy'] = config_dict['policy']
 
-def verify(opt):
     if 'protocol' in opt:
         for protocol, protocol_options in opt['protocol'].items():
             if 'route_map' in protocol_options:
                 verify_route_map(protocol_options['route_map'], opt)
     return
 
-def generate(opt):
-    opt['frr_zebra_config'] = render_to_string('frr/zebra.route-map.frr.j2', opt)
-    return
+def generate(config_dict):
+    if config_dict and 'frrender_cls' not in config_dict:
+        FRRender().generate(config_dict)
+    return None
+
+def apply(config_dict):
+    if not has_frr_protocol_in_dict(config_dict, 'ipv6'):
+        return None
+    opt = config_dict['ipv6']
 
-def apply(opt):
     # configure multipath
     tmp = dict_search('multipath.layer4_hashing', opt)
     value = '1' if (tmp != None) else '0'
@@ -88,11 +81,6 @@ def apply(opt):
     # Minimum number of stored records is indicated which is not cleared
     sysctl_write('net.ipv6.neigh.default.gc_thresh1', size // 8)
 
-    # enable/disable IPv6 forwarding
-    tmp = dict_search('disable_forwarding', opt)
-    value = '0' if (tmp != None) else '1'
-    write_file('/proc/sys/net/ipv6/conf/all/forwarding', value)
-
     # configure IPv6 strict-dad
     tmp = dict_search('strict_dad', opt)
     value = '2' if (tmp != None) else '1'
@@ -105,16 +93,11 @@ def apply(opt):
     # running when this script is called first. Skip this part and wait for initial
     # commit of the configuration to trigger this statement
     if is_systemd_service_active('frr.service'):
-        # Save original configuration prior to starting any commit actions
-        frr_cfg = frr.FRRConfig()
-        frr_cfg.load_configuration(frr.mgmt_daemon)
-        frr_cfg.modify_section(r'no ipv6 nht resolve-via-default')
-        frr_cfg.modify_section(r'ipv6 protocol \w+ route-map [-a-zA-Z0-9.]+', stop_pattern='(\s|!)')
-        if 'frr_zebra_config' in opt:
-            frr_cfg.add_before(frr.default_add_before, opt['frr_zebra_config'])
-        frr_cfg.commit_configuration()
+        if config_dict and 'frrender_cls' not in config_dict:
+            FRRender().apply()
 
     call_dependents()
+    return None
 
 if __name__ == '__main__':
     try: