Skip to content

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Aug 6, 2024

internal/sys: generate constants from unnamed enums

bpf.h defines a bunch of constants as unnamed enums. This makes them tricky
to pull out of vmlinux BTF, so instead we rely on the internal/unix shim.
This causes a small maintenance burden whenever we need a new flag, but more
importantly it makes some upcoming cross platform work harder.

Pull the constant definitions out of vmlinux BTF instead of relying on 
package unix. Unfortunately a bunch of the flags are re-used. This is most
noticeable for MapFlags, for which the enum in the kernel has become a bit
of a grab bag. Instead of trying hard to wrap everything into a neat type we
just generate untyped constants and get rid of MapFlags.

Signed-off-by: Lorenz Bauer <[email protected]>

internal/sys: add DEFINEd constants

Some constants in bpf.h are defined using the C preprocessor and are 
therefore not available in BTF. Add them manually.

Signed-off-by: Lorenz Bauer <[email protected]>

@github-actions github-actions bot added the breaking-change Changes exported API label Aug 6, 2024
@@ -227,7 +227,7 @@ func init() {
}

// MapFlags document which flags may be feature probed.
type MapFlags = sys.MapFlags
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the breaking change the linter is complaining about. I don't think this should make a difference in practice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, should be fine since external callers never had access to sys.MapFlags anyway.

@lmb lmb marked this pull request as ready for review August 6, 2024 13:40
@lmb lmb requested review from florianl, mmat11, rgo3 and a team as code owners August 6, 2024 13:40
Copy link
Collaborator

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

One suggestion, looks fine otherwise!

@ti-mo ti-mo removed the breaking-change Changes exported API label Aug 6, 2024
@github-actions github-actions bot added the breaking-change Changes exported API label Aug 6, 2024
lmb added 2 commits August 6, 2024 16:15
bpf.h defines a bunch of constants as unnamed enums. This makes them
tricky to pull out of vmlinux BTF, so instead we rely on the internal/unix
shim. This causes a small maintenance burden whenever we need a new flag,
but more importantly it makes some upcoming cross platform work harder.

Pull the constant definitions out of vmlinux BTF instead of relying on
package unix. Unfortunately a bunch of the flags are re-used.
This is most noticeable for MapFlags, for which the enum in the kernel
has become a bit of a grab bag. Instead of trying hard to wrap everything
into a neat type we just generate untyped constants and get rid of MapFlags.

Signed-off-by: Lorenz Bauer <[email protected]>
Some constants in bpf.h are defined using the C preprocessor and are
therefore not available in BTF. Add them manually.

Signed-off-by: Lorenz Bauer <[email protected]>
@lmb lmb merged commit 3403595 into cilium:main Aug 6, 2024
17 checks passed
@lmb lmb deleted the sys-constants branch August 6, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Changes exported API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants