btf: work around missing ENUM64 support #1132
Conversation
|
libbpf implementation was added in https://lwn.net/ml/bpf/20220603015937.1190992-1-yhs@fb.com/ |
lmb
force-pushed
the
enum64-workaround
branch
3 times, most recently
from
81fae4b
to
d02ac56
Compare
lmb
force-pushed
the
enum64-workaround
branch
from
d02ac56
to
f0149a8
Compare
|
Now uses a |
| @@ -464,6 +483,41 @@ func (e *encoder) deflateEnumValues(values []EnumValue) ([]btfEnum, error) { | |||
| return bes, nil | |||
| } | |||
|
|
|||
| func (e *encoder) deflateEnum64(raw *rawType, enum *Enum) (err error) { | |||
| if e.ReplaceEnum64 { | |||
There was a problem hiding this comment.
Let's add a quick explanation with some context here. (e.g. why a union and not simply an int like we discussed offline)
There was a problem hiding this comment.
Fair point, but I'll add it to the commit message instead. I think the comment would be too large.
btf/marshal.go
Outdated
| return nil, fmt.Errorf("value of enum %q exceeds 32 bits", value.Name) | ||
| if enum.Signed { | ||
| signedValue := int64(value.Value) | ||
| if signedValue != int64(int32(uint32(value.Value))) { |
There was a problem hiding this comment.
This is too clever to be uncommented. Why compare value.Value against its sign-extended self? Wouldn't truncating it to 32 bits put it within acceptable range anyway? (I assume that's part of the trick) FYI there's now math.MaxInt32 as well, maybe just check if value.Value falls within MaxInt32 and MaxUint32 instead of special-casing signed/unsigned?
There was a problem hiding this comment.
Thanks, comparing against the constants is much easier to understand.
No other changes intended. Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Replace an ENUM64 with a 64-bit integer on kernels which don't support the former. Match libbpf behaviour by replacing the enum with a union. I initially considered replacing it with an int instead, but this can create interoperability problems with libbpf. For example, consider a BPF program loaded by libbpf which contains a function signature that takes an enum64 as an argument. libbpf will replace the enum with a union. Instead of func(enum) we have func(union). This becomes important when the kernel compares type signatures as part of UAPI, like when attaching a BPF program to the function entry of the func(enum) (the mind boggles). If we replaced func(enum) with func(int) the in-kernel comparison would fail. Hence we match what libbpf does. Remove the old heuristic which encoded a 64-bit enum as a 32-bit enum if none of the values exceeded math.MaxUint32. The heuristic has no tests and doesn't take the signedness of the enum into account. Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
lmb
force-pushed
the
enum64-workaround
branch
from
f0149a8
to
6fcb8cb
Compare
btf: move feature tests to separate file
btf: add feature test for BTF_KIND_ENUM64
btf: work around missing ENUM64 support on older kernels
Fixes #1116