Fix the EVPN orch may access invalid address #1661
Conversation
gord1306
force-pushed
the
evpnorch_source_vtep
branch
from
bbdf214 to
267243c
Compare
orchagent/vxlanorch.cpp
Outdated
| return false; | ||
| } | ||
|
|
||
| ++it; |
There was a problem hiding this comment.
Please fix alignment in various places. Have this incr line in for-loop itself.
|
@srj102 to review |
There is an private pointer in the EVPN orch which is used save the local vtep. But the local vtep is add/removed in the vxlan tunnel orch, the vxlan tunnel orch did not notify EVPN orch when remove the vtep and cause EVPN orch may access invalid address
gord1306
force-pushed
the
evpnorch_source_vtep
branch
from
267243c to
661e109
Compare
|
retest this please |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -1405,14 +1406,27 @@ bool VxlanTunnelOrch::delOperation(const Request& request) | |||
| return false; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
The sequence as I understand from the PR description is that the VXLAN_TUNNEL entry is deleted first and then the VXLAN_EVPN_NVO from the config DB.
- If the user were to use Click then this will never occur as there are validation checks at Click which will prevent this.
- If the user were not to use Click and instead delete the entries from the CONFIG DB through some other way, then we should put in all the relevant checks in VxlanMgr::doVxlanTunnelDeleteTask instead of the orchagent.
- Besides EVPN NVO checks will also have to be made to delete the VXLAN_TUNNEL_MAP entries in VxlanMgr.
There was a problem hiding this comment.
For the point 1, could you help to provide the information about the validation checks, because I am not sure whether it is in the https://github.com/Azure/sonic-mgmt-common repo
There was a problem hiding this comment.
Please refer to sonic-net/sonic-utilities#870
The changes are in sonic-utilities/config/vxlan.py
Specifically the following snippet.
def del_vxlan(db, vxlan_name):
"""Del VXLAN"""
ctx = click.get_current_context()
vxlan_keys = db.cfgdb.get_keys('VXLAN_EVPN_NVO')
if not vxlan_keys:
vxlan_count = 0
else:
vxlan_count = len(vxlan_keys)
if(vxlan_count > 0):
ctx.fail("Please delete the EVPN NVO configuration.")
There was a problem hiding this comment.
Thanks, that maybe the difference. I am trying to use restapi to apply the config, and it has no validate check logic inside it. Is it better to add the same check logic (APP_DB not CONFIG_DB) in the vxlanmgrd ?
There was a problem hiding this comment.
Does the REST API write to the CONFIG DB or APP DB ? If CONFIG DB then we should put in all the checks in vxlanmgrd. If it is being written to the APP DB directly then yes checks will need to be put in orchagent.
However I think it would be better to keep all the entry points to VXLAN functionality through vxlanmgr i.e. we write to config db
There is an private pointer in the EVPN orch which is used save the local vtep. But the local vtep is add/removed in the vxlan tunnel orch, the vxlan tunnel orch did not notify EVPN orch when remove the vtep and cause EVPN orch may access invalid address
What I did
Verify EVPN related configurations
Why I did it
Test EVPN feature
How I verified it
Delete local vtep first then del nvo, it may cause the nvo access invalid address
Details if related