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

frr: upgrade to 10.2 and migrate protocols to unified FRRender class #4227

Merged
merged 28 commits into from
Dec 16, 2024

Conversation

c-po
Copy link
Member

@c-po c-po commented Dec 8, 2024

Change Summary

With FRR 10.0 daemons started to be migrated to integrated FRR mgmtd and a northbound interface. This led to some drawbacks in the current state how changes to FRR are handled. The current implementation will use frr-reload.py
and specifies excatly WHICH daemon needs a config update and will only replace this part inside FRR.

With FRR10 and mgmtd when a partial configuration is sent to mgmtd, it will remove configuration parts from other daemons like bgpd or ospfd which have not yet been migrated to mgmtd.

It's not possible to call frr-reload.py with daemon mgmtd - it will error out.

This commit will also change the CLI for static routes:

CLI command set protocols static route 10.0.0.0/8 next-hop 1.2.3.4 bfd multi-hop source 1.1.1.1 will be split into:

  • set protocols static route 10.0.0.0/8 next-hop 1.2.3.4 bfd source-address 1.1.1.1
  • set protocols static route 10.0.0.0/8 next-hop 1.2.3.4 bfd multi-hop

To make the XML blocks reusable, and comply with the FRR CLI - this was actually a wrong implementation from the beginning as you can not have multiple BFD source addresses.

CLI command set protocols static route 10.0.0.0/8 next-hop 1.2.3.4 bfd multi-hop source 1.1.1.1 profile bar is changed to:

  • set protocols static route 10.0.0.0/8 next-hop 1.2.3.4 bfd profile bar

CLI commands set protocols static multicast interface-route is moved to:

  • set protocols static multicast route <x.x.x.x/x> interface to have an identical look and feel with regular static routes.

TODO

  • Add performance improvement to render FRR config only once
  • Add smoketests for migrated FRR configurations

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): Package Upgrade

Related Task(s)

Related PR(s)

Component(s) name

FRR and all subsequent routing protocols

Smoketest result

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

Copy link

github-actions bot commented Dec 8, 2024


PR title does not match the required format

@c-po c-po changed the title frr: upgrade routing suite to 10.2 frr: upgrade to 10.2 and migrate protocols to unified FRRender class Dec 8, 2024
@c-po c-po force-pushed the T6746-frr-10 branch 4 times, most recently from 3bdd64a to 74bc43d Compare December 12, 2024 20:44
@c-po c-po marked this pull request as ready for review December 13, 2024 13:00
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 might argue that some parts of the PR like new logging commands could be separate PRs but I don't want to demand breaking it down — if we are to cherry-pick, all bits are still useful and uncontroversial.

c-po added 20 commits December 16, 2024 22:21
Drop newlines added by macro statement and Jinja2 comments. Jinja2 comments
will be removed during package build on the shipped files.
With FRR 10.0 daemons started to be migrated to integrated FRR mgmtd and a
northbound interface. This led to some drawbacks in the current state how
changes to FRR are handled. The current implementation will use frr-reload.py
and specifies excatly WHICH daemon needs a config update and will only replace
this part inside FRR.

With FRR10 and mgmtd when a partial configuration is sent to mgmtd, it will
remove configuration parts from other daemons like bgpd or ospfd which have
not yet been migrated to mgmtd.

It's not possible to call frr-reload.py with daemon mgmtd - it will error out.

This commit will also change the CLI for static routes:

CLI command "set protocols static route 10.0.0.0/8 next-hop 1.2.3.4 bfd multi-hop
source 1.1.1.1" will be split into:
* set protocols static route 10.0.0.0/8 next-hop 1.2.3.4 bfd source-address 1.1.1.1
* set protocols static route 10.0.0.0/8 next-hop 1.2.3.4 bfd multi-hop

To make the XML blocks reusable, and comply with the FRR CLI - this was actually
a wrong implementation from the beginning as you can not have multiple BFD
source addresses.

CLI command "set protocols static route 10.0.0.0/8 next-hop 1.2.3.4 bfd multi-hop
source 1.1.1.1 profile bar" is changed to:
* set protocols static route 10.0.0.0/8 next-hop 1.2.3.4 bfd profile bar

