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

kprobes: matchArgs improvements (for fd/file) #235

Merged
merged 10 commits into from
Jul 22, 2022
Merged

Conversation

tpapagian
Copy link
Member

@tpapagian tpapagian commented Jul 14, 2022

In kprobes, we support matchArgs selector for fd and file types with Equal, Prefix, and Postfix operators.

This PR first introduces unit-tests for all of these cases. These tests show an issue in the case of Postfix operator and we also fix this issue.

Currently, matchArgs for fd/file types support up to 2 values. This PR also increases this number for newer kernels (5.3+) to 8. It keeps that to 2 for older kernels.

Finally, we also update unit-tests to check these.

ps. Tested locally on 4.19, 5.4, 5.10, and 5.13.

Signed-off-by: Anastasios Papagiannis tasos.papagiannnis@gmail.com

@tpapagian tpapagian requested a review from a team as a code owner July 14, 2022 14:44
@tpapagian tpapagian requested a review from olsajiri July 14, 2022 14:44
@tpapagian tpapagian marked this pull request as draft July 14, 2022 14:44
@tpapagian tpapagian marked this pull request as ready for review July 14, 2022 14:50
@olsajiri
Copy link
Contributor

I'm getting tests fails (I ran on latest upstream, then I tried on 5.4 with similar results)
out.test.5.4.txt

5.4:

--- PASS: TestKprobeMatchArgsFileEqual (13.53s)
--- PASS: TestKprobeMatchArgsFilePostfix (14.33s)
--- PASS: TestKprobeMatchArgsFilePrefix (18.18s)
--- FAIL: TestKprobeMatchArgsFdEqual (82.54s)
--- FAIL: TestKprobeMatchArgsFdPostfix (76.45s)
--- FAIL: TestKprobeMatchArgsFdPrefix (81.63s)

5.19:

--- PASS: TestKprobeMatchArgsFileEqual (8.66s)
--- PASS: TestKprobeMatchArgsFilePostfix (10.43s)
--- PASS: TestKprobeMatchArgsFilePrefix (11.79s)
--- FAIL: TestKprobeMatchArgsFdEqual (67.11s)
--- FAIL: TestKprobeMatchArgsFdPostfix (61.51s)
--- PASS: TestKprobeMatchArgsFdPrefix (19.03s)

@tpapagian
Copy link
Member Author

@olsajiri I haven't tested them on 5.19.

On 5.4.182 both with:

CGO_LDFLAGS=-L$(realpath ./lib) go test -gcflags="" -c ./pkg/sensors/tracing/ -o go-tests/sensor-tracing.test
time sudo TETRAGON_LIB=$(realpath ./bpf/objs/)  CGO_LDFLAGS=-L$(realpath ./lib) LD_LIBRARY_PATH=$(realpath ./lib) ./go-tests/sensor-tracing.test

and

sudo TETRAGON_LIB=$(realpath ./bpf/objs/) LD_LIBRARY_PATH=$(realpath ./lib) /usr/local/go/bin/go test -p 1 -parallel 1 -gcflags="" -timeout 20m -failfast -cover ./pkg/sensors/...

I cannot see any issues. Even in the case of these that run successfully in your case i.e.
--- PASS: TestKprobeMatchArgsFileEqual (13.53s)
they seem to be very slow:

$ time sudo TETRAGON_LIB=$(realpath ./bpf/objs/)  CGO_LDFLAGS=-L$(realpath ./lib) LD_LIBRARY_PATH=$(realpath ./lib) ./go-tests/sensor-tracing.test -test.run TestKprobeMatchArgsFileEqual &> /dev/null

real	0m2.920s
user	0m2.434s
sys	0m1.500s

(on a gcloud VM with 4 vCPUs and 16GB DRAM).

Maybe a race? I will check further...

@olsajiri
Copy link
Contributor

I used 4 CPUs with 8GB, after adding another 8GB it's much faster, but still failing, I'll try to debug that

@tpapagian
Copy link
Member Author

Always the same tests are failing?

@olsajiri
Copy link
Contributor

Always the same tests are failing?

yes

@tpapagian tpapagian marked this pull request as draft July 15, 2022 12:43
@tpapagian tpapagian force-pushed the pr/apapag/kprobe_tests branch 3 times, most recently from e68748c to fb21d4a Compare July 18, 2022 07:37
@tpapagian tpapagian changed the base branch from main to pr/willfindlay/fix-argument-checkers July 18, 2022 07:38
@tpapagian tpapagian marked this pull request as ready for review July 18, 2022 17:01
@tpapagian tpapagian removed the request for review from willfindlay July 21, 2022 10:06
@tpapagian tpapagian changed the base branch from pr/willfindlay/fix-argument-checkers to main July 21, 2022 10:06
@tpapagian tpapagian force-pushed the pr/apapag/kprobe_tests branch 2 times, most recently from 2f81c86 to 9a55b0e Compare July 21, 2022 14:24
Currently, we support up to 2 values under matchArgs with "file"
type.

This commit introduces 3 new tests:
TestKprobeMatchArgsFileEqual
TestKprobeMatchArgsFilePostfix
TestKprobeMatchArgsFilePrefix

that checks the 3 different operators ("Equal", "Prefix", and "Postfix")
with 2 values. These tests does not use any other than matchArgs
selectors in order to do a targeted testing.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
The previous commit (c0a16b3) shows that there is a bug in
TestKprobeMatchArgsFilePrefix unit test and more specifically
in the case of Prefix operator.

Most possibly this bug was introduced in #90.

This commit fixes this issue.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
Now we have cmpbytes_small and cmpbytes. The difference is the
number of iterations in these 2 cases. We choose the small variant
in the case where __LARGE_BPF_PROG is not defined.

This commit removes cmpbytes_small and changed the number of iterations
based on __LARGE_BPF_PROG.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
A previous commit introduces tests for "file" type. Internally,
"fd" and "file" use the same low-level mechanism but there are
small differences in several code paths.

This commit introduces 3 new tests:
TestKprobeMatchArgsFdEqual
TestKprobeMatchArgsFdPostfix
TestKprobeMatchArgsFdPrefix

that checks the 3 different operators ("Equal", "Prefix", and "Postfix")
with 2 values. These tests does not use any other than matchArgs
selectors in order to do a targeted testing.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
…le")

Currently, matchArgs for "fd" and "file" types supports up to 2 values
in all kernel versions.

This commit increases this value to 8 for kernels >= 5.3 (i.e. when
__LARGE_BPF_PROG is defined). We still support 2 values for kernels
< 5.3.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
The previous commit introduces increased number of values in matchArgs
for "file" and "fd" types.

This commit updates TestKprobeMatchArgs* unit tests to use 4 values
(instead of 2) in newer kernels.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
This never changes

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
In a similar way to when we start listening for events.
Useful when we debug issues in the CI.

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
Now the timeout is 20 seconds. In the case where we load a large
number of kprobe hooks (i.e. > 3) in a loaded machine, this
can be exceeded easily.

In that case, we stopped watching for events when we run the actual
test and for this reason we are not seeing any kprobe events.

FIXES: #247

Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
@kkourt kkourt merged commit e0f14e0 into main Jul 22, 2022
@kkourt kkourt deleted the pr/apapag/kprobe_tests branch July 22, 2022 13:27
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.

4 participants