Skip to content

Comments

Add percpu buffer flag#552

Merged
brb merged 1 commit intocilium:mainfrom
AdvH039:buff-size-flag
May 28, 2025
Merged

Add percpu buffer flag#552
brb merged 1 commit intocilium:mainfrom
AdvH039:buff-size-flag

Conversation

@AdvH039
Copy link
Contributor

@AdvH039 AdvH039 commented May 15, 2025

Tries to fix #508
(Edit : What command to use for linting?)

@AdvH039 AdvH039 requested a review from a team as a code owner May 15, 2025 17:36
@AdvH039 AdvH039 requested review from brb and removed request for a team May 15, 2025 17:36
@brb
Copy link
Member

brb commented May 16, 2025

Thanks for the PR.

Unfortunately, we cannot use variable length arrays. One solution would be to use a BPF map with value char foo[0] for the buffer, and then set the value size with bpf_map__set_value_size from the agent before loading.

Please let me know if you are comfortable to implement the solution above.

@brb brb marked this pull request as draft May 16, 2025 14:58
@AdvH039
Copy link
Contributor Author

AdvH039 commented May 16, 2025

What do you mean by agent exactly?

@brb
Copy link
Member

brb commented May 16, 2025

The Go userspace process (main.go)

@brb
Copy link
Member

brb commented May 19, 2025

@ti-mo suggested:

I'd prob use a simple percpu array and adjust CollectionSpec.Maps["scratch"].ValueSize as appropriate before loading

@AdvH039 AdvH039 force-pushed the buff-size-flag branch 2 times, most recently from ad2b4f3 to 4e48b06 Compare May 20, 2025 17:39
@brb brb marked this pull request as ready for review May 22, 2025 15:40
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks! One nit.

flag.StringVar(&f.Backend, "backend", "",
fmt.Sprintf("Tracing backend('%s', '%s'). Will auto-detect if not specified.", BackendKprobe, BackendKprobeMulti))

flag.Uint32Var(&f.SetPerCPUBuf, "set-percpu-buf", 4096, "set the size of the percpu buffer")
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you could be more specific with the flag description? I.e. instead of saying "percpu buffer", you could say "set the size of buffers to print skb data (used by --output-skb and --output-skb-shared-info)".

Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks!

@brb brb merged commit 8f322d2 into cilium:main May 28, 2025
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.

--output-skb is cut short

2 participants