Feature: bird vrf support including bgp, ospf and route leaking#3498
Conversation
|
Test vrf/14-multi-vrf-unnumbered.yml can be fixed once #3475 is merged |
ipspace
left a comment
There was a problem hiding this comment.
This will need a significant rework. The hacks like "include everything else in initial.j2" suck.
Also, for all other devices, we configure routing protocol instances within the VRF configuration; here, you do it in the main OSPF/BGP template. While I understand why you went down that route, it will make for an "interesting" experience when someone has to fix this code, and the two similar experiences I already had to go through persuaded me to avoid the third one at all costs 🤷🏻♂️
I know you're facing the challenge of doing data-plane configuration and daemon configuration in the same template, and I don't have a good answer for it yet... but it has to be better than this.
Currently, the issue is that a Somehow we need to make the |
4088875 to
7160c8b
Compare
|
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
5dbb382 to
1ce59db
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
There was a problem hiding this comment.
Pull request overview
Adds full VRF support for the BIRD “device” integration, covering per-VRF routing tables, VRF-aware BGP/OSPF instances, and RT-based route leaking (including VRF/inter-VRF static routes), plus documentation and feature-flag updates.
Changes:
- Implemented VRF-aware BIRD configuration generation (VRF tables, kernel/direct/static, RT-based pipe leaking, VRF BGP/OSPF).
- Refactored BIRD BGP/OSPF templates into reusable macro libraries and updated feature flags for VRF + static route capabilities.
- Reused a shared Linux VRF dataplane setup script across FRR/BIRD and updated platform/module docs to reflect new support.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| netsim/daemons/bird/vrf-daemon.j2 | New VRF table + protocol definitions, VRF static routes, RT-based leaking, and per-VRF OSPF/BGP rendering. |
| netsim/daemons/bird/protocols.j2 | Excludes VRF interfaces from the global direct protocol. |
| netsim/daemons/bird/ospf.macros.j2 | New OSPF rendering macro with VRF-aware table/import/export handling. |
| netsim/daemons/bird/ospf.j2 | Refactored to use ospf.macros.j2. |
| netsim/daemons/bird/bgp.macros.j2 | New BGP rendering macros, including VRF-aware sessions. |
| netsim/daemons/bird/bgp.j2 | Refactored to use bgp.macros.j2. |
| netsim/daemons/bird.yml | Advertises new BIRD VRF + routing.static (vrf/inter_vrf) feature support. |
| netsim/ansible/templates/vrf/frr.vrf.j2 | New shared script that creates Linux VRFs and assigns interfaces to them. |
| netsim/ansible/templates/vrf/frr.j2 | Refactored to include the shared VRF script. |
| netsim/ansible/templates/vrf/bird.j2 | Uses the shared VRF script for BIRD VRF dataplane setup. |
| docs/platforms.md | Marks BIRD VRF support in platform capability matrix. |
| docs/module/vrf.md | Documents BIRD VRF support and VRF protocol support. |
| docs/module/routing.md | Updates routing module support table to include BIRD static routes. |
| docs/module/routing-static.txt | Adds BIRD to static routing support table. |
| docs/module/bgp.md | Updates BIRD BGP feature support table. |
| docs/caveats.md | Clarifies BIRD router-id behavior for BGP/OSPF instances. |
| {% for l in netlab_interfaces if l.vrf is defined %} | ||
| interface -"{{ l.ifname }}"; | ||
| {% endfor %} |
| {% if vrf_name and vrf_data.import|default([]) %} | ||
| if netlab_vrf_rt ~ [ {% for rt in vrf_data.import %}{{ rt_value(rt) }}{% if not loop.last %}, {% endif %}{% endfor %} ] then { | ||
| ospf_metric2 = 20; | ||
| accept; | ||
| } | ||
| {% endif %} |
* Shebang in the top-level file * No dependence on variables set in included files * Renamed DP template to reflect what it does
The sysctl used in debian/trixie cannot figure out how to handle the interfaces with dots (for example, ethX.Y). The workaround is to use slashes in the sysctl parameter. Works on FRR and Bird.
| # Move interfaces and loopbacks to vrfs | ||
| {% for i in interfaces if i.vrf is defined %} | ||
| sysctl -qw net.ipv6.conf.{{ i.ifname }}.keep_addr_on_down=1 | ||
| sysctl -qw net/ipv6/conf/{{ i.ifname }}/keep_addr_on_down=1 |
There was a problem hiding this comment.
Similar issue in
?There was a problem hiding this comment.
That's not a problem (not saying it should not be fixed) because SVI interface names don't have dots in them.
|
Looks like we're almost there. I ran the whole set of integration tests, and (no surprise there) a few unexpected ones failed. When doing a large change, it's worth testing beyond the immediate feature you think you implemented.
I'm perfectly OK disabling BGP default route functionality for the moment (obviously it never worked as it should), documenting a caveat on VRF-to-global static routes and merge this. Thoughts? |
I saw your sysctl patch and found other places in the codebase where this might be a problem. It tends to be in areas of combinations of features that we currently don't test ( e.g. VRFs with VLANs and IPv6 ) I'm ok with documenting the tests that currently fail, and then fixing those in follow-up PRs (instead of trying to fix them all in this one big PR) |
Test results: