Prevent route deletion in default vrf upon creating tunnel route #3995
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 3995 in repo sonic-net/sonic-swss |
|
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where route deletion operations in VNetOrch were incorrectly targeting routes in the default VRF instead of the intended VNet's VRF. The fix ensures that route existence checks and deletions are scoped to the correct VRF by passing the VRF ID as a parameter.
Key Changes:
- Modified
isRouteExists()to accept a VRF ID parameter instead of defaulting to the global virtual router ID - Updated all call sites in VNetOrch to pass the appropriate VRF ID when checking for route existence
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| orchagent/routeorch.h | Updated function signature to include vrf_id parameter |
| orchagent/routeorch.cpp | Removed hardcoded default VRF ID and used parameter instead |
| orchagent/vnetorch.cpp | Updated all call sites to pass VRF ID to isRouteExists() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| sai_object_id_t vr_id = vrf_obj->getVRidIngress(); | ||
| if(gRouteOrch && gRouteOrch->isRouteExists(vr_id, ipPrefixsubnet)) |
There was a problem hiding this comment.
[nitpick] The variable vr_id is declared immediately before use in the conditional. Consider moving this declaration to the top of the relevant scope block for better readability and consistency with the pattern used at line 1195, where vr_id appears to be available from the surrounding context.
anish-n
force-pushed
the
vnetRtTunFixDfltVrfDel
branch
from
40a74b2 to
fb55c5c
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 participants
…on default vnet
What I did
Why I did it
How I verified it
Details if related