Repository: cilium/ebpf
Pull request #546 (2022-01-21 10:53:01)

map: make Map.Extra field a pointer to a bytes.Reader
This avoids allocating a bytes.Reader for every MapSpec, as its presence is rather uncommon.

This is a breaking change that needs to be documented in the release.


Patch:
diff --git a/elf_reader.go b/elf_reader.go
index f1111ed..ac4496b 100644
--- a/elf_reader.go
+++ b/elf_reader.go
@@ -636,7 +636,7 @@ func (ec *elfCode) loadMaps(maps map[string]*MapSpec) error {
 				return fmt.Errorf("map %s: reading map tail: %w", mapName, err)
 			}
 			if len(extra) > 0 {
-				spec.Extra = *bytes.NewReader(extra)
+				spec.Extra = bytes.NewReader(extra)
 			}
 
 			if err := spec.clampPerfEventArraySize(); err != nil {
diff --git a/map.go b/map.go
index e95caec..b49b401 100644
--- a/map.go
+++ b/map.go
@@ -72,9 +72,9 @@ type MapSpec struct {
 	InnerMap *MapSpec
 
 	// Extra trailing bytes found in the ELF map definition when using structs
-	// larger than libbpf's bpf_map_def. Must be empty before instantiating
-	// the MapSpec into a Map.
-	Extra bytes.Reader
+	// larger than libbpf's bpf_map_def. nil if no trailing bytes were present.
+	// Must be nil or empty before instantiating the MapSpec into a Map.
+	Extra *bytes.Reader
 
 	// The BTF associated with this map.
 	BTF *btf.Map
@@ -322,8 +322,10 @@ func (spec *MapSpec) createMap(inner *sys.FD, opts MapOptions, handles *handleCa
 	// additional 'inner_map_idx' and later 'numa_node' fields.
 	// In order to support loading these definitions, tolerate the presence of
 	// extra bytes, but require them to be zeroes.
-	if _, err := io.Copy(internal.DiscardZeroes{}, &spec.Extra); err != nil {
-		return nil, errors.New("extra contains unhandled non-zero bytes, drain before creating map")
+	if spec.Extra != nil {
+		if _, err := io.Copy(internal.DiscardZeroes{}, spec.Extra); err != nil {
+			return nil, errors.New("extra contains unhandled non-zero bytes, drain before creating map")
+		}
 	}
 
 	switch spec.Type {


Test patch:
diff --git a/elf_reader_test.go b/elf_reader_test.go
index df0e6e3..f2c9daa 100644
--- a/elf_reader_test.go
+++ b/elf_reader_test.go
@@ -6,7 +6,6 @@ import (
 	"errors"
 	"flag"
 	"fmt"
-	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -558,14 +557,18 @@ func TestIPRoute2Compat(t *testing.T) {
 
 		var id, pinning, innerID, innerIndex uint32
 
+		if ms.Extra == nil {
+			t.Fatal("missing extra bytes")
+		}
+
 		switch {
-		case binary.Read(&ms.Extra, spec.ByteOrder, &id) != nil:
+		case binary.Read(ms.Extra, spec.ByteOrder, &id) != nil:
 			t.Fatal("missing id")
-		case binary.Read(&ms.Extra, spec.ByteOrder, &pinning) != nil:
+		case binary.Read(ms.Extra, spec.ByteOrder, &pinning) != nil:
 			t.Fatal("missing pinning")
-		case binary.Read(&ms.Extra, spec.ByteOrder, &innerID) != nil:
+		case binary.Read(ms.Extra, spec.ByteOrder, &innerID) != nil:
 			t.Fatal("missing inner_id")
-		case binary.Read(&ms.Extra, spec.ByteOrder, &innerIndex) != nil:
+		case binary.Read(ms.Extra, spec.ByteOrder, &innerIndex) != nil:
 			t.Fatal("missing inner_idx")
 		}
 
@@ -677,10 +680,10 @@ func TestLibBPFCompat(t *testing.T) {
 		case "test_sk_assign.o":
 			// Test contains a legacy iproute2 bpf_elf_map definition.
 			for _, m := range spec.Maps {
-				if m.Extra.Len() == 0 {
+				if m.Extra == nil || m.Extra.Len() == 0 {
 					t.Fatalf("Expected extra bytes in map %s", m.Name)
 				}
-				_, _ = io.Copy(io.Discard, &m.Extra)
+				m.Extra = nil
 			}
 		}