Skip to content
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

Merged
merged 3 commits into from
Jul 31, 2019
Merged

[vxlanorch] Fix Logic of Vxlan tunnel removal #995

merged 3 commits into from
Jul 31, 2019

Conversation

sdddean
Copy link
Contributor

@sdddean sdddean commented Jul 26, 2019

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

…e tunnel_id and tunnel_term_id.

Signed-off-by: sundandan <[email protected]>
@@ -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)
Copy link
Contributor

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 (

Copy link
Contributor Author

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)
{
Copy link
Contributor

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?

Copy link
Contributor Author

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.

orchagent/vxlanorch.cpp Outdated Show resolved Hide resolved
@lguohan
Copy link
Contributor

lguohan commented Jul 27, 2019

retest this please

{
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");
Copy link
Collaborator

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

Copy link
Contributor Author

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!

@prsunny
Copy link
Collaborator

prsunny commented Jul 30, 2019

Changes looks good to me. Minor comments!

@lguohan
Copy link
Contributor

lguohan commented Jul 31, 2019

retest this please

@lguohan lguohan merged commit a5e6bea into sonic-net:master Jul 31, 2019
tonytitus pushed a commit to tonytitus/sonic-swss that referenced this pull request Jul 31, 2019
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]>
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
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.
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants