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

[macsecorch]: Fix deleting MACsec bug #2127

Merged
merged 2 commits into from
Feb 18, 2022

Conversation

Pterosaur
Copy link
Contributor

@Pterosaur Pterosaur commented Jan 30, 2022

Signed-off-by: Ze Gan ganze718@gmail.com

What I did
Fix a segment fault in MACsec orchagent to record the next iterator before deleting the corresponding MACsec object.

Why I did it
The segment fault will happen if to delete the MACsec Port or MACsec SC that have MACsec SC(s) or MACsec SA(s), Because the original implementation used the for loop to iterate all MACsec sub objects, but the MACsec sub object will be delete on each iteration so that the removed iterator cannot go to the next one.

How I verified it
Use appl_db_del(appl_db, "MACSEC_PORT_TABLE", ifname) to delete a MACsec port that has MACsec SC, The segment fault shouldn't happen.
Details if related

Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur changed the title Fix delete MACsec bug [macsecorch]: Fix delete MACsec bug Jan 30, 2022
@Pterosaur Pterosaur marked this pull request as ready for review January 30, 2022 10:57
@Pterosaur Pterosaur changed the title [macsecorch]: Fix delete MACsec bug [macsecorch]: Fix deleting MACsec bug Jan 30, 2022
@Pterosaur
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lguohan
Copy link
Contributor

lguohan commented Feb 3, 2022

@Pterosaur , is the vstest failure related to this pr or not related, if it is not, can you ask @prsunny to check?

@Pterosaur
Copy link
Contributor Author

@Pterosaur , is the vstest failure related to this pr or not related, if it is not, can you ask @prsunny to check?

Sure, I have connected Prince.

@Pterosaur
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@judyjoseph
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Pterosaur
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Pterosaur Pterosaur merged commit 5a651d0 into sonic-net:master Feb 18, 2022
@Pterosaur Pterosaur deleted the fix_delete_macsec_bug branch February 18, 2022 04:21
dprital pushed a commit to dprital/sonic-swss that referenced this pull request May 8, 2022
* Fix delete MACsec bug

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Fix

Signed-off-by: Ze Gan <ganze718@gmail.com>
preetham-singh pushed a commit to preetham-singh/sonic-swss that referenced this pull request Aug 6, 2022
* Fix delete MACsec bug

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Fix

Signed-off-by: Ze Gan <ganze718@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants