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

Don't abort program if next_key is NULL in ebpf_map_get_next_key() #3900

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Oct 3, 2024

On Linux it is valid to pass NULL as next_key for queue type maps, since that map type doesn't really have a key. Return EINVAL instead of aborting the process via an assert.

@lmb lmb force-pushed the bpf-compat-next-key-assert branch 2 times, most recently from 70b4642 to 1bfaab6 Compare October 6, 2024 11:29
dthaler
dthaler previously approved these changes Oct 6, 2024
@shpalani
Copy link
Collaborator

shpalani commented Oct 7, 2024

Fix libbpf_test failed test case.

Alan-Jowett
Alan-Jowett previously approved these changes Oct 7, 2024
@@ -1005,6 +1005,11 @@ TEST_CASE("libbpf map", "[libbpf]")
REQUIRE(result < 0);
REQUIRE(errno == EINVAL);

// next_key is NULL.
result = bpf_map_get_next_key(map_fd, NULL, &value);
Copy link
Contributor

Choose a reason for hiding this comment

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

question: the second argument to bpf_map_get_next_key() is previous_key, which is being passed as NULL here, whereas the comment says next_key is NULL. The above code change is also for handling next_key being NULL. Is this expected?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, good catch!

On Linux it is valid to pass NULL as next_key for queue type maps, since
that map type doesn't really have a key. Return EINVAL instead of aborting
the process via an assert.
@lmb lmb dismissed stale reviews from Alan-Jowett and dthaler via ecf7ea9 October 17, 2024 09:12
@lmb lmb force-pushed the bpf-compat-next-key-assert branch from 1bfaab6 to ecf7ea9 Compare October 17, 2024 09:12
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.

5 participants