Skip to content

Commit

Permalink
btf: remove Int.Bits and Int.Offset
Browse files Browse the repository at this point in the history 
Unfortunately BTF has two ways to encode bitfields. You can
create a Member and set Offset / BitfieldSize or create an Int that
has the offset and bitfield size baked in. The background for this
can be found in [1].

Remove Int.Bits and Int.Offset in favour of Member.BitfieldSize and
Member.Offset. Also get rid of a bunch of incorrect checks for bitfields,
which we didn't catch due to the weird encoding. This cleans up the API,
but may make our lives worse if we end up having to support writing out
legacy bitfields. Let's hope it doesn't come to that.

The performance of parsing vmlinux hasn't changed dramatically:

    name            old time/op    new time/op    delta
    ParseVmlinux-4    62.1ms ± 1%    61.8ms ± 2%    ~     (p=0.686 n=4+4)

    name            old alloc/op   new alloc/op   delta
    ParseVmlinux-4    44.2MB ± 0%    44.7MB ± 0%  +1.14%  (p=0.029 n=4+4)

    name            old allocs/op  new allocs/op  delta
    ParseVmlinux-4      718k ± 0%      718k ± 0%  +0.00%  (p=0.029 n=4+4)

1: torvalds/linux@9d5f9f7
  • Loading branch information
@lmb
lmb committed May 4, 2022
1 parent cc00f26 commit 3187212
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 62 deletions.
54 changes: 32 additions & 22 deletions internal/btf/btf_test.go
View file
Expand Up @@ -134,28 +134,38 @@ func TestTypeByName(t *testing.T) {
t.Fatal("multiple TypeByName calls for `iphdr` name do not return the same addresses")
}

for _, m := range iphdr1.Members {
if m.Name == "version" {
// __u8 is a typedef
td, ok := m.Type.(*Typedef)
if !ok {
t.Fatalf("version member of iphdr should be a __u8 typedef: actual: %T", m.Type)
}
u8int, ok := td.Type.(*Int)
if !ok {
t.Fatalf("__u8 typedef should point to an Int type: actual: %T", td.Type)
}
if u8int.Bits != 8 {
t.Fatalf("incorrect bit size of an __u8 int: expected: 8 actual: %d", u8int.Bits)
}
if u8int.Encoding != 0 {
t.Fatalf("incorrect encoding of an __u8 int: expected: 0 actual: %x", u8int.Encoding)
}
if u8int.Offset != 0 {
t.Fatalf("incorrect int offset of an __u8 int: expected: 0 actual: %d", u8int.Offset)
}
break
}
// Excerpt from linux/ip.h, https://elixir.bootlin.com/linux/latest/A/ident/iphdr
//
// struct iphdr {
// #if defined(__LITTLE_ENDIAN_BITFIELD)
// __u8 ihl:4, version:4;
// #elif defined (__BIG_ENDIAN_BITFIELD)
// __u8 version:4, ihl:4;
// #else
// ...
// }
//
// The BTF we test against is for little endian.
m := iphdr1.Members[1]
if m.Name != "version" {
t.Fatal("Expected version as the second member, got", m.Name)
}
td, ok := m.Type.(*Typedef)
if !ok {
t.Fatalf("version member of iphdr should be a __u8 typedef: actual: %T", m.Type)
}
u8, ok := td.Type.(*Int)
if !ok {
t.Fatalf("__u8 typedef should point to an Int type: actual: %T", td.Type)
}
if m.BitfieldSize != 4 {
t.Fatalf("incorrect bitfield size: expected: 4 actual: %d", m.BitfieldSize)
}
if u8.Encoding != 0 {
t.Fatalf("incorrect encoding of an __u8 int: expected: 0 actual: %x", u8.Encoding)
}
if m.Offset != 4 {
t.Fatalf("incorrect bitfield offset: expected: 4 actual: %d", m.Offset)
}
}

