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

libbpfgo: Improve error message in batch operations #101

Conversation

derekparker
Copy link
Contributor

This patch adds support for improved error messages in batch operations.
It seemed as if the return code would be sufficient, but that seems not
to be the case. It turns out that we actually need the errno information
to further figure out what went wrong and communicate that to the
caller.

For example, for batch read (or batch read and delete) operations a
errno value of ENOENT can indicate that the map is empty, and that is
information that should be propogated back to the caller.

This patch adds support for improved error messages in batch operations.
It seemed as if the return code would be sufficient, but that seems not
to be the case. It turns out that we actually need the errno information
to further figure out what went wrong and communicate that to the
caller.

For example, for batch read (or batch read and delete) operations a
errno value of ENOENT can indicate that the map is empty, and that is
information that should be propogated back to the caller.
@derekparker
Copy link
Contributor Author

@grantseltzer seems you were correct on the last PR when you suggested to check the errno value as well, my bad!

I'm submitting this initial patch as is in an effort to get feedback on if this is a good pattern to follow for the rest of the operations. Once that is settled I will update this patch to add errno checking for the other batch operations where necessary.

@grantseltzer
Copy link
Contributor

I have this included in my PR here: https://github.com/aquasecurity/libbpfgo/pull/100/files#diff-c9fafdb5381ed3544be49e84b94f3e3342e61382a849cb5b4bf674765ad27a20L647

but I don't have a specific check for ENOENT, do you think that should be handled different from different errors? If you agree this is a duplicate PR let's discuss in #100

Just to note, both the actual return from bpf_map_lookup_and_delete_batch and errno afterwards (in the case of an error) will contain the same value, so it isn't necessary to pull the value of errno with the double return value (see linked line above for example)

@derekparker
Copy link
Contributor Author

Going to move the discussion over there.

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.

None yet

2 participants