Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T5595 : PIM multicast bfd feature #2411

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions data/templates/frr/pimd.frr.j2
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@ interface {{ iface }}
no ip pim
!
{% endfor %}
{% for iface in pim.ifaces %}
interface {{ iface }}
{% for interface, interface_config in pim.ifaces.items() %}
interface {{ interface }}
ip pim
{% if pim.ifaces[iface].dr_prio %}
ip pim drpriority {{ pim.ifaces[iface].dr_prio }}
{% if interface_config.dr_prio %}
ip pim drpriority {{ interface_config.dr_prio }}
{% endif %}
{% if interface_config.hello %}
ip pim hello {{ interface_config.hello }}
{% endif %}
{% if pim.ifaces[iface].hello %}
ip pim hello {{ pim.ifaces[iface].hello }}
{% if interface_config.bfd is vyos_defined('enable') %}
ip pim bfd
{% endif %}
!
{% endfor %}
Expand Down
19 changes: 19 additions & 0 deletions interface-definitions/protocols-pim.xml.in
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,25 @@
</constraint>
</properties>
</leafNode>
<leafNode name="bfd">
<properties>
<help>BFD support</help>
<completionHelp>
<list>enable disable</list>
</completionHelp>
Copy link
Member

@sever-sever sever-sever Oct 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have bfd does it mean we want to enable it? Let’s avoid enable/disable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see BGP neighbors, there is a simple bfd node that is valueless

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All nodes with values like true/false, enable/disable that still remain in the system are legacy and should be converted to valueless as soon as it's feasible. The policy not to make any new ones has been in place since Vyatta times, in fact. ;)

<valueHelp>
<format>enable</format>
<description>BFD enable</description>
</valueHelp>
<valueHelp>
<format>disable</format>
<description>BFD disable</description>
</valueHelp>
<constraint>
<regex>(enable|disable)</regex>
</constraint>
</properties>
</leafNode>
</children>
</tagNode>
<node name="rp">
Expand Down
66 changes: 66 additions & 0 deletions smoketest/scripts/cli/test_protocols_pim.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
#!/usr/bin/env python3
#
# Copyright (C) 2019-2020 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
# published by the Free Software Foundation.
#
# This program 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 General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import os
import unittest

from base_vyostest_shim import VyOSUnitTestSHIM
from vyos.configsession import ConfigSessionError
from vyos.ifconfig import Section
from vyos.utils.process import process_named_running

PROCESS_NAME = 'pimd'
base_path = ['protocols', 'pim']

class TestProtocolsPIM(VyOSUnitTestSHIM.TestCase):
def tearDown(self):
# Check for running process
self.assertTrue(process_named_running(PROCESS_NAME))
self.cli_delete(base_path)
self.cli_commit()

def test_pim_01_simple(self):
rp = '127.0.0.1'
group = '224.0.0.0/4'
hello = '100'
# commit changes

self.cli_set(base_path + ['rp', 'address', rp, 'group', group])
interfaces = Section.interfaces('ethernet')

for interface in interfaces:
self.cli_set(base_path + ['interface', interface])
self.cli_set(base_path + ['interface', interface , 'hello', hello])
self.cli_set(base_path + ['interface', interface , 'bfd', 'enable'])

self.cli_commit()


# Verify FRR pimd configuration
frrconfig = self.getFRRconfig('ip pim rp {rp} {group}', daemon=PROCESS_NAME)

for interface in interfaces:
frrconfig = self.getFRRconfig(
f'interface {interface}', daemon=PROCESS_NAME)
self.assertIn(f'interface {interface}', frrconfig)
self.assertIn(f' ip pim', frrconfig)
self.assertIn(f' ip pim bfd', frrconfig)
self.assertIn(f' ip pim hello {hello}', frrconfig)

self.cli_commit()

if __name__ == '__main__':
unittest.main(verbosity=2)
4 changes: 3 additions & 1 deletion src/conf_mode/protocols_pim.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,16 @@ def get_config(config=None):
for iface in conf.list_effective_nodes('interface'):
pim_conf['old_pim']['ifaces'].update({
iface : {
'bfd' : conf.return_effective_value('interface {0} bfd'.format(iface)),
'hello' : conf.return_effective_value('interface {0} hello'.format(iface)),
'dr_prio' : conf.return_effective_value('interface {0} dr-priority'.format(iface))
'dr_prio' : conf.return_effective_value('interface {0} dr-priority'.format(iface)),
}
})

for iface in conf.list_nodes('interface'):
pim_conf['pim']['ifaces'].update({
iface : {
'bfd' : conf.return_value('interface {0} bfd'.format(iface)),
'hello' : conf.return_value('interface {0} hello'.format(iface)),
'dr_prio' : conf.return_value('interface {0} dr-priority'.format(iface)),
}
Expand Down
Loading