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

feat: Add method Run to BPFProg #440

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Conversation

geyslan
Copy link
Member

@geyslan geyslan commented Jun 4, 2024

feat: Add method Run to BPFProg

Run method is used to load and run the BPF program without attaching it
to any hook. This is useful for testing the BPF program.

It makes use of the bpf(BPF_PROG_TEST_RUN) via bpf_prog_test_run_opts().

The prog-run selftest is added to test the Run method. More examples
can be found in the linux kernel source code:
tools/testing/selftests/bpf/prog_tests.

Original code #428 was authored by sc07kvm, and this commiter have only
made cosmetics adjustments.

Run method is used to load and run the BPF program without attaching it
to any hook. This is useful for testing the BPF program.

It makes use of the bpf(BPF_PROG_TEST_RUN) via bpf_prog_test_run_opts().

The prog-run selftest is added to test the Run method. More examples
can be found in the linux kernel source code:
tools/testing/selftests/bpf/prog_tests.

Original code aquasecurity#428 was authored by sc07kvm, and this commiter have only
made cosmetics adjustments.

Co-authored-by: Geyslan Gregório <[email protected]>
@geyslan geyslan added the feature New feature or request label Jun 4, 2024
@geyslan geyslan self-assigned this Jun 4, 2024
@geyslan geyslan marked this pull request as ready for review June 4, 2024 16:51
Copy link
Contributor

@NDStrahilevitz NDStrahilevitz left a comment

Choose a reason for hiding this comment

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

LGTM, a nice addition indeed.

// t.Errorf("result = %d; want 1", opts.RetVal)
// }
// }
func (p *BPFProg) Run(opts *RunOpts) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be nice to add an error here when the program type isn't compatible (rather than relying on an internal failure).

Copy link
Member Author

Choose a reason for hiding this comment

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

It will be dependent on kernel version support, e.g.:

The output for ENOTSUPP is failed to run program: errno 524. https://elixir.bootlin.com/linux/v6.9.3/source/include/linux/errno.h#L27

.Error() doesn't find a correct entry, since "not supported" for syscall is ENOTSUP 95. https://github.com/golang/sys/blob/master/unix/zerrors_linux_amd64.go#L614

Copy link
Member Author

@geyslan geyslan Jun 6, 2024

Choose a reason for hiding this comment

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

WDYT? If you agree, we must check all others returns for this ENOTSUPP.

failed to run program: operation not supported

diff --git a/prog.go b/prog.go
index e47c8ec..56ea57e 100644
--- a/prog.go
+++ b/prog.go
@@ -779,7 +779,11 @@ func (p *BPFProg) Run(opts *RunOpts) error {
 
        retC := C.bpf_prog_test_run_opts(C.int(p.FileDescriptor()), optsC)
        if retC < 0 {
-               return fmt.Errorf("failed to run program: %w", syscall.Errno(-retC))
+               errno := syscall.Errno(-retC)
+               if errno == 524 { // ENOTSUPP https://elixir.bootlin.com/linux/v6.9.3/source/include/linux/errno.h#L27
+                       errno = syscall.ENOTSUP
+               }
+               return fmt.Errorf("failed to run program: %s", errno)
        }
 
        // update runOpts with the values from the kernel and libbpf

Copy link
Member Author

Choose a reason for hiding this comment

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

Anyway, merging. If decided for it, we can tackle it in a bunch.

@geyslan geyslan merged commit 466e180 into aquasecurity:main Jun 6, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants