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

Bump libbpfgo + add regression tests #381

Merged
merged 2 commits into from May 4, 2022

Conversation

javierhonduco
Copy link
Contributor

@javierhonduco javierhonduco commented May 3, 2022

Bump libbpfgo:

Ran:

$ go get github.com/aquasecurity/libbpfgo@80f44303d40f3e65702de30d4976ada883e79fe2

Changes from the version we have until this pinned revision: aquasecurity/libbpfgo@v0.2.5-libbpf-0.7.0...80f4430.

This fixes a number of issues that were affecting us, including:

Add libbpfgo map tests:

To ensure that the libbpfgo invariants hold true. Adding these tests
will also help us with library upgrades, as well as configuration
changes in the way libbpf deals with errors, etc.

Test plan:

$ make test_profiler
sudo -E GOOS=linux GOARCH=arm64 CC=clang CGO_CFLAGS="-I /home/vagrant/work/parca-agent/dist/libbpf/usr/include" CGO_LDFLAGS="/home/vagrant/work/parca-agent/dist/libbpf/libbpf.a" go test -v github.com/parca-dev/parca-agent/pkg/profiler
=== RUN   TestDeleteNonExistentKeyReturnsEnoent
--- PASS: TestDeleteNonExistentKeyReturnsEnoent (0.00s)
=== RUN   TestDeleteExistentKey
--- PASS: TestDeleteExistentKey (0.00s)
=== RUN   TestGetValueAndDeleteBatchWithEmptyMap
--- PASS: TestGetValueAndDeleteBatchWithEmptyMap (0.00s)
=== RUN   TestGetValueAndDeleteBatchFewerElementsThanCount
--- PASS: TestGetValueAndDeleteBatchFewerElementsThanCount (0.00s)
=== RUN   TestGetValueAndDeleteBatchExactElements
--- PASS: TestGetValueAndDeleteBatchExactElements (0.00s)
PASS
ok      github.com/parca-dev/parca-agent/pkg/profiler   (cached)

With ASAN:

$ make test_profiler ENABLE_ASAN=yes
sudo -E GOOS=linux GOARCH=arm64 CC=clang CGO_CFLAGS="-I /home/vagrant/work/parca-agent/dist/libbpf/usr/include" CGO_LDFLAGS="/home/vagrant/work/parca-agent/dist/libbpf/libbpf.a" go test -v github.com/parca-dev/parca-agent/pkg/profiler -asan
=== RUN   TestDeleteNonExistentKeyReturnsEnoent
--- PASS: TestDeleteNonExistentKeyReturnsEnoent (0.00s)
=== RUN   TestDeleteExistentKey
--- PASS: TestDeleteExistentKey (0.00s)
=== RUN   TestGetValueAndDeleteBatchWithEmptyMap
--- PASS: TestGetValueAndDeleteBatchWithEmptyMap (0.00s)
=== RUN   TestGetValueAndDeleteBatchFewerElementsThanCount
--- PASS: TestGetValueAndDeleteBatchFewerElementsThanCount (0.00s)
=== RUN   TestGetValueAndDeleteBatchExactElements
--- PASS: TestGetValueAndDeleteBatchExactElements (0.00s)
PASS
ok      github.com/parca-dev/parca-agent/pkg/profiler   0.047s

Ran:
```
$ go get github.com/aquasecurity/libbpfgo@80f44303d40f3e65702de30d4976ada883e79fe2
```

Changes from the version we have until this pinned revision: aquasecurity/libbpfgo@v0.2.5-libbpf-0.7.0...80f4430
@javierhonduco javierhonduco force-pushed the bump-libbpfgo branch 3 times, most recently from b591f07 to 817340d Compare May 3, 2022 14:53
@kakkoyun kakkoyun self-requested a review May 3, 2022 14:59
Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

Amazing PR! Thanks for adding the tests 👍

I have added a couple of suggestions.

Makefile Outdated Show resolved Hide resolved
pkg/profiler/profiler_test.go Show resolved Hide resolved
pkg/profiler/profiler_test.go Outdated Show resolved Hide resolved
pkg/profiler/profiler_test.go Outdated Show resolved Hide resolved
pkg/profiler/profiler_test.go Outdated Show resolved Hide resolved
To ensure that the libbpfgo invariants hold true. Adding these tests
will also help us with library upgrades, as well as configuration
changes in the way libbpf deals with errors, etc.

**Test plan**:
```
$ make test_profiler
sudo -E GOOS=linux GOARCH=arm64 CC=clang CGO_CFLAGS="-I /home/vagrant/work/parca-agent/dist/libbpf/usr/include" CGO_LDFLAGS="/home/vagrant/work/parca-agent/dist/libbpf/libbpf.a" go test -v github.com/parca-dev/parca-agent/pkg/profiler
=== RUN   TestDeleteNonExistentKeyReturnsEnoent
--- PASS: TestDeleteNonExistentKeyReturnsEnoent (0.00s)
=== RUN   TestDeleteExistentKey
--- PASS: TestDeleteExistentKey (0.00s)
=== RUN   TestGetValueAndDeleteBatchWithEmptyMap
--- PASS: TestGetValueAndDeleteBatchWithEmptyMap (0.00s)
=== RUN   TestGetValueAndDeleteBatchFewerElementsThanCount
--- PASS: TestGetValueAndDeleteBatchFewerElementsThanCount (0.00s)
=== RUN   TestGetValueAndDeleteBatchExactElements
--- PASS: TestGetValueAndDeleteBatchExactElements (0.00s)
PASS
ok      github.com/parca-dev/parca-agent/pkg/profiler   (cached)
```

