-
Notifications
You must be signed in to change notification settings - Fork 649
[vnetorch] Prevent route deletion in default VRF upon conflicting route creation in non default VNET #4029
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: master
Are you sure you want to change the base?
Conversation
…on default vnet Signed-off-by: anish-n <[email protected]>
Signed-off-by: anish-n <[email protected]>
Signed-off-by: anish-n <[email protected]>
Signed-off-by: anish-n <[email protected]>
Signed-off-by: anish-n <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Pull request overview
This PR fixes a bug where routes in the default VRF were incorrectly deleted when conflicting routes were created in non-default VNETs. The fix modifies isRouteExists() to check for route existence in a specific VRF rather than always checking the default VRF.
- Modified
isRouteExists()to accept avrf_idparameter for VRF-specific route lookups - Fixed return value in
isRouteExists()fromtruetofalsewhen route table doesn't exist - Updated all call sites in vnetorch to pass the appropriate VRF ID
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| orchagent/routeorch.h | Updated function signature to accept VRF ID parameter |
| orchagent/routeorch.cpp | Modified isRouteExists() to check routes in specified VRF and fixed return value when route table not found |
| orchagent/vnetorch.cpp | Updated three call sites to pass VRF ID to isRouteExists() |
| tests/vnet_lib.py | Added helper functions to verify default VRF routes and VNET route deletion, but contains bugs |
| tests/test_vnet2.py | Added new test and updated existing tests to validate conflicting route behavior with non-default VNETs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: anish-n <[email protected]>
be974c2 to
4e3d162
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
What I did
Modified vnetorch to not delete conflicting route space in default vrf if the vnet scope is non default
Why I did it
To support conflicting IP spaces
How I verified it
SWSS UT which shows that conflicting IP space is no longer deleted, and manual testing on device which shows this as well
Details if related
ASIC DB route deleteion issue can be seen below(which is fixed by this PR):
Route entry on switch:
Route entry in ASIC DB:
Add conflicting route entry in VNET:
Default route is deleted from default VRF: