-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add templates and code to support VoQ chassis iBGP peers #5622
Conversation
* Add support to convert a new VoQChassisInternal element in the BGPSession element of the minigraph to a new BGP_VOQ_CHASSIS_NEIGHBOR table in CONFIG_DB. * Add a new set of "voq_chassis" templates to docker-fpm-frr * Add a new BGP peer manager to bgpcfgd to add neighbors from the BGP_VOQ_CHASSIS_NEIGHBOR table using the voq_chassis templates. * Add a test case for minigraph.py, making sure the VoQChassisInternal element creates a BGP_VOQ_CHASSIS_NEIGHBOR entry, but not if its value is "false". * Add a set of test cases for the new voq_chassis templates in sonic-bgpcfgd tests. Note that the templates expect the new "bgp bestpath peer-type multipath-relax" bgpd configuration to be available. Signed-off-by: Joanne Mikkelson <[email protected]>
retest baseimage please |
This includes a bit of refactoring of parse_cpg in src/sonic-config-engine/minigraph.py, to avoid having 3 copies of the code that creates the bgp_*sessions tables.
src/sonic-bgpcfgd/tests/data/voq_chassis/instance.conf/param_all.json
Outdated
Show resolved
Hide resolved
- Remove the require_intf_in_config_db workaround in bgpcfgd. The VOQ_INBAND_INTERFACE table is now in config_db and adding an InterfaceMgr works, so now we do not need to exempt VOQ chassis neighbors from the interface check. - Remove the BackEnd sub_role from the voq_chassis bgpcfgd test inputs in response to a comment on the PR. It was copy-pasted from the general templates and had no effect on the test, but since it does not belong, it is gone now.
dockers/docker-fpm-frr/frr/bgpd/templates/voq_chassis/peer-group.conf.j2
Show resolved
Hide resolved
dockers/docker-fpm-frr/frr/bgpd/templates/voq_chassis/peer-group.conf.j2
Show resolved
Hide resolved
src/sonic-config-engine/minigraph.py
Outdated
voq_chassis = session.find(str(QName(ns, "VoQChassisInternal"))) | ||
if voq_chassis is not None and voq_chassis.text == "true": | ||
table = bgp_voq_chassis_sessions | ||
admin_status = None |
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.
should the internal sessions be up by default, if not the admin status will be changed by CLI ?
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.
It was agreed the sessions would be up by default so that routing works as intended as soon as any eBGP session comes up. The admin status should be changeable with the CLI though this PR does not cover the CLI. Are you saying admin_status should be 'up', rather than not-specified? Not-specified is not 'down' so the sessions would be configured up either way.
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.
My point was should we set the admin_status to up here as we want these session to up by default.
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.
Thanks. I have changed it to 'up'. Please take a look!
Previously admin_status was not populated for these peers.
705529c
@jmmikkel, the unit tests are failing. Can you take a look |
Ugh, how embarrassing, I had tested it to work in our local build system, then copied over the change wrong. |
@arlakshm I don't think this failure is due to my changes. Is there a way to retry this test or does it need a full rebuild? Thanks |
…onic-net#5622) This commit has following changes: * Add templates and code to support VoQ chassis iBGP peers * Add support to convert a new VoQChassisInternal element in the BGPSession element of the minigraph to a new BGP_VOQ_CHASSIS_NEIGHBOR table in CONFIG_DB. * Add a new set of "voq_chassis" templates to docker-fpm-frr * Add a new BGP peer manager to bgpcfgd to add neighbors from the BGP_VOQ_CHASSIS_NEIGHBOR table using the voq_chassis templates. * Add a test case for minigraph.py, making sure the VoQChassisInternal element creates a BGP_VOQ_CHASSIS_NEIGHBOR entry, but not if its value is "false". * Add a set of test cases for the new voq_chassis templates in sonic-bgpcfgd tests. Note that the templates expect the new "bgp bestpath peer-type multipath-relax" bgpd configuration to be available. Signed-off-by: Joanne Mikkelson <[email protected]>
…onic-net#5622) This commit has following changes: * Add templates and code to support VoQ chassis iBGP peers * Add support to convert a new VoQChassisInternal element in the BGPSession element of the minigraph to a new BGP_VOQ_CHASSIS_NEIGHBOR table in CONFIG_DB. * Add a new set of "voq_chassis" templates to docker-fpm-frr * Add a new BGP peer manager to bgpcfgd to add neighbors from the BGP_VOQ_CHASSIS_NEIGHBOR table using the voq_chassis templates. * Add a test case for minigraph.py, making sure the VoQChassisInternal element creates a BGP_VOQ_CHASSIS_NEIGHBOR entry, but not if its value is "false". * Add a set of test cases for the new voq_chassis templates in sonic-bgpcfgd tests. Note that the templates expect the new "bgp bestpath peer-type multipath-relax" bgpd configuration to be available. Signed-off-by: Joanne Mikkelson <[email protected]>
BGPSession element of the minigraph to a new BGP_VOQ_CHASSIS_NEIGHBOR
table in CONFIG_DB.
BGP_VOQ_CHASSIS_NEIGHBOR table using the voq_chassis templates.
element creates a BGP_VOQ_CHASSIS_NEIGHBOR entry, but not if its
value is "false".
sonic-bgpcfgd tests.
Note that the templates expect the new
"bgp bestpath peer-type multipath-relax" bgpd configuration to be
available.
Signed-off-by: Joanne Mikkelson [email protected]
- Why I did it
For the VoQ chassis system, the iBGP sessions between the ASIC Instances have substantially different configuration needs from the usual eBGP neighbors. Pushing those differences into the "general" templates would involve a lot of if-statements and the like, while the bgpcfgd system looks designed around keeping a set of templates per kind of neighbor. So this change adds a new set of templates, keeping the VoQ chassis peers out of the other templates.
See the VoQ chassis BGP HLD for additional details, particularly of the contents of the new templates: sonic-net/SONiC#674
- How I did it
Added new voq_chassis directory in the fpm-frr docker image. In order to make these templates selectable, changed bgpcfgd to look at a new BGP_VOQ_CHASSIS_NEIGHBOR table in config_db. To get those entries into config_db, added a new element to BGPSession in the minigraph and code to honor it when parsing the minigraph.
- How to verify it
Boot a VoQ chassis system with VoQChassisInternal BGPSessions (minigraph) or BGP_VOQ_CHASSIS_NEIGHBOR config_db entries, and check that all the peerings in the iBGP mesh are up. Advertise routes from an eBGP neighbor and confirm they are learned by the iBGP peers.
- Which release branch to backport (provide reason below if selected)
- Description for the changelog
Add support for configuring VoQ chassis iBGP peers
- A picture of a cute animal (not mandatory but encouraged)