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.

Sign up for GitHub

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

Jump to bottom

pim(6): T5733: add missing FRR related features #2476

Merged
merged 8 commits into from
Nov 15, 2023
Merged

pim(6): T5733: add missing FRR related features #2476

merged 8 commits into from
Nov 15, 2023

Conversation

c-po
Copy link
Member

@c-po c-po commented Nov 12, 2023 •
edited

Change Summary

Migrate CLI configuration retrival to common get_config_dict(). In addition add new functionality to VyOS that is PIM related and already available in FRR.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

Component(s) name

Proposed changes

How to test

set protocols pim rp address 127.0.0.1 group 224.0.0.0/4
set protocols pim interface eth0 bfd
set protocols pim interface eth0 dr-priority 64
set protocols pim interface eth0 hello 100
set protocols pim interface eth0 no-bsm
set protocols pim interface eth0 no-unicast-bsm
set protocols pim interface eth0 passive

Smoketest result

cpo@LR1.wue3:~$ /usr/libexec/vyos/tests/smoke/cli/test_protocols_igmp-proxy.py
test_igmpproxy (__main__.TestProtocolsIGMPProxy.test_igmpproxy) ... ok

----------------------------------------------------------------------
Ran 1 test in 6.980s

OK
cpo@LR1.wue3:~$ /usr/libexec/vyos/tests/smoke/cli/test_protocols_pim.py
test_01_pim_basic (__main__.TestProtocolsPIM.test_01_pim_basic) ... ok
test_02_pim_advanced (__main__.TestProtocolsPIM.test_02_pim_advanced) ... ok
test_03_pim_igmp_proxy (__main__.TestProtocolsPIM.test_03_pim_igmp_proxy) ... ok
test_04_igmp (__main__.TestProtocolsPIM.test_04_igmp) ... ok

----------------------------------------------------------------------
Ran 4 tests in 17.010s

OK
cpo@LR1.wue3:~$ /usr/libexec/vyos/tests/smoke/cli/test_protocols_pim6.py
test_pim6_01_mld_simple (__main__.TestProtocolsPIMv6.test_pim6_01_mld_simple) ... ok
test_pim6_02_mld_join (__main__.TestProtocolsPIMv6.test_pim6_02_mld_join) ... ok
test_pim6_03_basic (__main__.TestProtocolsPIMv6.test_pim6_03_basic) ... ok

----------------------------------------------------------------------
Ran 3 tests in 19.279s

OK

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro and sever-sever and removed request for a team November 12, 2023 17:32
dmbaturin
dmbaturin requested changes Nov 12, 2023
View reviewed changes
Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

I have no objections against the implementation, just did some proofreading of the help strings.

interface-definitions/protocols-pim.xml.in Outdated Show resolved Hide resolved
interface-definitions/protocols-pim.xml.in Outdated Show resolved Hide resolved
interface-definitions/protocols-pim.xml.in Outdated Show resolved Hide resolved
interface-definitions/protocols-pim.xml.in Outdated Show resolved Hide resolved
interface-definitions/protocols-pim.xml.in Outdated Show resolved Hide resolved
@c-po c-po changed the title pim: T5733: migrate to get_config_dict() pim: T5733: add missing FRR related features Nov 13, 2023
@c-po c-po force-pushed the frr-pim-T5733 branch 2 times, most recently from 83ba584 to 0d4b5ec Compare November 13, 2023 12:11
@c-po c-po mentioned this pull request Nov 13, 2023
Merged
@c-po
pim: T5733: add missing FRR PIM related features
9abc02e 
Migrate CLI configuration retrival to common get_config_dict(). In addition
add new functionality to VyOS that is PIM related and already available in FRR.
@c-po
pim: T5733: split out XML definitions to be re-used by pim6
c5e2c25
@c-po
pim: T5733: rename watermark-warn -> watermark-warning
45ea9ed
@c-po
igmp: T5736: migrate "protocols igmp" to "protocols pim"
bc83fb0 
IGMP and PIM are two different but related things.

FRR has both combined in pimd. As we use get_config_dict() and FRR reload it
is better to have both centrally stored under the same CLI node (as FRR does,
too) to just "fire and forget" the commit to the daemon.

"set protocols igmp interface eth1" -> "set protocols pim interface eth1 igmp"
@c-po
pim: T5733: fix CLI level of global PIM commands
dd13213
@c-po c-po marked this pull request as ready for review November 13, 2023 21:21
@vyosbot vyosbot requested a review from a team November 13, 2023 21:21
@c-po c-po changed the title pim: T5733: add missing FRR related features pim: pim6: T5733: add missing FRR related features Nov 13, 2023
@c-po c-po requested a review from dmbaturin November 13, 2023 21:30
@c-po c-po changed the title pim: pim6: T5733: add missing FRR related features pim(6): T5733: add missing FRR related features Nov 13, 2023
sever-sever
sever-sever approved these changes Nov 15, 2023
View reviewed changes
interface-definitions/include/pim/dr-priority.xml.i Outdated Show resolved Hide resolved
interface-definitions/include/pim/join-prune-interval.xml.i Outdated Show resolved Hide resolved
interface-definitions/include/pim/register-suppress-time.xml.i Outdated Show resolved Hide resolved
interface-definitions/protocols-pim.xml.in Outdated Show resolved Hide resolved
src/conf_mode/protocols_pim.py Outdated Show resolved Hide resolved
@c-po c-po merged commit e085f3e into vyos:current Nov 15, 2023
4 of 5 checks passed
@c-po c-po deleted the frr-pim-T5733 branch November 15, 2023 17:21
@c-po
Copy link
Member Author

c-po commented Nov 15, 2023

@Mergifyio backport sagitta

@mergify Mergify
Copy link

mergify bot commented Nov 15, 2023 •
edited

backport sagitta

✅ Backports have been created

@mergify mergify bot mentioned this pull request Nov 15, 2023
Merged
c-po added a commit that referenced this pull request Nov 16, 2023
@c-po
Merge pull request #2489 from vyos/mergify/bp/sagitta/pr-2476
71823db 
pim(6): T5733: add missing FRR related features (backport #2476)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sever-sever sever-sever sever-sever approved these changes

@sarthurdev sarthurdev Awaiting requested review from sarthurdev sarthurdev is a code owner automatically assigned from vyos/reviewers

@zdc zdc Awaiting requested review from zdc zdc is a code owner automatically assigned from vyos/reviewers

@jestabro jestabro Awaiting requested review from jestabro jestabro is a code owner automatically assigned from vyos/reviewers

@dmbaturin dmbaturin Awaiting requested review from dmbaturin

Assignees

@c-po c-po

Labels
current
Milestone
No milestone
3 participants
@c-po @dmbaturin @sever-sever