Add support for stack argument

[liyan-ah]

Hi, I'm new in rust and eBPF. Here I want to add sarg support in aya, and my code is in this PR. The code builds okay in my environment. But when I use it in uprobe, and try to print message with aya-log-ebpf, the following error comes:
bpf code:

fn try_bpf_agent(ctx: ProbeContext) -> Result<u32, u32> {
    info!(&ctx, "function getaddrinfo called by user");
    ...
}

when I build:

error[E0277]: the trait bound `ProbeContext: aya_bpf::BpfContext` is not satisfied
  --> src/main.rs:20:11
   |
20 |     info!(&ctx, "function getaddrinfo called by user");
   |     ------^^^^----------------------------------------
   |     |     |
   |     |     the trait `aya_bpf::BpfContext` is not implemented for `ProbeContext`
   |     required by a bound introduced by this call
   |
   = help: the following other types implement trait `aya_bpf::BpfContext`:
             aya_bpf::programs::device::DeviceContext
             aya_bpf::programs::fentry::FEntryContext
             aya_bpf::programs::fexit::FExitContext
             aya_bpf::programs::lsm::LsmContext
             aya_bpf::programs::perf_event::PerfEventContext
             aya_bpf::programs::probe::ProbeContext
             aya_bpf::programs::raw_tracepoint::RawTracePointContext
             aya_bpf::programs::sk_buff::SkBuffContext
           and 11 others
note: required by a bound in `aya_bpf::maps::perf::perf_event_byte_array::PerfEventByteArray::output`
  --> ~/.cargo/git/checkouts/aya-c55fbc69175ac116/76d35d1/bpf/aya-bpf/src/maps/perf/perf_event_byte_array.rs:50:22
   |
50 |     pub fn output<C: BpfContext>(&self, ctx: &C, data: &[u8], flags: u32) {
   |                      ^^^^^^^^^^ required by this bound in `PerfEventByteArray::output`

Any one with any ideas? Much thanks.

[netlify]

❌ Deploy Preview for aya-rs-docs failed.

Built without sensitive environment variables

Name	Link
🔨 Latest commit	3241e5a
🔍 Latest deploy log	https://app.netlify.com/sites/aya-rs-docs/deploys/64eee68c905adc0008220f63

[alessandrod]

can you paste the whole compiler output? it looks like you might have two different aya_bpf versions in your project

[liyan-ah]

can you paste the whole compiler output? it looks like you might have two different aya_bpf versions in your project

Oh, yes. I see.
Thanks a lot.

[liyan-ah]

Add support for stack argument.

[liyan-ah]

@alessandrod
Hi, I fixed the issue. And this stack_argument solution works in my test environment, in x86_64 platform.
Please check these changes. Any advice would be appreciated.

[liyan-ah]

@tamird @vadorovsky
Please check these files. Any advice would be appreciated.

[tamird]

Please add a test.

[tamird]

Instead of .ok() can we just return the Result?

[liyan-ah]

I just follow from_argument. It seems that .ok() is more readable.

[tamird]

It's discarding information.

[liyan-ah]

It's discarding information.

I'm confused. I know bpf_probe_read returns Result, and if from_stack_argument returns Option, Err info may be lost.
But the existing from_argument just return Option. Should it be better to return Result to save information or return Option to adapt the style ?

[tamird]

I'm not sure why the existing functions return Option. In my opinion they should also return Result. Perhaps @alessandrod knows something about the return value that I don't.

[alessandrod]

Yeah, bpf_probe_read returns only 0 or -EFAULT, so it's not really discarding anything.

But I'll say that this from_argument stuff is very involved - in the existing code I mean, not just in the PR, and needs to be reworked. Sadly I forgot how I'd rework it it's been a while :P

[tamird]

if bpf_probe_read only returns one of two values, should we wrap it and do the Result -> Option conversion there instead of everywhere it's used?

[alessandrod]

Maybe, but I'm inclined to say no.

My idea is we want to offer different levels of abstractions:

• Generated bindings that give you the original (unsafe) helpers API. This is useful when you want to call things for which we don't provide safe or higher level APIs yet.
• Safe wrappers for the generated helpers. These are still low level, but safe. We don't control these APIs, we just wrap them, and they're unstable so they might change. bpf_probe_read might start returning something else at some point (unlikely for the bpf_probe_read case specifically but possible for a more complex helper). Additionally, say I want to write an ebpf program without using any high level bits of aya-ebpf, I should be able to do so, and I should be able to build my abstractions on top of these foundational helpers.
• We have our own API. ProbeContext::arg is an example. It internally uses bpf_probe_read, but that's an implementation detail. If bpf_probe_read starts returning something else, it's conceivable that we won't care at this level and we'll still return None; or if we do decide to return the inner error, we can deprecated the API and have ::argument or something, but we won't be as weird as having to deprecate bpf_probe_read which technically we don't control we're just wrapping.

[tamird]

Yeah, I wasn't suggesting changing the behavior of bpf_probe_read, but as it stands we have a lot of .ok() calls that I think we could centralize.

[liyan-ah]

Please add a test.

Any advice about the test?
A function with more than 6 arguments is needed so that from_stack_argument can be used. As far as I know, very few system functions can be used.

[tamird]

You can write a test that uses a uprobe. See e.g. https://github.com/aya-rs/aya/blob/main/test/integration-test/src/tests/log.rs or the integration tests in #629.

[liyan-ah]

https://github.com/aya-rs/aya/blob/main/test/integration-test/src/tests/log.rs

Ok.

[mergify]

@liyan-ah, this pull request is now in conflict and requires a rebase.

[liyan-ah]

moved to #773.
this pr closed.