Skip to content

Commit

Permalink
Replace internal.NativeEndian with stdlib
Browse files Browse the repository at this point in the history
Yet another attempt at replacing internal.NativeEndian with NativeEndian
from "encoding/binary". The conversion is mostly a straight-forward
replacement. The only challenge are checks whether a ByteOrder is
Big/Little/NativeEndian. In stdlib, NativeEndian is a distinct type
hence never compares equal to BigEndian or LittleEndian.

Introduce internal.EqualByteOrder(bo1, bo2). It works by passing
[]byte{0x12, 0x34} through bo1.Uint16() and bo2.Uint16() and comparing
the results.

Signed-off-by: Nick Zavaritsky <[email protected]>
  • Loading branch information
mejedi committed Feb 6, 2025
1 parent 1bcc12e commit b802ad6
Show file tree
Hide file tree
Showing 32 changed files with 148 additions and 147 deletions.
39 changes: 19 additions & 20 deletions asm/instruction.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,6 @@ func (ins Instruction) Marshal(w io.Writer, bo binary.ByteOrder) (uint64, error)
cons = int32(uint32(ins.Constant))
}

regs, err := newBPFRegisters(ins.Dst, ins.Src, bo)
if err != nil {
return 0, fmt.Errorf("can't marshal registers: %s", err)
}

