FRR 4.0 integration with SONiC #2099
Conversation
-- Uses SONiC FRR repo frr/4.0 (which has SONiC support) to build image -- Makefile changes to make frr4.0 builtable. -- Updated/Added FRR configuration files -- bgpd jinja template fixes To build SONiC images with FRR4.0, simply edit rules/config file and change routing stack to following: SONIC_ROUTING_STACK = frr and then build images as usual.
zhenggen-xu
force-pushed
the
master-frr4.0
branch
from
7139630 to
116b86f
Compare
|
agree with jipan, please add a configuration generation test for frr bgpd.conf |
|
I am of the opinion that frr should use the unified config. |
|
@nikos-github The current change in PR disabled " integrated-vtysh-config" to make it consistent with quagga configurations, means separated configuration files. @lguohan @jipanyang Do you see concerns to move the configurations to unified config? In such case, we will have frr.conf instead of multiple configuration files. |
|
@zhenggen-xu I understand. Today though, bgpd has no purpose in running by itself (without zebra) in a sonic device and if it did, it wouldn't be able to communicate with sonic without zebra. So from frr's perspective, multiple configuration files don't really offer anything in my opinion. For simplicity, I propose that we move into unified config. |
|
Agree with Nikos here. We should exclusively rely on the Integrated-vtysh-mode (frr.conf) which is becoming the common/default operational mode in FRR community. It's not only simpler from an operational perspective, but it's also better tested nowadays. |
|
@zhenggen-xu Switching to Integrated-vtysh-mode for FRR looks like a good move to me. |
Changed to single template: frr.conf.j2 for configuration and added tests
|
Changed the code to use integrated-vtysh-config in FRR, also updated the template and test cases. |
|
what about unit test? need config gen unit test for frr.conf in frr. can you add. other changes are fine with me. |
|
I have approved this. However if you don't mind, I'd like to propose one more change for this PR. Move frr.conf under /etc/sonic/frr or /etc/sonic/bgp directories (preferably bgp to match the docker name) and if you want to take one more extra step, mount the directory into the docker as rw. |
As talked, unit tests were already included in the commit. |
@nikos-github Let's have that change in a later PR with a design and use case. Also it could/should be done not only for frr. |
|
Ok. I will send out a design next week along with use cases next week. |
…t#2120) #### What I did Fixes sonic-net#2099 Grouping all fields under tables/keys to be added/deleted in 1 JsonChange. **BEFORE** how did we generate moves to try? - Start with the low level differences, low level means differences that has no children - Then extend this low level by different extenders, the most used is the `UpperLevelMoveExtender` - Extended the already extended moves to go upper in level, and do that multiple times until there are no possible moves to extend The diagram below shows: - Generation (low level): ```json {"op":"replace", "path":"/ACL_RULE/SNMP_ACL|RULE_1/SRC_IP", "value": "1.1.1.1/21"} {"op":"remove", "path":"/ACL_RULE/SNMP_ACL|RULE_2/PRIORITY"} {"op":"remove", "path":"/ACL_RULE/SNMP_ACL|RULE_2/SRC_IP"} {"op":"remove", "path":"/ACL_RULE/SNMP_ACL|RULE_2/IP_PROTOCOL"} {"op":"remove", "path":"/ACL_RULE/SNMP_ACL|RULE_2/PACKET_ACTION"} {"op":"add", "path":"/ACL_RULE/SNMP_ACL|RULE_3/PRIORITY", "value": "9997"} {"op":"add", "path":"/ACL_RULE/SNMP_ACL|RULE_3/SRC_IP", "value": "3.3.3.3/20"} {"op":"add", "path":"/ACL_RULE/SNMP_ACL|RULE_3/IP_PROTOCOL", "value": "17"} {"op":"add", "path":"/ACL_RULE/SNMP_ACL|RULE_3/PACKET_ACTION", "value": "ACCEPT"} ``` - Extension - 1st level ```json {"op":"replace", "path":"/ACL_RULE/SNMP_ACL|RULE_1", "value": {"PRIORITY": "9999","SRC_IP": "2.2.2.2/21","SRC_IP": "1.1.1.1/21","IP_PROTOCOL": "17","PACKET_ACTION": "ACCEPT"}} {"op":"remove", "path":"/ACL_RULE/SNMP_ACL|RULE_2"} {"op":"add", "path":"/ACL_RULE/SNMP_ACL|RULE_3", "value": {"PRIORITY": "9997","SRC_IP": "3.3.3.3/20","IP_PROTOCOL": "17","PACKET_ACTION": "ACCEPT"}} ``` - Extension - 2nd level ```json {"op":"replace", "path":"/ACL_RULE", "value": {"SNMP_ACL|RULE_1": {"PRIORITY": "9999","SRC_IP": "1.1.1.1/21","IP_PROTOCOL": "17","PACKET_ACTION": "ACCEPT"},"SNMP_ACL|RULE_3": {"PRIORITY": "9997","SRC_IP": "3.3.3.3/20","IP_PROTOCOL": "17","PACKET_ACTION": "ACCEPT"},"SNMP_ACL|DEFAULT_RULE": {"PRIORITY": "1","ETHER_TYPE": "2048","PACKET_ACTION": "DROP"}}} ``` - Extension - 3rd level ```json {"op":"replace", "path":"", "value": {"ACL_RULE": {"SNMP_ACL|RULE_1": {"PRIORITY": "9999","SRC_IP": "1.1.1.1/21","IP_PROTOCOL": "17","PACKET_ACTION": "ACCEPT"},"SNMP_ACL|RULE_3": {"PRIORITY": "9997","SRC_IP": "3.3.3.3/20","IP_PROTOCOL": "17","PACKET_ACTION": "ACCEPT"},"SNMP_ACL|DEFAULT_RULE": {"PRIORITY": "1","ETHER_TYPE": "2048","PACKET_ACTION": "DROP"}},"ACL_TABLE": {"SNMP_ACL": {"type": "CTRLPLANE","policy_desc": "SNMP_ACL","services": ["SNMP"]}}}} ```  **AFTER** In this PR, we introduce a different kind of generator. The non-extendable generators i.e. generators that their moves do not get extended using the extenders. We added 2 generators: - KeyLevelGenerator: if a whole key to be deleted or added, do that directly. - TableLevelGenerator: if a whole table to be deleted or added, do that directly. Only **enabled** KeyLevelGenerator, to enable **TableLevelGenerator** we have to confirm with Renuka if it is possible to update a whole table at once in the `ChangeApplier` (instead of just keys like it is today). We have to be able to update a table as a whole because within the table there can be internal dependencies between the object, so we have to guarantee all objects are deleted together. For the keys it is guaranteed for the whole key to be updated at once according to the `ChangeApplier`. **Why non-extendable generators?** Calling the non-extendable generators precedes the extendable ones, it is like checking for the low hanging fruits. If we use the same extenders for these new generators we will always go up the config tree. Imaging a move that tries to update a key but fails, then we call the extenders on this move. What will happen is the extenders will go up the config tree to the table level, will first try to replace the table, then try to delete the table. Deleting the table is a problem because it affects multiple more than intended and thus violates the minimum disruption guarantee. We decided to leave the extenders only for the LowLevelGenerator thus making sure we try all possible moves from the lowest levels and go up from there. Another way to look at it is like this: - Non-extendable generators do top-down search: Check tables, keys in this order - Extendable generators and extenders do bottom-up approach: Check fields, keys, tables in this order #### How I did it Modifying the `MoveWrapper.Generate` method to allow for different type of move generators #### How to verify it Unit-test Please check `tests/generic_config_updater/files/patch_sorter_test_success.json` to see how the moved generated got much compacted. #### Previous command output (if the output of a command-line utility has changed) #### New command output (if the output of a command-line utility has changed)
FRR 4.0 integration with SONiC
-- Uses SONiC FRR repo frr/4.0 (which has SONiC support) to build image
-- Makefile changes to make frr4.0 builtable.
-- Updated/Added FRR configuration files
-- Change FRR to use integrated-vtysh-config
-- Add FRR configuration template and tests
To build SONiC images with FRR4.0, simply edit rules/config file and change
routing stack to following:
SONIC_ROUTING_STACK = frr
and then build images as usual.
Note: With some changes, we can support FRR 5.0 as well, basic tests looked good too. I have code ready to switch between FRR versions with SONiC based on single line of rule/config change, but due to the interest at this time and test efforts, this PR is focusing on FRR4.0 support only.
- What I did
FRR 4.0 integration with SONiC
- How I did it
submodule changes, makefile changes, configuration file changes, template changes.
- How to verify it
Change rule/config file:
SONIC_ROUTING_STACK = frr
Build the image and load onto the switch with configuration:
===== FPM test ====
Full regression test on testbed is also in progress...
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)