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

ebpf: Change error types to c_long #84

Closed
wants to merge 2 commits into from

Conversation

vadorovsky
Copy link
Member

All helper functions and context methods are returning c_long as an error. Before this change, aya-template and book examples were adjusting the error type in try_* functionns return type of the program. For example, for XDP programs programs, which return u32, the type returned by try_* functions, was Result<u32, u32>.

This approach is forcing people to write boilerplate code for casting errors from c_long to the expected type, like:

MY_MAP.insert(k, v, 0).map_err(|_| 1u32)?;

This change solves that problem by:

  • Using Result<i32, c_long> as a type for try_* functions for SKB-based programs and then replacing errors with TC_ACT_SHOT or 0 once in the main program function.
  • Using Result<u32, c_long> as a type for try_* functions in try_* funnctions for XDP programs and then replacing errors with XDP_ABORT.
  • Using either Result<u32, c_long> or Result<i32, c_long> as types in try_* functions for all other program types where the return value matters (LSM, sysctl etc.) and replacing the error either with 0 or 1 (depending on which number means "block the action" for particular type of program).
  • Using Result<(), c_long for all other program types where the return value doesn't matter (kprobe, uprobe etc.).

So errors can be handled without casting, like:

MY_MAP.insert(k, v, 0)?;

The other changes included in this commit are:

  • Using core::ffi::c_long instead of aya_cty::c_long, like book examples (they have to be changed too).
  • Fixing return values of cgroup_skb, sockopt and sysctl programs (1 means "allow", 0 means "deny").

All helper functions and context methods are returning `c_long` as an
error. Before this change, aya-template and book examples were adjusting
the error type in `try_*` functionns return type of the program. For
example, for XDP programs programs, which return `u32`, the type
returned by `try_*` functions, was `Result<u32, u32>`.

This approach is forcing people to write boilerplate code for casting
errors from `c_long` to the expected type, like:

```rust
MY_MAP.insert(k, v, 0).map_err(|_| 1u32)?;
```

This change solves that problem by:

* Using `Result<i32, c_long>` as a type for `try_*` functions for
  SKB-based programs and then replacing errors with `TC_ACT_SHOT` or
  `0` once in the main program function.
* Using `Result<u32, c_long>` as a type for `try_*` functions in `try_*`
  funnctions for XDP programs and then replacing errors with `XDP_ABORT`.
* Using either `Result<u32, c_long>` or `Result<i32, c_long>` as types
  in `try_*` functions for all other program types where the return value
  matters (LSM, sysctl etc.) and replacing the error either with `0` or
  `1` (depending on which number means "block the action" for particular
  type of program).
* Using `Result<(), c_long` for all other program types where the
  return value doesn't matter (kprobe, uprobe etc.).

So errors can be handled without casting, like:

```rust
MY_MAP.insert(k, v, 0)?;
```

The other change is fixing return values of cgroup_skb, sockopt and
sysctl programs (1 means "allow", 0 means "deny").

Signed-off-by: Michal Rostecki <[email protected]>
@tamird
Copy link
Member

tamird commented Oct 10, 2024

Looks like a good thing to do. Rebase?

@tamird tamird force-pushed the ebpf-error-types branch 10 times, most recently from 17c1e17 to 485dca4 Compare October 11, 2024 19:02
@vadorovsky
Copy link
Member Author

We are inconsistent in usage of c_long and u32 as error codes.

Furthermore, thiserror now supports no_std, so I think it would be better to just use it for all library errors.

@vadorovsky vadorovsky closed this Nov 11, 2024
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.

2 participants