-
Notifications
You must be signed in to change notification settings - Fork 105
Add bgp.role support to bgp.policy plugin #3429
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
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
|
|
||
| from box import Box | ||
|
|
||
| from netsim import api, data | ||
| from netsim import api, data, modules | ||
| from netsim.augment import devices | ||
| from netsim.data import types | ||
| from netsim.modules.routing.policy import check_routing_policy, import_routing_policy | ||
|
|
@@ -11,7 +11,10 @@ | |
|
|
||
| _config_name = 'bgp.policy' | ||
|
|
||
| _requires = [ 'bgp' ] | ||
| _requires = [ 'bgp' ] | ||
| _execute_after = [ 'bgp.session' ] | ||
|
|
||
| _ROLE_ATTR_LIST = [ 'role', 'role_strict' ] | ||
|
|
||
| @types.type_test() | ||
| def must_be_autobw( | ||
|
|
@@ -161,6 +164,44 @@ def apply_config(node: Box, ngb: Box) -> None: | |
| api.node_config(node,_config_name) # Remember that we have to do extra configuration | ||
| _bgp.clear_bgp_session(node,ngb) | ||
|
|
||
| def _use_role_plugin_template(node: Box) -> bool: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels so wrong. Why would you ever hard-code a device into a plugin code? |
||
| """Return False for the BIRD daemon (roles are rendered in daemons/bird/bgp.j2).""" | ||
| return not (node.get('_daemon') and node.device == 'bird') | ||
|
|
||
| def apply_role_attributes(node: Box, ngb: Box, intf: Box, topology: Box) -> bool: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels like duplicating a lot of functionality that should be already available elsewhere. If you want to ensure "role_strict" is only used when "role" is there, we could solve that in the generic validation code (if it's not there yet). |
||
| """Copy bgp.role* interface attributes to an EBGP neighbor.""" | ||
| values: dict[str, typing.Any] = {} | ||
| for attr in _ROLE_ATTR_LIST: | ||
| attr_value = modules.get_effective_module_attribute( | ||
| path=f'bgp.{attr}', intf=intf, node=node) | ||
| if attr_value: | ||
| values[attr] = attr_value | ||
|
|
||
| if not values: | ||
| return False | ||
|
|
||
| if not _bgp.check_device_attribute_support('role', node, ngb, topology, _config_name): | ||
| return False | ||
|
|
||
| if 'role_strict' in values and 'role' not in values: | ||
| where = f'node {node.name}' | ||
| if intf is not None: | ||
| where += f' interface {intf.name}' | ||
| log.error( | ||
| f'Cannot use bgp.role_strict without bgp.role ({where})', | ||
| category=log.IncorrectValue, | ||
| module=_config_name, | ||
| ) | ||
| return False | ||
|
|
||
| for attr, attr_value in values.items(): | ||
| ngb[attr] = attr_value | ||
|
|
||
| if _use_role_plugin_template(node): | ||
| apply_config(node, ngb) | ||
|
|
||
| return True | ||
|
|
||
| ''' | ||
| Apply attributes supported by bgp.policy plugin to a single neighbor | ||
| Returns True if at least some relevant attributes were found | ||
|
|
@@ -313,7 +354,7 @@ def post_transform(topology: Box) -> None: | |
| continue | ||
|
|
||
| route_aggregation(ndata,topology) | ||
| _bgp.cleanup_neighbor_attributes(ndata,topology,_attr_list + [ 'policy' ]) | ||
| _bgp.cleanup_neighbor_attributes(ndata,topology,_attr_list + _ROLE_ATTR_LIST + [ 'policy' ]) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be better to add role keywords into "_direct" or "_compound" attribute list? Is there a reason not to do it? |
||
| policy_idx = 0 | ||
|
|
||
| # Get _default_locpref feature flag (could be None), then figure out if we need to copy | ||
|
|
@@ -323,8 +364,7 @@ def post_transform(topology: Box) -> None: | |
| default_locpref = _bgp.get_device_bgp_feature('_default_locpref',ndata,topology) | ||
| copy_locpref = False if default_locpref else 'locpref' in ndata.bgp | ||
|
|
||
| # Now iterate over all EBGP neighbors (global and VRF) and apply bgp.policy interface | ||
| # attributes to the neighbors | ||
| # Iterate over BGP neighbors and apply bgp.policy interface attributes to EBGP sessions. | ||
| # | ||
| for (intf,ngb) in _bgp.intf_neighbors(ndata,select=['ebgp']): | ||
| policy_idx += 1 | ||
|
|
@@ -338,6 +378,7 @@ def post_transform(topology: Box) -> None: | |
| communities.append('extended') | ||
| if copy_locpref and not intf.get('bgp.locpref',False): | ||
| intf.bgp.locpref = ndata.bgp.locpref | ||
| apply_role_attributes(ndata,ngb,intf,topology) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like the only reason you're dealing with roles as an exception is because of Bird template stuff. The easiest way to solve that is to have an empty template in the plugin directory. |
||
| if intf.get('bgp.policy',{}): | ||
| apply_bgp_routing_policy(ndata,ngb,intf,topology) | ||
| if apply_policy_attributes(ndata,ngb,intf,topology): # If we applied at least some bgp.policy attribute to the neighbor | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move this section to the end (just before the "Sample Topologies")