-
Notifications
You must be signed in to change notification settings - Fork 553
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
[vxlanorch] Fix Logic of Vxlan tunnel removal #995
Conversation
…e tunnel_id and tunnel_term_id. Signed-off-by: sundandan <[email protected]>
orchagent/vxlanorch.cpp
Outdated
@@ -295,10 +295,17 @@ create_tunnel( | |||
void | |||
remove_tunnel(sai_object_id_t tunnel_id) | |||
{ | |||
sai_status_t status = sai_tunnel_api->remove_tunnel(tunnel_id); | |||
if (status != SAI_STATUS_SUCCESS) | |||
if(tunnel_id != SAI_NULL_OBJECT_ID) |
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.
Add a space between if
and (
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.
Add a space between
if
and(
OK
{ | ||
sai_status_t status = sai_tunnel_api->remove_tunnel(tunnel_id); | ||
if (status != SAI_STATUS_SUCCESS) | ||
{ |
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.
Could you add an error message here?
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.
Could you add an error message here?
The "runtime_error" in the next line has thrown the error message.
Signed-off-by: sundandan <[email protected]>
retest this please |
orchagent/vxlanorch.cpp
Outdated
{ | ||
sai_status_t status = sai_tunnel_api->remove_tunnel_term_table_entry(term_table_id); | ||
if (status != SAI_STATUS_SUCCESS) | ||
{ | ||
throw std::runtime_error("Can't remove a tunnel term table object"); |
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 add 4 spaces here
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.
I'm sorry, thank you!
Changes looks good to me. Minor comments! |
Signed-off-by: sundandan <[email protected]>
retest this please |
As the vxlan tunnel_id and tunnel_term_id are not created until the map entrys are added, in case of configuring the vxlan tunnel without map entry, and then it is invalid to remove it without validity checking. Signed-off-by: sundandan <[email protected]>
sonic-net#995) Root group name was changed from `cli` to `sonic_installer` in sonic-net/sonic-utilities#983. However, `verify-next-image` subcommand was being added in an already-open PR sonic-net/sonic-utilities#979 at the time. It did not get updated before merge. This aligns it with the new group name.
…nic-net#995) * Put the downloaded swss common artifact in a separated directory while build sairedis target. * The downloaded swss common artifact is in a sub-directory of sairedis artifact, then it avoids overriding the previous swss common packages.
What I did
Add the validity checking before we call sai_api to remove tunnel_id and tunnel_term_id.
Why I did it
As the vxlan tunnel_id and tunnel_term_id are not created until the map entrys are added, in case of configuring the vxlan tunnel without map entry, and then it is invalid to remove it without validity checking.
How I verified it
Applied Vxlan tunnel configure without the map entry and then remove the tunnel, there are no errors.
Details if related