CLI commands "set protocols static multicast interface-route" is moved to:
* set protocols static multicast route <x.x.x.x/x> interface

To have an identical look and feel with regular static routes.
…" CLI tagNode

This will save an entire level for the configuration and there is no need for a
parent "multicast" node, as it will only have "route" as tagNode below.

Move set protocols static multicast route <x.x.x.x/y> to:
* set protocols static mroute <x.x.x.x/y>
This is pretty usefull to monitor what's going on under the hood

Dec 08 15:27:34 vyos-configd[4324]: Received message: {"type": "init"}
Dec 08 15:27:34 vyos-configd[4324]: config session pid is 4400
Dec 08 15:27:34 vyos-configd[4324]: config session sudo_user is cpo
Dec 08 15:27:34 vyos-configd[4324]: commit_scripts: ['protocols_babel', 'protocols_bfd', 'protocols_bgp']
Dec 08 15:27:34 vyos-configd[4324]: Received message: {"type": "node", "last": false, "data": "/usr/libexec/vyos/conf_mode/protocols_babel.py"}
Dec 08 15:27:34 vyos-configd[4324]: Sending reply: error_code 1 with output
Dec 08 15:27:34 vyos-configd[4324]: Received message: {"type": "node", "last": false, "data": "/usr/libexec/vyos/conf_mode/protocols_bgp.py"}
Dec 08 15:27:34 vyos-configd[4324]: Sending reply: error_code 1 with output
Dec 08 15:27:34 vyos-configd[4324]: Received message: {"type": "node", "last": true, "data": "/usr/libexec/vyos/conf_mode/protocols_bfd.py"}
Dec 08 15:27:34 vyos-configd[4324]: Sending reply: error_code 1 with output
Dec 08 15:27:34 vyos-configd[4324]: scripts_called: ['protocols_babel', 'protocols_bgp', 'protocols_bfd']
Dec 08 15:27:34 vyos-configd[4324]: FRR: Reloading configuration - tries: 1 Python class ID: 139842739583248

Debugging the new FRRender/vyos-config integration
A lot of services have dynamic debug capabilities which will be turned on by
creating a file in /tmp. These scripts have the path hardcoded and sometimes
accross multiple places (bad).

This commit introduces vyos.defaults.frr_debug_enable to get the path for the
debug file from a single location.
When running under vyos-configd only a single apply() is done as last step in
the commit algorithm. FRRender class address is provided via an attribute from
vyos-configd process.
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.
Migrate "set protocols static route <x.x.x.x/x> next-hop <y.y.y.y> bfd multi-hop
source <z.z.z.z> profile <NAME>" to: "set protocols static route <x.x.x.x/x>
next-hop <y.y.y.y> bfd profile bar"

FRR supports only one source IP address per BFD multi-hop session. VyOS
had CLI cupport for multiple source addresses which made no sense.
VNI was always retrieved via effective configuration and not active
configuration.
Consolidate "multicast interface-route" and "multicast route" under common
"mroute <x.x.x.x/y>" CLI node.
Sometimes FRR needs some time after reloading the configuration to appear in
vtysh. This is a workaround addiung a 2 second guard timer.
Do not use custom daemon definitions like bgpd - re-use them from e.g.
vyos.frrender.bgp_daemon
As vyos-configd will take care about the commit via FRRender class, and FRR
needs to internally process the configuration we might read it back via vtysh
"to fast". Add a 5 seconds guard timer after each cli_commit() and before
calling getFRRconfig().

Guard timer is reset every time, cli_commit() is called.
@c-po c-po merged commit 1da518c into vyos:current Dec 16, 2024
7 of 9 checks passed
@c-po c-po deleted the T6746-frr-10 branch December 16, 2024 21:25
Copy link

CI integration ❌ failed!

Details

CI logs

  • CLI Smoketests (no interfaces) ❌ failed
  • CLI Smoketests (interfaces only) ❌ failed
  • Config tests 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants