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

Add lock to prevent race conditions on self._multipart_replies_flows #28

Merged
merged 3 commits into from
Nov 10, 2021

Conversation

italovalcy
Copy link

Fixes #26

Description of the change

The variable _multipart_replies_flows holds a dictionary of received flows per switch when using OpenFlow 1.3 multipart messages. The dictionary is updated upon receiving the KytosEvent kytos/of_core.v0x04.messages.in.ofpt_multipart_reply and can be accessible from many different threads at the same time. This situation may lead to race conditions when two or more threads handling the flows for the same switch update the all_flows at the same time, resulting on inconsistent/missing data.

This PR adds a threading.Lock mechanism to prevent race conditions at this variable.

Release notes

  • Added mechanism to prevent race conditions at OF1.3 multipart reply handler

@italovalcy italovalcy marked this pull request as ready for review October 22, 2021 19:20
Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Great fix, one fewer potential race condition.

@italovalcy looks like the multipart handler of port stats also has a similar race condition vulnerability, should we map this in another issue to be fixed in another opportunity?

@italovalcy
Copy link
Author

LGTM. Great fix, one fewer potential race condition.

@italovalcy looks like the multipart handler of port stats also has a similar race condition vulnerability, should we map this in another issue to be fixed in another opportunity?

@viniarck yes, you are right. The tests I've been running show that the port stats message usually doesn't get fragmented at the OpenVSwitch nor Noviflow switches. However, I think we should map this to be handled in the future. I've just created issue #32

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.

self._multipart_replies_flows is vulnerable to race conditions which can lead to data loss
3 participants