Skip to content

map: unmarshal values for partial batch #1742

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

Merged
merged 1 commit into from
Apr 16, 2025
Merged

map: unmarshal values for partial batch #1742

merged 1 commit into from
Apr 16, 2025

Conversation

aibor
Copy link
Contributor

@aibor aibor commented Apr 2, 2025

In case of a partial batch the values are not unmarshalled in BatchLookup and BatchLookupDelete for non-perCPU maps.

This commit fixes it by adapting the error handling from batchLookupPerCPU.

Fixes #1741

@aibor aibor requested a review from a team as a code owner April 2, 2025 21:26
@lmb
Copy link
Collaborator

lmb commented Apr 16, 2025

Oof, thanks. Do you think you can extend the unit tests to cover this case?

@aibor
Copy link
Contributor Author

aibor commented Apr 16, 2025

I could add a test that uses a binary.Unmarshaler implementation of a slice type. Kinda like in my reproducer but without the global counter variable. Might be a bit hacky, but should be fine for a test, I guess?

Do you think this should be a new separate test? Or do you have an existing test function in mind that should be extended with a test case for this issue?

@aibor aibor force-pushed the fix-1741 branch 2 times, most recently from d7a28ae to e45940d Compare April 16, 2025 11:37
@aibor
Copy link
Contributor Author

aibor commented Apr 16, 2025

I have added a new test function that tests key and value unmarshaling with a custom binary.Unmarshaler. Without the fix it fails the unmarshaled values assertion:

--- FAIL: TestMapBatchLookupCustomUnmarshaler (0.00s)
    map_test.go:1040:
        error:
          values are not deep equal
        diff (-want +got):
            []uint8{
            	0x03,
            	0x04,
          - 	0x05,
          + 	0x03,
            }
        got:
          []uint8("\x03\x04\x03")
        want:
          []uint8("\x03\x04\x05")

In case of a partial batch the values are not unmarshalled in
BatchLookup and BatchLookupDelete for non-perCPU maps.

This commit fixes it by adapting the error handling from
batchLookupPerCPU.

Fixes cilium#1741

Signed-off-by: Tobias Böhm <[email protected]>
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@lmb lmb merged commit 130bbbb into cilium:main Apr 16, 2025
28 of 31 checks passed
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.

Value unmarshalling skipped for partial batch in BatchLookup
2 participants