Support for in-band-mgmt via management VRF #1707
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@prsunny Please take a look at the in-band-mgmt via mgmt VRF support changes. |
|
@prsunny Can you please review this PR? |
prsunny
reviewed
Apr 23, 2021
cfgmgr/vrfmgr.cpp
Outdated
| if (fvValue(i) == "true") { | ||
| mgmt_vrf_enabled = true; | ||
| } else { | ||
| op = DEL_COMMAND; |
There was a problem hiding this comment.
Why are we overwriting the op command here?. This operation is expected from the notification
There was a problem hiding this comment.
We should delete the VRF (Virtual router) from HW when no in-band-intf is part of the mgmt VRF i.e when in_band_mgmt_enabled is set false, we can set the op as DEL_COMMAND to delete the VRF.
orchagent/intfsorch.cpp
Outdated
| @@ -389,7 +403,8 @@ set<IpPrefix> IntfsOrch:: getSubnetRoutes() | |||
| return subnet_routes; | |||
| } | |||
|
|
|||
| bool IntfsOrch::setIntf(const string& alias, sai_object_id_t vrf_id, const IpPrefix *ip_prefix, const bool adminUp, const uint32_t mtu) | |||
| bool IntfsOrch::setIntf(const string& alias, const string& vrf_name, sai_object_id_t vrf_id, const IpPrefix *ip_prefix, | |||
There was a problem hiding this comment.
Lets discuss this - Why we need to pass the vrf_name to this function.
There was a problem hiding this comment.
This helps to ignore the HW programming of the nbr learnt thru in-band-intf (attached to mgmt VRF).
venkatmahalingam
force-pushed
the
master
branch
from
d106b4c
to
d8b8ca9
Compare
EdenGri
pushed a commit
to EdenGri/sonic-swss
that referenced
this pull request
Feb 28, 2022
…commands executed from linecard (sonic-net#1707) * [multi-asic][cli][chassis-db] Avoid connecting to chassis db Currently, for all the cli commands, we connect to all databases mentioned in the database_config.json. The database_config.json also includes the databases from chassis redis server from supervisor card. It is unneccessary to connect to databases from chassis redis server when cli commands are executed form linecard. But we need to allow connection to chassis databases when the cli commands are executed from supervisor card. The changes in this PR fixes this problem. The constructor of Db() class which is instantiated for every CLI command execution is changed to skip chassis databases from the list of collected databases if the card is not supervisor card. Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Venkatesan Mahalingam venkatesan_mahalinga@dell.com
Please refer HLD and swss-common pull requests for more information.
What I did
Support for in-band-mgmt via management VRF
Why I did it
Other than eth0, mgmt VRF should have in-band L3 interfaces as well.
How I verified it
Verified the functionality using sonic-vs testcases
Details if related