Expand Down
17 changes: 2 additions & 15 deletions internal/btf/core.go
View file
Expand Up @@ -893,15 +893,9 @@ func coreAreTypesCompatible(localType Type, targetType Type) error {
}

switch lv := (localType).(type) {
case *Void, *Struct, *Union, *Enum, *Fwd:
case *Void, *Struct, *Union, *Enum, *Fwd, *Int:
// Nothing to do here

case *Int:
tv := targetType.(*Int)
if lv.isBitfield() || tv.isBitfield() {
return fmt.Errorf("bitfield: %w", errImpossibleRelocation)
}

case *Pointer, *Array:
depth++
localType.walk(&localTs)
Expand Down Expand Up @@ -983,7 +977,7 @@ func coreAreMembersCompatible(localType Type, targetType Type) error {
}

switch lv := localType.(type) {
case *Array, *Pointer, *Float:
case *Array, *Pointer, *Float, *Int:
return nil

case *Enum:
Expand All @@ -994,13 +988,6 @@ func coreAreMembersCompatible(localType Type, targetType Type) error {
tv := targetType.(*Fwd)
return doNamesMatch(lv.Name, tv.Name)

case *Int:
tv := targetType.(*Int)
if lv.isBitfield() || tv.isBitfield() {
return fmt.Errorf("bitfield: %w", errImpossibleRelocation)
}
return nil

default:
return fmt.Errorf("type %s: %w", localType, ErrNotSupported)
}
Expand Down
2 changes: 0 additions & 2 deletions internal/btf/core_test.go
View file
Expand Up @@ -25,7 +25,6 @@ func TestCOREAreTypesCompatible(t *testing.T) {
{&Enum{Name: "a"}, &Enum{Name: "b"}, true},
{&Fwd{Name: "a"}, &Fwd{Name: "b"}, true},
{&Int{Name: "a", Size: 2}, &Int{Name: "b", Size: 4}, true},
{&Int{Offset: 1}, &Int{}, false},
{&Pointer{Target: &Void{}}, &Pointer{Target: &Void{}}, true},
{&Pointer{Target: &Void{}}, &Void{}, false},
{&Array{Type: &Void{}}, &Array{Type: &Void{}}, true},
Expand Down Expand Up @@ -99,7 +98,6 @@ func TestCOREAreMembersCompatible(t *testing.T) {
{&Fwd{Name: "a"}, &Fwd{Name: "a___foo"}, true},
{&Fwd{Name: "a"}, &Fwd{Name: ""}, true},
{&Int{Name: "a", Size: 2}, &Int{Name: "b", Size: 4}, true},
{&Int{Offset: 1}, &Int{}, false},
{&Pointer{Target: &Void{}}, &Pointer{Target: &Void{}}, true},
{&Pointer{Target: &Void{}}, &Void{}, false},
{&Array{Type: &Int{Size: 1}}, &Array{Type: &Int{Encoding: Signed}}, true},
Expand Down
82 changes: 60 additions & 22 deletions internal/btf/types.go
View file
Expand Up @@ -127,20 +127,10 @@ type Int struct {
// The size of the integer in bytes.
Size uint32
Encoding IntEncoding
// Offset is the starting bit offset. Currently always 0.
Offset Bits
Bits Bits
}

func (i *Int) Format(fs fmt.State, verb rune) {
extra := []interface{}{
i.Encoding,
"size=", i.Size * 8,
}
if i.Bits > 0 {
extra = append(extra, "bits=", i.Bits)
}
formatType(fs, verb, i, extra...)
formatType(fs, verb, i, i.Encoding, "size=", i.Size*8)
}

func (i *Int) TypeName() string { return i.Name }
Expand All @@ -151,10 +141,6 @@ func (i *Int) copy() Type {
return &cpy
}

func (i *Int) isBitfield() bool {
return i.Offset > 0
}

// Pointer is a pointer to another type.
type Pointer struct {
TypeID
Expand Down Expand Up @@ -844,6 +830,15 @@ func inflateRawTypes(rawTypes []rawType, rawStrings *stringTable) ([]Type, map[e
return nil
}

type bitfieldFixupDef struct {
id TypeID
m *Member
}

var (
legacyBitfields = make(map[TypeID][2]Bits) // offset, size
bitfieldFixups []bitfieldFixupDef
)
convertMembers := func(raw []btfMember, kindFlag bool) ([]Member, error) {
// NB: The fixup below relies on pre-allocating this array to
// work, since otherwise append might re-allocate members.
Expand All @@ -853,18 +848,48 @@ func inflateRawTypes(rawTypes []rawType, rawStrings *stringTable) ([]Type, map[e
if err != nil {
return nil, fmt.Errorf("can't get name for member %d: %w", i, err)
}
m := Member{

members = append(members, Member{
Name: name,
Offset: Bits(btfMember.Offset),
}
})

m := &members[i]
fixup(raw[i].Type, &m.Type)

if kindFlag {
m.BitfieldSize = Bits(btfMember.Offset >> 24)
m.Offset &= 0xffffff
// We ignore legacy bitfield definitions if the current composite
// is a new-style bitfield. This is kind of safe since offset and
// size on the type of the member must be zero if kindFlat is set
// according to spec.
continue
}
members = append(members, m)
}
for i := range members {
fixup(raw[i].Type, &members[i].Type)

// This may be a legacy bitfield, try to fix it up.
data, ok := legacyBitfields[raw[i].Type]
if ok {
// Bingo!
m.Offset += data[0]
m.BitfieldSize = data[1]
continue
}

if m.Type != nil {
// We couldn't find a legacy bitfield, but we know that the member's
// type has already been inflated. Hence we know that it can't be
// a legacy bitfield and there is nothing left to do.
continue
}

// We don't have fixup data, and the type we're pointing
// at hasn't been inflated yet. No choice but to defer
// the fixup.
bitfieldFixups = append(bitfieldFixups, bitfieldFixupDef{
raw[i].Type,
m,
})
}
return members, nil
}
Expand All @@ -884,8 +909,12 @@ func inflateRawTypes(rawTypes []rawType, rawStrings *stringTable) ([]Type, map[e

switch raw.Kind() {
case kindInt:
size := raw.Size()
encoding, offset, bits := intEncoding(*raw.data.(*uint32))
typ = &Int{id, name, raw.Size(), encoding, offset, bits}
if offset > 0 || bits.Bytes() != size {
legacyBitfields[id] = [2]Bits{offset, bits}
}
typ = &Int{id, name, size, encoding}

case kindPointer:
ptr := &Pointer{id, nil}
Expand Down Expand Up @@ -1031,6 +1060,15 @@ func inflateRawTypes(rawTypes []rawType, rawStrings *stringTable) ([]Type, map[e
*fixup.typ = types[i]
}

for _, bitfieldFixup := range bitfieldFixups {
data, ok := legacyBitfields[bitfieldFixup.id]
if ok {
// This is indeed a legacy bitfield, fix it up.
bitfieldFixup.m.Offset += data[0]
bitfieldFixup.m.BitfieldSize = data[1]
}
}

for _, assertion := range assertions {
if reflect.TypeOf(*assertion.typ) != assertion.want {
return nil, nil, fmt.Errorf("expected %s, got %T", assertion.want, *assertion.typ)
Expand Down
2 changes: 1 addition & 1 deletion internal/btf/types_test.go
View file
Expand Up @@ -102,7 +102,7 @@ func ExampleType_validTypes() {
func TestType(t *testing.T) {
types := []func() Type{
func() Type { return &Void{} },
func() Type { return &Int{Size: 2, Bits: 3} },
func() Type { return &Int{Size: 2} },
func() Type { return &Pointer{Target: &Void{}} },
func() Type { return &Array{Type: &Int{}} },
func() Type {
Expand Down

0 comments on commit 3187212

Please sign in to comment.