Add static default routes when originating BGP default routes#3527
Conversation
| continue | ||
|
|
||
| append_to_list(ndata,'module','routing') # Activate the routing module in the node | ||
| append_to_list(topology,'module','routing') # ... assuming the developer setting BGP features was sane ;) |
There was a problem hiding this comment.
Hope this doesn't activate the routing module on all nodes in the topology?
There was a problem hiding this comment.
Of course not. This is done late in the process (well after the topology.module is copied into node.module). The same two calls are used to add static routes to hosts.
There was a problem hiding this comment.
In that case, what breaks if we don’t set this? Why is this necessary?
There was a problem hiding this comment.
In that case, what breaks if we don’t set this? Why is this necessary?
The static routes are not configured 🤷🏻♂️
Some devices (Junos, BIRD) need a default route in the routing table to originate a BGP default route. To simplify the configuration templates, the bgp.session plugin adds the necessary static routes and activates the 'routing' module on the affected node(s). The functionality is used to implement BGP default routes for BIRD Also added: * Coverage test to check all potential scenarios (IPv4, IPv6, DS, single neighbor, multiple neighbors, routing module active or not) * Non-default-route BGP neighbors in the integration test to check whether the default route is propagated only to the correct neighbors
ipspace
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the bgp.session plugin and BIRD templates to support devices that require an existing default route in the routing table before they can originate a BGP default route, by auto-adding discard static defaults (global/VRF) and enabling the routing module where needed.
Changes:
- Extend
bgp.sessionplugin post-transform logic to inject per-AF discard static default routes (global and per-VRF) whendefault_originateis used on devices requiringstatic. - Simplify BIRD BGP configuration by removing the per-neighbor “static reject default” helper and relying on routing/static-route infrastructure instead.
- Add integration and coverage tests validating default-originate behavior and route propagation/non-propagation.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/bgp.session/04-default-originate.yml | Expands the integration scenario to test default-originate propagation and non-propagation across additional peers (global + VRF). |
| tests/coverage/input/bgp-default.yml | Adds a coverage input topology exercising multiple global/VRF/IPv4/IPv6 default-originate combinations. |
| tests/coverage/expected/bgp-default.yml | Captures the expected transformed topology, including injected discard static defaults and auto-enabled routing module. |
| netsim/extra/bgp.session/plugin.py | Implements static default route injection and routing module activation for devices requiring static defaults to originate. |
| netsim/daemons/bird/bgp.macros.j2 | Removes the old “static reject default” macro and adjusts default-route handling in the export prefix selection function. |
| netsim/daemons/bird/bgp.j2 | Drops invocation of the removed default-originate static macro. |
| netsim/daemons/bird.yml | Changes BIRD default_originate feature to static to trigger static-default injection behavior. |
| route_list = [ # Find relevant static routes based on neighbor AFs | ||
| BGP_DEFAULT[af] for af in log.AF_LIST if af in ngb | ||
| ] |
There was a problem hiding this comment.
Probably not applicable, but I nevertheless rewrote the SR creation code
Some devices (Junos, BIRD) need a default route in the routing table to originate a BGP default route. To simplify the configuration templates, the bgp.session plugin adds the necessary static routes and activates the 'routing' module on the affected node(s).
Working:
Still missing: