-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
VRF HLD version 1.1 #408
VRF HLD version 1.1 #408
Conversation
Removed test plan since its tracked as separate document
In the VRF diagram, in each VRF, site-1 and site-2 are repeated. Should they be site-3 and site-4? Or it means a multi-homing configuration? Please clarify. |
Removed test plan since its tracked as separate document Addressing comments
Removed test plan since its tracked as separate document Addressing comments
Prefix lists within route maps are used to match route prefixes in source VRF and various action can be applied on these matching prefixes. | ||
If route map action is permit, these routes will be installed into destination VRF. | ||
|
||
Leaked routes will be automatically deleted when corresponding routes are deleted in source VRF. |
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.
Please provide and describe the use case for VRF route leak similar to multiple VRF deployment use-case?
remove the version number in the file name. |
@@ -238,7 +277,7 @@ Add vrf-binding information in config_db.json file. | |||
|
|||
With this approach, there is no redundant vrf info configured with an interface where multiple IP addresses are configured. | |||
|
|||
Logically IP address configuration must be processed after interface binding to vrf is processed. In intfmgrd/intfOrch process intf-bind-vrf event must be handled before IP address event. So interface-name entry in config_db.json is necessary even user doesn't use VRF feature. e.g. `"Ethernet2":{}` in the above example configuration. For version upgrade compatibility we need to add a script, this script will convert old config_db.json to new config_db.json at bootup, then the new config_db.json would contain the interface-name entry for interfaces associated in the global VRF table. | |||
Logically IP address configuration must be processed after interface binding to vrf is processed. In intfmgrd/intfOrch process intf-bind-vrf event must be handled before IP address event. So interface-name entry in config_db.json is necessary even though user doesn't use VRF feature. e.g. `"Ethernet2":{}` in the above example configuration. For version upgrade compatibility we need to add a script, this script will convert old config_db.json to new config_db.json at bootup automatically, then the new config_db.json would contain the interface-name entry for interfaces associated in the global VRF table. |
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.
@preetham-singh @ben-gale @prsunny Regarding the comment I made in the community meeting last Tuesday on changing config_db.json when it comes to interfaces belonging in the global table and also mentioned in here https://github.com/Azure/SONiC/blob/master/doc/sonic-vrf-hld-v1.0.md
"Ethernet2":{}, // it means this interface belongs to global vrf. It is necessary even user doesnt use vrf.
In order to preserve backwards compatibility with existing config_db.json files and deployments where the config_db.json is checked for deviations, when saving the contents of config db from redis back into config_db.json, default entries like the the entry above can be removed. Basically it shouldn't be a requirement for the user to either provide that in their config_db.json or have this saved back.
Such approach is being followed already by some BGP cmds I introduced and implemented.
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.
Based on discussion on this topic in the Sonic user group, it was agreed upon that in certain cases where the feature demands it, it is expected to have a config_db
change as long as there is a provision to migrate the old config_db.json
to the new image version to ensure backward compatibility. This means, after a save of the migrated config_db
, the saved json file could be different from the old one and the expectation is that the monitoring application must accommodate to the new schema defined in the migrated image version.
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.
Prince - what about the reverse direction (i.e. downgrade)? I assume the user is expected to manually 'repair' their configuration?
Is there really no way that this could be handled differently?
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.
For the downgrade, the user has to manually repair. In general, it is not an automated support. W.r.t VRF, if you are asking whether it could handled differently, yes it is possible but with the complexities of supporting move feature. Moreover, some of the code to support the config_db approach like the db_migrator, interface changes etc were already in-place for VNet feature. That's why we want to take this path.
* Updating VRF HLD Version 1.0 * VRF HLD v1.1 * Addressing comments from Nephos: Removed test plan since its tracked as separate document * Addressing comments from Nephos: Removed test plan since its tracked as separate document Addressing comments * Addressing comments from Nephos: Removed test plan since its tracked as separate document Addressing comments * Addressing few review comments from VRF HLD review * Rename VRF HLD file to not have version numbre in file name
Updating VRF HLD to create version 1.1