Skip to content

datapath/linux: protect against concurrent access in NodeValidateImplementation#12461

Merged
brb merged 1 commit intomasterfrom
pr/tklauser/datapath-neigh-by-node-concurrent
Jul 10, 2020
Merged

datapath/linux: protect against concurrent access in NodeValidateImplementation#12461
brb merged 1 commit intomasterfrom
pr/tklauser/datapath-neigh-by-node-concurrent

Conversation

@tklauser
Copy link
Copy Markdown
Member

@tklauser tklauser commented Jul 8, 2020

The linuxNodeHandler.neighByNode map is also protected by
linuxNodeHandler.mutex in all other code paths. Protect access to it in
`(*linuxNodeHandler).NodeValidateImplementation as well.

Fixes #12460

Fixes: 6c06c51 ("node: Remove permanent ARP entry when remote node is deleted")

@tklauser tklauser added release-note/bug This PR fixes an issue in a previous release of Cilium. needs-backport/1.8 labels Jul 8, 2020
@tklauser tklauser requested a review from a team July 8, 2020 17:52
@tklauser
Copy link
Copy Markdown
Member Author

tklauser commented Jul 8, 2020

test-me-please

…ementation

The linuxNodeHandler.neighByNode map is also protected by
linuxNodeHandler.mutex in all other code paths. Protect access to it in
(*linuxNodeHandler).NodeValidateImplementation as well.

Fixes #12460

Fixes: 6c06c51 ("node: Remove permanent ARP entry when remote node is deleted")
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser force-pushed the pr/tklauser/datapath-neigh-by-node-concurrent branch from 9d14a39 to 4b1cf84 Compare July 8, 2020 17:58
@tklauser tklauser requested a review from brb July 8, 2020 17:59
@tklauser
Copy link
Copy Markdown
Member Author

tklauser commented Jul 8, 2020

test-me-please

@coveralls
Copy link
Copy Markdown

coveralls commented Jul 8, 2020

Coverage Status

Coverage decreased (-0.006%) to 36.948% when pulling 4b1cf84 on pr/tklauser/datapath-neigh-by-node-concurrent into 53c17cd on master.

Comment thread pkg/datapath/linux/node.go
tklauser added a commit that referenced this pull request Jul 9, 2020
Follow-up for #12461 per review comment
#12461 (comment)

Suggested-by: Tam Mach <sayboras@yahoo.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
aanm pushed a commit that referenced this pull request Jul 9, 2020
Follow-up for #12461 per review comment
#12461 (comment)

Suggested-by: Tam Mach <sayboras@yahoo.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Copy link
Copy Markdown
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks! I assume you have checked all other paths where nodeUpdate() is called?

@maintainer-s-little-helper maintainer-s-little-helper Bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 10, 2020
@brb brb merged commit d88d5f9 into master Jul 10, 2020
@brb brb deleted the pr/tklauser/datapath-neigh-by-node-concurrent branch July 10, 2020 10:06
@tklauser
Copy link
Copy Markdown
Member Author

Thanks! I assume you have checked all other paths where nodeUpdate() is called?

Yes, all other code paths calling nodeUpdate already hold the mutex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

agent: fatal error: concurrent map writes

7 participants