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

bpf: read_call_arg, copy_path once #2239

Merged
merged 1 commit into from
Mar 18, 2024
Merged

Conversation

kkourt
Copy link
Contributor

@kkourt kkourt commented Mar 18, 2024

The generic tracepoint program does not seem to work on GKE kernel 5.15.133+. This patch, modifies read_call_arg to call copy_path only once. The goal is to reduce the complexity by having the code for copy_path() being inlined only once.

The error on the GKE kernel is:

loadInstance: opening collection 'bpf/objs/bpf_generic_tracepoint_v511.o' failed: program generic_tracepoint_process_event: load program: operation not supported: stack depth 256 (3214132 line(s) omitted)"

As well as:

    from 22 to 1547: safe
    1563: R0=inv0 R6=ctx(id=0,off=0,imm=0) R9=map_value(id=0,off=0,ks=4,vs=24304,imm=0) R10=fp0 fp-176=mmmm????
    ; return generic_process_event(ctx, (struct bpf_map_def *)&tp_heap,
    1563: (b7) r0 = 0
    1564: R0_w=inv0 R6=ctx(id=0,off=0,imm=0) R9=map_value(id=0,off=0,ks=4,vs=24304,imm=0) R10=fp0 fp-176=mmmm????
    1564: (95) exit
    1563: R0_w=inv0 R6_w=ctx(id=0,off=0,imm=0) R9_w=inv0 R10=fp0 fp-176=mmmm????
    1563: (b7) r0 = 0
    1564: R0_w=inv0 R6_w=ctx(id=0,off=0,imm=0) R9_w=inv0 R10=fp0 fp-176=mmmm????
    1564: (95) exit
    verification time 22016603 usec
    stack depth 256
    processed 559046 insns (limit

It's not clear why the error happens, and we were not able to reproduce it in upstream kernels but this patch seems to fix the issue.

The generic tracepoint program does not seem to work on GKE kernel
5.15.133+. This patch, modifies read_call_arg to call copy_path only
once. The goal is to reduce the complexity by having the code for
copy_path() being inlined only once.

The error on the GKE kernel is:
> loadInstance: opening collection 'bpf/objs/bpf_generic_tracepoint_v511.o' failed: program generic_tracepoint_process_event: load program: operation not supported: stack depth 256 (3214132 line(s) omitted)"

As well as:
>         from 22 to 1547: safe
>         1563: R0=inv0 R6=ctx(id=0,off=0,imm=0) R9=map_value(id=0,off=0,ks=4,vs=24304,imm=0) R10=fp0 fp-176=mmmm????
>         ; return generic_process_event(ctx, (struct bpf_map_def *)&tp_heap,
>         1563: (b7) r0 = 0
>         1564: R0_w=inv0 R6=ctx(id=0,off=0,imm=0) R9=map_value(id=0,off=0,ks=4,vs=24304,imm=0) R10=fp0 fp-176=mmmm????
>         1564: (95) exit
>         1563: R0_w=inv0 R6_w=ctx(id=0,off=0,imm=0) R9_w=inv0 R10=fp0 fp-176=mmmm????
>         1563: (b7) r0 = 0
>         1564: R0_w=inv0 R6_w=ctx(id=0,off=0,imm=0) R9_w=inv0 R10=fp0 fp-176=mmmm????
>         1564: (95) exit
>         verification time 22016603 usec
>         stack depth 256
>         processed 559046 insns (limit

It's not clear why the error happens, and we were not able to reproduce
it in upstream kernels but this patch seems to fix the issue.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
@kkourt kkourt added the release-note/misc This PR makes changes that have no direct user impact. label Mar 18, 2024
@kkourt
Copy link
Contributor Author

kkourt commented Mar 18, 2024

veristat output seems promising:

File                            Program                           Verdict (A)  Verdict (B)  Verdict (DIFF)  Duration (us) (A)  Duration (us) (B)  Duration (us) (DIFF)  Insns (A)  Insns (B)  Insns      (DIFF)  States (A)  States (B)  States    (DIFF)  Peak states (A)  Peak states (B)  Peak states (DIFF)
------------------------------  --------------------------------  -----------  -----------  --------------  -----------------  -----------------  --------------------  ---------  ---------  -----------------  ----------  ----------  ----------------  ---------------  ---------------  ------------------
bpf_generic_tracepoint_v511.o   generic_tracepoint_process_event  success      success      MATCH                      275438             138036     -137402 (-49.88%)     278944     141400  -137544 (-49.31%)       21254       10807  -10447 (-49.15%)            16955             8672     -8283 (-48.85%)

@kkourt kkourt marked this pull request as ready for review March 18, 2024 15:29
@kkourt kkourt requested a review from a team as a code owner March 18, 2024 15:29
@kkourt kkourt requested a review from olsajiri March 18, 2024 15:29
Copy link
Contributor

@kevsecurity kevsecurity left a comment

Choose a reason for hiding this comment

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

LGTM

@kkourt kkourt merged commit 1fbb7f3 into main Mar 18, 2024
37 of 38 checks passed
@kkourt kkourt deleted the pr/kkourt/bpf-copy-path-once branch March 18, 2024 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants