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

map: Do not chec maxEntries for PerfEventArray map #714

Merged
merged 1 commit into from
Jun 20, 2022

Conversation

olsajiri
Copy link
Contributor

When I use perf event map in MapReplacements for ebpf.CollectionOptions,
I'm getting error for maxEntries check.

The perf event map is created with maxEntries initialized to number
of cpus, so we can't check maxEntries before the map is created.

I'm not sure we can postpone the check, but we can surely skip it
for perf event map.

Signed-off-by: Jiri Olsa jolsa@kernel.org

@lmb
Copy link
Collaborator

lmb commented Jun 16, 2022

Ah, good point! We already have a bit of special casing for PerfEventArray:

ebpf/map.go

Lines 381 to 387 in be4d571

if spec.MaxEntries == 0 {
n, err := internal.PossibleCPUs()
if err != nil {
return nil, fmt.Errorf("perf event array: %w", err)
}
spec.MaxEntries = uint32(n)
}

What if you only skip the check if ms.Type == PerfEventArray && ms.MaxEntries == 0? That way it's in line with the other special case we have.

@olsajiri
Copy link
Contributor Author

What if you only skip the check if ms.Type == PerfEventArray && ms.MaxEntries == 0? That way it's in line with the other special case we have.

sounds good, will change, thanks

When I use perf event map in MapReplacements for ebpf.CollectionOptions,
I'm getting error for maxEntries check.

The perf event map is created with maxEntries initialized to number
of cpus, so we can't check maxEntries before the map is created.

I'm not sure we can postpone the check, but we can surely skip it
for perf event map.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@olsajiri
Copy link
Contributor Author

@lmb any idea what's wrong with the CI? looks like I can't check results, thanks

@lmb
Copy link
Collaborator

lmb commented Jun 17, 2022

Sometimes it gets stuck, I think due to the VM crashing. I've triggered a rerun.

@ti-mo ti-mo merged commit d17ebbe into cilium:master Jun 20, 2022
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.

3 participants