if ins.OpCode.Class().IsALU() {
newOffset := int16(0)
switch ins.OpCode.ALUOp() {
Expand All @@ -158,14 +153,10 @@ func (ins Instruction) Marshal(w io.Writer, bo binary.ByteOrder) (uint64, error)
ins.Offset = newOffset
}

op, err := ins.OpCode.bpfOpCode()
if err != nil {
data := make([]byte, InstructionSize)
if err := putOpCodeAndRegs(data[:2], ins, bo); err != nil {
return 0, err
}

data := make([]byte, InstructionSize)
data[0] = op
data[1] = byte(regs)
bo.PutUint16(data[2:4], uint16(ins.Offset))
bo.PutUint32(data[4:8], uint32(cons))
if _, err := w.Write(data); err != nil {
Expand Down Expand Up @@ -931,17 +922,25 @@ func (iter *InstructionIterator) Next() bool {
return true
}

type bpfRegisters uint8
func putOpCodeAndRegs(data []byte, ins Instruction, bo binary.ByteOrder) error {
// Source and destination registers are encoded in 4-bit bitfields
// within a byte, the order being different on big and little endian
// systems. Use ByteOrder.Uint16() as "swap source and destination
// if needed" device; reuse data[:] as scratch space to avoid
// allocations.
data[0] = byte(ins.Dst)
data[1] = byte(ins.Src)
v := bo.Uint16(buf)

Check failure on line 933 in asm/instruction.go

View workflow job for this annotation

GitHub Actions / Build and Lint

undefined: buf) (typecheck)

Check failure on line 933 in asm/instruction.go

View workflow job for this annotation

GitHub Actions / Build and Lint

undefined: buf) (typecheck)

Check failure on line 933 in asm/instruction.go

View workflow job for this annotation

GitHub Actions / Build and Lint

undefined: buf) (typecheck)

Check failure on line 933 in asm/instruction.go

View workflow job for this annotation

GitHub Actions / Build and Lint

undefined: buf (typecheck)

func newBPFRegisters(dst, src Register, bo binary.ByteOrder) (bpfRegisters, error) {
switch bo {
case binary.LittleEndian:
return bpfRegisters((src << 4) | (dst & 0xF)), nil
case binary.BigEndian:
return bpfRegisters((dst << 4) | (src & 0xF)), nil
default:
return 0, fmt.Errorf("unrecognized ByteOrder %T", bo)
data[1] = byte((v >> 4) | (v & 0xf))

op, err := ins.OpCode.bpfOpCode()
if err != nil {
return err
}

data[0] = op
return nil
}

// IsUnreferencedSymbol returns true if err was caused by
Expand Down
2 changes: 1 addition & 1 deletion btf/btf.go
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ func (s *Spec) TypeByName(name string, typ interface{}) error {
// Types from base are used to resolve references in the split BTF.
// The returned Spec only contains types from the split BTF, not from the base.
func LoadSplitSpecFromReader(r io.ReaderAt, base *Spec) (*Spec, error) {
return loadRawSpec(r, internal.NativeEndian, base)
return loadRawSpec(r, binary.NativeEndian, base)
}

// TypesIterator iterates over types of a given spec.
Expand Down
7 changes: 5 additions & 2 deletions btf/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"strings"

"github.com/cilium/ebpf/asm"
"github.com/cilium/ebpf/internal"
)

// Code in this file is derived from libbpf, which is available under a BSD
Expand Down Expand Up @@ -205,8 +206,10 @@ func CORERelocate(relos []*CORERelocation, targets []*Spec, bo binary.ByteOrder,
resolveTargetTypeID := targets[0].TypeID

for _, target := range targets {
if bo != target.imm.byteOrder {
return nil, fmt.Errorf("can't relocate %s against %s", bo, target.imm.byteOrder)
if !internal.EqualByteOrder(bo, target.imm.byteOrder) {
return nil, fmt.Errorf("can't relocate %s against %s",
internal.NormalizeByteOrder(bo),
internal.NormalizeByteOrder(target.imm.byteOrder))
}
}

Expand Down
12 changes: 6 additions & 6 deletions btf/core_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package btf

import (
"encoding/binary"
"errors"
"fmt"
"os"
Expand All @@ -10,7 +11,6 @@ import (

"github.com/google/go-cmp/cmp"

"github.com/cilium/ebpf/internal"
"github.com/cilium/ebpf/internal/testutils"

"github.com/go-quicktest/qt"
Expand Down Expand Up @@ -588,7 +588,7 @@ func TestCOREReloFieldSigned(t *testing.T) {
relo := &CORERelocation{
typ, coreAccessor{0}, reloFieldSigned, 0,
}
fixup, err := coreCalculateFixup(relo, &Void{}, internal.NativeEndian, dummyTypeID)
fixup, err := coreCalculateFixup(relo, &Void{}, binary.NativeEndian, dummyTypeID)
qt.Assert(t, qt.IsTrue(fixup.poison))
qt.Assert(t, qt.IsNil(err))
})
Expand All @@ -598,7 +598,7 @@ func TestCOREReloFieldSigned(t *testing.T) {
relo := &CORERelocation{
&Array{}, coreAccessor{0}, reloFieldSigned, 0,
}
_, err := coreCalculateFixup(relo, &Array{}, internal.NativeEndian, dummyTypeID)
_, err := coreCalculateFixup(relo, &Array{}, binary.NativeEndian, dummyTypeID)
qt.Assert(t, qt.ErrorIs(err, errNoSignedness))
})
}
Expand All @@ -615,7 +615,7 @@ func TestCOREReloFieldShiftU64(t *testing.T) {
{typ, coreAccessor{0, 0}, reloFieldLShiftU64, 1},
} {
t.Run(relo.kind.String(), func(t *testing.T) {
_, err := coreCalculateFixup(relo, typ, internal.NativeEndian, dummyTypeID)
_, err := coreCalculateFixup(relo, typ, binary.NativeEndian, dummyTypeID)
qt.Assert(t, qt.ErrorIs(err, errUnsizedType))
})
}
Expand All @@ -639,7 +639,7 @@ func TestCORERelosKmodTypeID(t *testing.T) {
fixups, err := coreCalculateFixups(
relos,
[]Type{a, b},
internal.NativeEndian,
binary.NativeEndian,
typeID,
)
qt.Assert(t, qt.IsNil(err))
Expand All @@ -649,7 +649,7 @@ func TestCORERelosKmodTypeID(t *testing.T) {
fixups, err = coreCalculateFixups(
relos,
[]Type{b},
internal.NativeEndian,
binary.NativeEndian,
typeID,
)
qt.Assert(t, qt.IsNil(err))
Expand Down
12 changes: 6 additions & 6 deletions btf/ext_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,8 +421,8 @@ func (fi *FuncOffset) marshal(w *bytes.Buffer, b *Builder) error {
TypeID: id,
}
buf := make([]byte, FuncInfoSize)
internal.NativeEndian.PutUint32(buf, bfi.InsnOff)
internal.NativeEndian.PutUint32(buf[4:], uint32(bfi.TypeID))
binary.NativeEndian.PutUint32(buf, bfi.InsnOff)
binary.NativeEndian.PutUint32(buf[4:], uint32(bfi.TypeID))
_, err = w.Write(buf)
return err
}
Expand Down Expand Up @@ -627,10 +627,10 @@ func (li *LineOffset) marshal(w *bytes.Buffer, b *Builder) error {
}

buf := make([]byte, LineInfoSize)
internal.NativeEndian.PutUint32(buf, bli.InsnOff)
internal.NativeEndian.PutUint32(buf[4:], bli.FileNameOff)
internal.NativeEndian.PutUint32(buf[8:], bli.LineOff)
internal.NativeEndian.PutUint32(buf[12:], bli.LineCol)
binary.NativeEndian.PutUint32(buf, bli.InsnOff)
binary.NativeEndian.PutUint32(buf[4:], bli.FileNameOff)
binary.NativeEndian.PutUint32(buf[8:], bli.LineOff)
binary.NativeEndian.PutUint32(buf[12:], bli.LineCol)
_, err = w.Write(buf)
return err
}
Expand Down
10 changes: 4 additions & 6 deletions btf/ext_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"strings"
"testing"

"github.com/cilium/ebpf/internal"

"github.com/go-quicktest/qt"
)

Expand All @@ -18,11 +16,11 @@ func TestParseExtInfoBigRecordSize(t *testing.T) {
t.Fatal(err)
}

if _, err := parseFuncInfos(rd, internal.NativeEndian, table); err == nil {
if _, err := parseFuncInfos(rd, binary.NativeEndian, table); err == nil {
t.Error("Parsing func info with large record size doesn't return an error")
}

if _, err := parseLineInfos(rd, internal.NativeEndian, table); err == nil {
if _, err := parseLineInfos(rd, binary.NativeEndian, table); err == nil {
t.Error("Parsing line info with large record size doesn't return an error")
}
}
Expand All @@ -36,7 +34,7 @@ func BenchmarkParseLineInfoRecords(b *testing.B) {
b.ResetTimer()

for i := 0; i < b.N; i++ {
parseLineInfoRecords(bytes.NewReader(buf), internal.NativeEndian, size, count, true)
parseLineInfoRecords(bytes.NewReader(buf), binary.NativeEndian, size, count, true)
}
}

Expand All @@ -46,7 +44,7 @@ func TestParseLineInfoRecordsAllocations(t *testing.T) {
buf := make([]byte, size*count)

allocs := testing.AllocsPerRun(5, func() {
parseLineInfoRecords(bytes.NewReader(buf), internal.NativeEndian, size, count, true)
parseLineInfoRecords(bytes.NewReader(buf), binary.NativeEndian, size, count, true)
})

// 7 is the number of allocations on go 1.22
Expand Down
10 changes: 4 additions & 6 deletions btf/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@ import (
"fmt"
"io"
"testing"

"github.com/cilium/ebpf/internal"
)

func FuzzSpec(f *testing.F) {
var buf bytes.Buffer
err := binary.Write(&buf, internal.NativeEndian, &btfHeader{
err := binary.Write(&buf, binary.NativeEndian, &btfHeader{
Magic: btfMagic,
Version: 1,
HdrLen: uint32(binary.Size(btfHeader{})),
Expand All @@ -26,7 +24,7 @@ func FuzzSpec(f *testing.F) {
t.Skip("data is too short")
}

spec, err := loadRawSpec(bytes.NewReader(data), internal.NativeEndian, nil)
spec, err := loadRawSpec(bytes.NewReader(data), binary.NativeEndian, nil)
if err != nil {
if spec != nil {
t.Fatal("spec is not nil")
Expand All @@ -47,7 +45,7 @@ func FuzzSpec(f *testing.F) {

func FuzzExtInfo(f *testing.F) {
var buf bytes.Buffer
err := binary.Write(&buf, internal.NativeEndian, &btfExtHeader{
err := binary.Write(&buf, binary.NativeEndian, &btfExtHeader{
Magic: btfMagic,
Version: 1,
HdrLen: uint32(binary.Size(btfExtHeader{})),
Expand All @@ -70,7 +68,7 @@ func FuzzExtInfo(f *testing.F) {
emptySpec := specFromTypes(t, nil)
emptySpec.strings = table

info, err := loadExtInfos(bytes.NewReader(data), internal.NativeEndian, emptySpec)
info, err := loadExtInfos(bytes.NewReader(data), binary.NativeEndian, emptySpec)
if err != nil {
if info != nil {
t.Fatal("info is not nil")
Expand Down
3 changes: 2 additions & 1 deletion btf/handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package btf

import (
"bytes"
"encoding/binary"
"errors"
"fmt"
"math"
Expand Down Expand Up @@ -143,7 +144,7 @@ func (h *Handle) Spec(base *Spec) (*Spec, error) {
return nil, fmt.Errorf("missing base types")
}

return loadRawSpec(bytes.NewReader(btfBuffer), internal.NativeEndian, base)
return loadRawSpec(bytes.NewReader(btfBuffer), binary.NativeEndian, base)
}

// Close destroys the handle.
Expand Down
5 changes: 3 additions & 2 deletions btf/kernel.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package btf

import (
"encoding/binary"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -97,7 +98,7 @@ func loadKernelSpec() (_ *Spec, fallback bool, _ error) {
if err == nil {
defer fh.Close()

spec, err := loadRawSpec(fh, internal.NativeEndian, nil)
spec, err := loadRawSpec(fh, binary.NativeEndian, nil)
return spec, false, err
}

Expand All @@ -123,7 +124,7 @@ func loadKernelModuleSpec(module string, base *Spec) (*Spec, error) {
}
defer fh.Close()

return loadRawSpec(fh, internal.NativeEndian, base)
return loadRawSpec(fh, binary.NativeEndian, base)
}

// findVMLinux scans multiple well-known paths for vmlinux kernel images.
Expand Down
4 changes: 2 additions & 2 deletions btf/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type MarshalOptions struct {
// KernelMarshalOptions will generate BTF suitable for the current kernel.
func KernelMarshalOptions() *MarshalOptions {
return &MarshalOptions{
Order: internal.NativeEndian,
Order: binary.NativeEndian,
StripFuncLinkage: haveFuncLinkage() != nil,
ReplaceDeclTags: haveDeclTags() != nil,
ReplaceTypeTags: haveTypeTags() != nil,
Expand Down Expand Up @@ -160,7 +160,7 @@ func (b *Builder) Marshal(buf []byte, opts *MarshalOptions) ([]byte, error) {
}

if opts == nil {
opts = &MarshalOptions{Order: internal.NativeEndian}
opts = &MarshalOptions{Order: binary.NativeEndian}
}

// Reserve space for the BTF header.
Expand Down
Loading

0 comments on commit b802ad6

Please sign in to comment.