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

lbmap: fix deletion and recreation logic for maglev maps #16850

Merged
merged 1 commit into from Jul 19, 2021

Conversation

jibi
Copy link
Member

@jibi jibi commented Jul 12, 2021

When a maglev BPF map is initialized, before creating it we check if it
already exists, and if its inner map size matches the desired maglev
table size ("M" parameter), so that we can delete and recreate it in
case of a mismatch.

The lbmap.MaglevOuterMapTableSize function is reponsible for reporting
to the caller if the map already exists and its inner map size.

Currently, if the map exists but its empty, MaglevOuterMapTableSize will
incorrectly return false (i.e. "map does not exist"), preventing
lbmap.deleteMapIfMNotMatch from deleting it in case of a mismatch with
the M parameter.

This commit fixes this logic.

Fixes: #16844
Fixes: 879f9eb

@jibi jibi added release-note/bug This PR fixes an issue in a previous release of Cilium. sig/loadbalancing labels Jul 12, 2021
@jibi jibi requested review from christarazi and a team July 12, 2021 09:52
@jibi jibi requested a review from kkourt July 12, 2021 09:52
@jibi
Copy link
Member Author

jibi commented Jul 12, 2021

Manually run the privileged unit tests:

➜  cilium git:(pr/jibi/fix-16844) sudo SKIP_VET=true TESTPKGS=pkg/maps/lbmap make tests-privileged
..
=== RUN   Test
START: maglev_test.go:43: MaglevSuite.SetUpSuite
PASS: maglev_test.go:43: MaglevSuite.SetUpSuite	0.000s

START: maglev_test.go:81: MaglevSuite.TestInitMaps
PASS: maglev_test.go:81: MaglevSuite.TestInitMaps	0.191s

OK: 1 passed
--- PASS: Test (0.19s)
PASS

When a maglev BPF map is initialized, before creating it we check if it
already exists, and if its inner map size matches the desired maglev
table size ("M" parameter), so that we can delete and recreate it in
case of a mismatch.

The lbmap.MaglevOuterMapTableSize function is reponsible for reporting
to the caller if the map already exists and its inner map size.

Currently, if the map exists but its empty, MaglevOuterMapTableSize will
incorrectly return false (i.e. "map does not exist"), preventing
lbmap.deleteMapIfMNotMatch from deleting it in case of a mismatch with
the M parameter.

This commit fixes this logic.

Fixes: #16844
Fixes: 879f9eb
Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
@jibi
Copy link
Member Author

jibi commented Jul 13, 2021

test-me-please
(rebased)

@sugangli
Copy link
Contributor

Hi, may I know the status of this PR? We are currently blocked on this as we have some changes needed to be sync with the gke cilium.

@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 16, 2021
@pchaigno
Copy link
Member

I've removed the merge-freeze label given CI is now a lot more stable and tests are passing.

@aanm aanm merged commit c021445 into master Jul 19, 2021
@aanm aanm deleted the pr/jibi/fix-16844 branch July 19, 2021 01:01
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.

CI: Maglev unit test failing on newer kernels with: update failed: invalid argument
6 participants