Skip to content

Add percpu buffer flag #552

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

Merged
merged 1 commit into from
May 28, 2025
Merged

Add percpu buffer flag #552

merged 1 commit into from
May 28, 2025

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.

@@ -110,6 +112,7 @@ func (f *Flags) SetFlags() {
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