Fix arm64 testsuite failures #745
Conversation
|
@lmb The
|
|
Thanks! Just saw the following as well: Why is that? |
|
Ok, I read that as "don't let EPERM fall into the default switch case". Wrapping the error forces use of |
lmb
force-pushed
the
arm64
branch
4 times, most recently
from
cd71052 to
3c1038c
Compare
ENOTSUPP is not included in x/sys/unix since it's meant to be kernel internal. Move it from our wrapper package into internal/sys. Also provide a descriptive error message.
Linux on arm64 returns ENOTSUPP when trying to create a tracing link via BPF_LINK_CREATE and BPF_RAW_TRACEPOINT_OPEN. Turn this into ErrNotSupported.
lmb
force-pushed
the
arm64
branch
2 times, most recently
from
029442b to
cbd720f
Compare
Use symbols and tracepoints which are portable across amd64 and arm64. Based on ubuntu 5.13.0-52-generic aarch64.
Don't return unwrapped sentinel errors from various feature tests. testutils.SkipIfNotSupported will bail out if it encounters an unwrapped ErrNotSupported.
|
Ugh, finally a green build. I'll merge this tomorrow-ish unless someone objects. |
Ha, I just asked myself the same question today while working on #746. We should probably remove it, it complicates the logic unnecessarily. I want to wrap everything except |
Trying to create a perf_event uprobe on arm64 doesn't seem to allow passing an address in the first page. Use the first byte of the second page instead.
Passing flags to BPF_MAP_LOOKUP_AND_DELETE_ELEM is only possible from 5.14. Skip the test on earlier kernels.
TestKprobeOffset used to hardcode specific offsets to test against, which meant we had to make it amd64 specific. It turns out that even on a single arch the offsets aren't stable due to changes in the compiler. Hence the test was changed to brute force a valid offset, but we forgot to run it on all arches. Trying to execute the test on Linux 5.13 on arm64 triggers EINVAL instead of the expected EILSEQ however. This is because arm64 returns EINVAL when trying to use an offset that is not a multiple of four. Relying on EILSEQ is therefore not portable, and probably shouldn't be encouraged. Drop the check for EILSEQ from TestKprobeOffset and execute it on all arches.
|
@lmb I've dropped eb06124 from the PR since it breaks API, and from what I understand this is more cosmetic than functional. Callers might already rely on ENOENT specifically being returned for missing symbols across all kernels, as well as when scanning for valid offsets within a known symbol. The original meaning of ENOENT in this context is 'symbol not found', which, with some creativity, could be interpreted as 'offset not found' when trying to attach to specific offsets, although that might be a stretch. Could you maybe try to tackle this from another angle? I don't see a simple way out here. |
|
Turning EINVAL into ENOENT is just plain wrong unfortunately, and does obscure bugs. There are a ton of reasons why perf event open will return EINVAL, some of them arch specific, that don't come from the symbol being missing. Trying to paper them over doesn't work, so I think the breakage is inevitable. |
|
@lmb Fair enough, will add it back in through a separate PR. This'll make it pop up in the changelog, so we won't forget to add a breaking change to the relnotes. |
Tests now pass on arm64:
Best review on a per-commit basis.
Updates #266