From df22720c7af9fab801f2f7b3b69afd05e6e511e9 Mon Sep 17 00:00:00 2001 From: Alex Kudentsov <43482574+alexk37@users.noreply.github.com> Date: Tue, 6 Oct 2026 13:25:22 +0700 Subject: [PATCH] bgp: T9404: do not render "bgp default local-preference" when the value is 100 FRR does not print "bgp default local-preference 100" in show running-config because 100 is its default, so frr-reload never finds the line in the running config and sends it again on every reload. The command handler calls bgp_clear_star_soft_in() whether or not the value changed, so every reload ran "clear bgp * soft in" (once in pass 0 and twice in pass 1), a full inbound re-evaluation of every peer. Add the default (100) to the XML definition of parameters default local-pref. Templates cannot query the XML definition while rendering, so frrender takes the value from the config defaults of "parameters default" (conf.get_config_defaults) and adds it to the BGP dict as the key xml_default_local_pref, for the global instance and for every VRF instance. bgpd.frr.j2 renders the line only when the configured value differs from xml_default_local_pref, so the value 100 is not hardcoded in the template. A real value change (100 to 200, or 200 to 100) is still sent: frr-reload sees the difference between the rendered file and the running config. --- data/templates/frr/bgpd.frr.j2 | 3 +- .../include/bgp/protocol-common-config.xml.i | 1 + python/vyos/frrender.py | 8 +++ smoketest/scripts/cli/test_protocols_bgp.py | 65 +++++++++++++++++++ 4 files changed, 76 insertions(+), 1 deletion(-) diff --git a/data/templates/frr/bgpd.frr.j2 b/data/templates/frr/bgpd.frr.j2 index e2def6e6f7..7671576d13 100644 --- a/data/templates/frr/bgpd.frr.j2 +++ b/data/templates/frr/bgpd.frr.j2 @@ -619,7 +619,8 @@ router bgp {{ system_as }} {{ 'vrf ' ~ vrf if vrf is vyos_defined }} {{ 'as-nota {# Doesn't work in current FRR configuration; vtysh (bgp dampening 16 751 2001 61) #} bgp dampening {{ parameters.dampening.half_life }} {{ parameters.dampening.re_use if parameters.dampening.re_use is vyos_defined }} {{ parameters.dampening.start_suppress_time if parameters.dampening.start_suppress_time is vyos_defined }} {{ parameters.dampening.max_suppress_time if parameters.dampening.max_suppress_time is vyos_defined }} {% endif %} -{% if parameters.default.local_pref is vyos_defined %} +{# xml_default_local_pref is the XML default of parameters default local-pref, added to the dict by frrender #} +{% if parameters.default.local_pref is vyos_defined and parameters.default.local_pref != xml_default_local_pref %} bgp default local-preference {{ parameters.default.local_pref }} {% endif %} {% if parameters.deterministic_med is vyos_defined %} diff --git a/interface-definitions/include/bgp/protocol-common-config.xml.i b/interface-definitions/include/bgp/protocol-common-config.xml.i index 44aa7f6f95..d4b46a7319 100644 --- a/interface-definitions/include/bgp/protocol-common-config.xml.i +++ b/interface-definitions/include/bgp/protocol-common-config.xml.i @@ -1427,6 +1427,7 @@ + 100 diff --git a/python/vyos/frrender.py b/python/vyos/frrender.py index 69bb93a968..eac03e4d37 100644 --- a/python/vyos/frrender.py +++ b/python/vyos/frrender.py @@ -297,6 +297,12 @@ def dict_helper_nhrp_defaults(nhrp): no_tag_node_value_mangle=True, with_recursive_defaults=True) bgp['dependent_vrfs'] = {} + # The XML default of "parameters default local-pref" is added to the + # dict, the template cannot query it while rendering. Only the node + # "parameters default" is asked for, not the whole BGP tree. + tmp = conf.get_config_defaults(bgp_cli_path + ['parameters', 'default'], + key_mangling=('-', '_'), get_first_key=True) + bgp['xml_default_local_pref'] = tmp.get('local_pref') dict.update({'bgp' : bgp}) elif conf.exists_effective(bgp_cli_path): dict.update({'bgp' : {'deleted' : '', 'dependent_vrfs' : {}}}) @@ -522,6 +528,8 @@ def dict_helper_nhrp_defaults(nhrp): # merge in remaining default values vrf_config['protocols']['bgp'] = config_dict_merge(default_values, vrf_config['protocols']['bgp']) + vrf_config['protocols']['bgp']['xml_default_local_pref'] = dict_search( + 'parameters.default.local_pref', default_values) # Add this BGP VRF instance as dependency into the default VRF if 'bgp' in dict: diff --git a/smoketest/scripts/cli/test_protocols_bgp.py b/smoketest/scripts/cli/test_protocols_bgp.py index 2ab8c88440..3e417a0f56 100755 --- a/smoketest/scripts/cli/test_protocols_bgp.py +++ b/smoketest/scripts/cli/test_protocols_bgp.py @@ -2126,5 +2126,70 @@ def test_bgp_106_interface_l3vpn_multi_domain_switching(self): self.assertIn(f'interface {interface}', frrconfig) self.assertIn(f' mpls bgp l3vpn-multi-domain-switching', frrconfig) + def test_bgp_107_default_local_pref_default_value(self): + # T9404: FRR does not print "bgp default local-preference 100" in its + # running config because 100 is the default. If we render the line + # anyway, frr-reload finds it missing on every reload and sends it + # again, and FRR runs "clear bgp * soft in" for each of those. So the + # default value must not be rendered. + # + # This reads the generated FRR config file instead of getFRRconfig(): + # FRR never prints the line for 100, so vtysh cannot tell if we render it. + frr_conf = '/run/frr/config/vyos.frr.conf' + local_pref = ' bgp default local-preference' + + # the XML default (100) is always in the config dict, but it is the + # default, so nothing is rendered when local-pref is not configured + self.cli_commit() + frrconfig = read_file(frr_conf) + self.assertIn(f'router bgp {ASN}', frrconfig) + self.assertNotIn(local_pref, frrconfig) + + self.cli_set(base_path + ['parameters', 'default', 'local-pref', '100']) + self.cli_commit() + + frrconfig = read_file(frr_conf) + self.assertIn(f'router bgp {ASN}', frrconfig) + self.assertNotIn(local_pref, frrconfig) + + # a value other than the default is still rendered and applied + self.cli_set(base_path + ['parameters', 'default', 'local-pref', '200']) + self.cli_commit() + + self.assertIn(f'{local_pref} 200', read_file(frr_conf)) + frrconfig = self.getFRRconfig(f'router bgp {ASN}', stop_section='^exit') + self.assertIn(f'{local_pref} 200', frrconfig) + + # back to the default value, FRR must have the default again + self.cli_set(base_path + ['parameters', 'default', 'local-pref', '100']) + self.cli_commit() + + self.assertNotIn(local_pref, read_file(frr_conf)) + frrconfig = self.getFRRconfig(f'router bgp {ASN}', stop_section='^exit') + self.assertNotIn(local_pref, frrconfig) + + # the VRF instance has the same default: 100 is not rendered in the + # VRF block, 200 is rendered in the VRF block only + vrf_header = f'router bgp {ASN} vrf {import_vrf}' + vrf_local_pref = import_vrf_base + [import_vrf, 'protocols', 'bgp', 'parameters', 'default', 'local-pref'] + self.create_bgp_instances_for_import_test() + self.cli_set(vrf_local_pref + ['100']) + self.cli_commit() + + frrconfig = read_file(frr_conf) + self.assertIn(vrf_header, frrconfig) + self.assertNotIn(local_pref, frrconfig) + + self.cli_set(vrf_local_pref + ['200']) + self.cli_commit() + + # the global block comes first in the file, the VRF block after it + global_block, _, vrf_block = read_file(frr_conf).partition(vrf_header) + self.assertIn(f'router bgp {ASN}', global_block) + self.assertNotIn(local_pref, global_block) + self.assertIn(f'{local_pref} 200', vrf_block) + frrconfig = self.getFRRconfig(vrf_header, stop_section='^exit') + self.assertIn(f'{local_pref} 200', frrconfig) + if __name__ == '__main__': unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on())