With ASAN:
```
$ make test_profiler ENABLE_ASAN=yes
sudo -E GOOS=linux GOARCH=arm64 CC=clang CGO_CFLAGS="-I /home/vagrant/work/parca-agent/dist/libbpf/usr/include" CGO_LDFLAGS="/home/vagrant/work/parca-agent/dist/libbpf/libbpf.a" go test -v github.com/parca-dev/parca-agent/pkg/profiler -asan
=== RUN   TestDeleteNonExistentKeyReturnsEnoent
--- PASS: TestDeleteNonExistentKeyReturnsEnoent (0.00s)
=== RUN   TestDeleteExistentKey
--- PASS: TestDeleteExistentKey (0.00s)
=== RUN   TestGetValueAndDeleteBatchWithEmptyMap
--- PASS: TestGetValueAndDeleteBatchWithEmptyMap (0.00s)
=== RUN   TestGetValueAndDeleteBatchFewerElementsThanCount
--- PASS: TestGetValueAndDeleteBatchFewerElementsThanCount (0.00s)
=== RUN   TestGetValueAndDeleteBatchExactElements
--- PASS: TestGetValueAndDeleteBatchExactElements (0.00s)
PASS
ok      github.com/parca-dev/parca-agent/pkg/profiler   0.047s
```
@javierhonduco javierhonduco marked this pull request as ready for review May 3, 2022 16:01
@kakkoyun kakkoyun merged commit 3c502dc into parca-dev:main May 4, 2022
@javierhonduco javierhonduco deleted the bump-libbpfgo branch May 4, 2022 07:56
javierhonduco pushed a commit to javierhonduco/parca-agent that referenced this pull request May 5, 2022
This PR is based off [@kakkoyun's work](parca-dev#326)
to use libbpf(go)'s batch APIs.

**Context**
The main issue we found while working with this API was that they were erroring with `EPERM`.
After some debugging, we realised that libbpf wasn't handle with errors in the way we
expected.

The debugging write-up and more context can be found [here](aquasecurity/libbpfgo#159),
and the fix is in [this PR](aquasecurity/libbpfgo#157).

After landing these changes upstream, we pointed to the updated libbpfgo version, as well as added
some [regression tests](parca-dev#381) to ensure that libbpfgo
behaves as expected, and to make it easier in the future to write further compatibility tests.

Note: the rest of the batch APIs error handling is still unfixed. Tracking in aquasecurity/libbpfgo#162.
javierhonduco added a commit to javierhonduco/parca-agent that referenced this pull request May 5, 2022
This PR is based off [@kakkoyun's work](parca-dev#326)
to use libbpf(go)'s batch APIs.

**Context**
The main issue we found while working with this API was that they were erroring with `EPERM`.
After some debugging, we realised that libbpf wasn't handle with errors in the way we
expected.

The debugging write-up and more context can be found [here](aquasecurity/libbpfgo#159),
and the fix is in [this PR](aquasecurity/libbpfgo#157).

After landing these changes upstream, we pointed to the updated libbpfgo version, as well as added
some [regression tests](parca-dev#381) to ensure that libbpfgo
behaves as expected, and to make it easier in the future to write further compatibility tests.

Note: the rest of the batch APIs error handling is still unfixed. Tracking in aquasecurity/libbpfgo#162.
javierhonduco added a commit to javierhonduco/parca-agent that referenced this pull request May 5, 2022
This PR is based off [@kakkoyun's work](parca-dev#326)
to use libbpf(go)'s batch APIs.

**Context**
The main issue we found while working with this API was that they were erroring with `EPERM`.
After some debugging, we realised that libbpf wasn't handle with errors in the way we
expected.

The debugging write-up and more context can be found [here](aquasecurity/libbpfgo#159),
and the fix is in [this PR](aquasecurity/libbpfgo#157).

After landing these changes upstream, we pointed to the updated libbpfgo version, as well as added
some [regression tests](parca-dev#381) to ensure that libbpfgo
behaves as expected, and to make it easier in the future to write further compatibility tests.

Note: the rest of the batch APIs error handling is still unfixed. Tracking in aquasecurity/libbpfgo#162.
javierhonduco added a commit to javierhonduco/parca-agent that referenced this pull request May 5, 2022
This PR is based off [@kakkoyun's work](parca-dev#326)
to use libbpf(go)'s batch APIs.

**Context**
The main issue we found while working with this API was that they were erroring with `EPERM`.
After some debugging, we realised that libbpf wasn't handle with errors in the way we
expected.

The debugging write-up and more context can be found [here](aquasecurity/libbpfgo#159),
and the fix is in [this PR](aquasecurity/libbpfgo#157).

After landing these changes upstream, we pointed to the updated libbpfgo version, as well as added
some [regression tests](parca-dev#381) to ensure that libbpfgo
behaves as expected, and to make it easier in the future to write further compatibility tests.

Note: the rest of the batch APIs error handling is still unfixed. Tracking in aquasecurity/libbpfgo#162.
javierhonduco added a commit to javierhonduco/parca-agent that referenced this pull request May 5, 2022
This PR is based off [@kakkoyun's work](parca-dev#326) to use libbpf(go)'s batch APIs.

**Context**
The main issue we found while working with this API was that they were erroring with `EPERM`.
After some debugging, we realised that libbpf wasn't handling errors in the way we
expected.

The debugging write-up and more context can be found [here](aquasecurity/libbpfgo#159),
and the fix is in [this PR](aquasecurity/libbpfgo#157).

After landing these changes upstream, we pointed to the updated libbpfgo version, as well as added
some [regression tests](parca-dev#381) to ensure that libbpfgo
behaves as expected, and to make it easier in the future to write further compatibility tests.

Note: the rest of the batch APIs error handling is still unfixed. Tracking in aquasecurity/libbpfgo#162.
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