Repository: cilium/ebpf
Pull request #770 (2022-08-29 13:30:11)

bpf2go: add flag for alternative filename stem
In our project, the eBPF source files are named with underscores, like `ext_vx_decap.c`. We want to have the .go and .o files generated by bpf2go to follow the same naming scheme (e.g. `ext_vx_decap_bpfel.go`). But currently, when setting the `<ident>` parameter to `ext_vx_decap`, the result will be some broken Go type names like `type ext_vx_decapPrograms`

So instead, this PR adds a new `-filename-stem` flag, which allows specifying an alternative filename prefix while one can keep the `ident` as it is supposed to.
The `-filename-stem` flag is optional and if it's not set, the `ident` will be used instead. So this is not a breaking change.

Sample usage:
```
bpf2go [...] -filename-stem ext_vx_decap ExtVxDecap src/ext_vx_decap.c
```

lmb: To clarify, you want your Go types to be `ExtVxDecap...` but the output to be called `ext_vx_decap_bpfel.go`?
MarcusWichelmann: Exactly.
MarcusWichelmann: Thanks for the review!
An automatic transformation of the `ident` would also be fine, but will be hard to get right in cases like uppercase abbreviations as often seen (per convention) in Go type names. e.g. `ExtIPV4Firewall -> ext_ipv4_firwall`.
Of course, this is also algorithmically solvable, but there might still be cases where the algorithm will get it wrong and then the developer has no way to fix it manually. e.g. `SFlowSomething -> sflow_something (not s_flow_something)`
 So a flag does probably provide better control.

But I'll work on your suggestions and update this PR.
lmb: I had a look around, and there are at least a couple open source cases that would break: https://sourcegraph.com/search?q=context:global+go:generate+.%2B%3F+bpf2go&patternType=regexp Let's stick to the flag.
MarcusWichelmann: Have done the requested changes now. I have squashed my changes, because the new commit changes everything from the previous commit.

I'm not sure, how you intended the usage of [`filepath.Clean`](https://pkg.go.dev/path/filepath#Clean), because that won't prevent one from using `-output-stem "dir/some_stem"`. Instead I rely on the absence of path separation chars (`/`).
MarcusWichelmann: I rebased this branch onto current master.

How do you think about this PR, can this be merged?

Patch:
diff --git a/cmd/bpf2go/main.go b/cmd/bpf2go/main.go
index 55a462d..03ff1f8 100644
--- a/cmd/bpf2go/main.go
+++ b/cmd/bpf2go/main.go
@@ -81,6 +81,7 @@ func run(stdout io.Writer, pkg, outputDir string, args []string) (err error) {
 	fs.StringVar(&b2g.makeBase, "makebase", "", "write make compatible depinfo files relative to `directory`")
 	fs.Var(&b2g.cTypes, "type", "`Name` of a type to generate a Go declaration for, may be repeated")
 	fs.BoolVar(&b2g.skipGlobalTypes, "no-global-types", false, "Skip generating types for map keys and values, etc.")
+	fs.StringVar(&b2g.outputStem, "output-stem", "", "alternative stem for names of generated files (defaults to ident)")
 
 	fs.SetOutput(stdout)
 	fs.Usage = func() {
@@ -152,6 +153,10 @@ func run(stdout io.Writer, pkg, outputDir string, args []string) (err error) {
 		}
 	}
 
+	if b2g.outputStem != "" && strings.ContainsRune(b2g.outputStem, filepath.Separator) {
+		return fmt.Errorf("-output-stem %q must not contain path separation characters", b2g.outputStem)
+	}
+
 	if strings.ContainsRune(b2g.tags, '\n') {
 		return fmt.Errorf("-tags mustn't contain new line characters")
 	}
@@ -240,6 +245,8 @@ type bpf2go struct {
 	sourceFile string
 	// Absolute path to a directory where .go are written
 	outputDir string
+	// Alternative output stem. If empty, ident is used.
+	outputStem string
 	// Valid go package name.
 	pkg string
 	// Valid go identifier.
@@ -269,9 +276,13 @@ func (b2g *bpf2go) convert(tgt target, arches []string) (err error) {
 		f.Close()
 	}
 
-	stem := fmt.Sprintf("%s_%s", strings.ToLower(b2g.ident), tgt.clang)
+	outputStem := b2g.outputStem
+	if outputStem == "" {
+		outputStem = strings.ToLower(b2g.ident)
+	}
+	stem := fmt.Sprintf("%s_%s", outputStem, tgt.clang)
 	if tgt.linux != "" {
-		stem = fmt.Sprintf("%s_%s_%s", strings.ToLower(b2g.ident), tgt.clang, tgt.linux)
+		stem = fmt.Sprintf("%s_%s_%s", outputStem, tgt.clang, tgt.linux)
 	}
 
 	objFileName := filepath.Join(b2g.outputDir, stem+".o")
diff --git a/features/prog.go b/features/prog.go
index f434ca0..a3a3a83 100644
--- a/features/prog.go
+++ b/features/prog.go
@@ -60,7 +60,7 @@ func createProgLoadAttr(pt ebpf.ProgramType, helper asm.BuiltinFunc) (*sys.ProgL
 	instructions := sys.NewSlicePointer(bytecode)
 
 	// Some programs have expected attach types which are checked during the
-	// BPD_PROG_LOAD syscall.
+	// BPF_PROG_LOAD syscall.
 	switch pt {
 	case ebpf.CGroupSockAddr:
 		expectedAttachType = ebpf.AttachCGroupInet4Connect
diff --git a/go.mod b/go.mod
index cf7c7cb..5da855f 100644
--- a/go.mod
+++ b/go.mod
@@ -5,7 +5,7 @@ go 1.18
 require (
 	github.com/frankban/quicktest v1.14.0
 	github.com/google/go-cmp v0.5.6
-	golang.org/x/sys v0.0.0-20220829200755-d48e67d00261
+	golang.org/x/sys v0.0.0-20220927170352-d9d178bc13c6
 )
 
 require (
diff --git a/go.sum b/go.sum
index 087ab91..e2db3e9 100644
--- a/go.sum
+++ b/go.sum
@@ -12,8 +12,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k=
 github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
-golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 h1:v6hYoSR9T5oet+pMXwUWkbiVqx/63mlHjefrHmxwfeY=
-golang.org/x/sys v0.0.0-20220829200755-d48e67d00261/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220927170352-d9d178bc13c6 h1:cy1ko5847T/lJ45eyg/7uLprIE/amW5IXxGtEnQdYMI=
+golang.org/x/sys v0.0.0-20220927170352-d9d178bc13c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
diff --git a/internal/sys/signals.go b/internal/sys/signals.go
new file mode 100644
index 0000000..84d6331
--- /dev/null
+++ b/internal/sys/signals.go
@@ -0,0 +1,82 @@
+package sys
+
+import (
+	"fmt"
+	"runtime"
+	"unsafe"
+
+	"github.com/cilium/ebpf/internal/unix"
+)
+
+var profSet unix.Sigset_t
+
+func init() {
+	if err := sigsetAdd(&profSet, unix.SIGPROF); err != nil {
+		panic(fmt.Errorf("creating signal set: %w", err))
+	}
+}
+
+// maskProfilerSignal locks the calling goroutine to its underlying OS thread
+// and adds SIGPROF to the thread's signal mask. This prevents pprof from
+// interrupting expensive syscalls like e.g. BPF_PROG_LOAD.
+//
+// The caller must defer sys.UnmaskProfilerSignal() to reverse the operation.
+func maskProfilerSignal() {
+	runtime.LockOSThread()
+
+	if err := unix.PthreadSigmask(unix.SIG_BLOCK, &profSet, nil); err != nil {
+		runtime.UnlockOSThread()
+		panic(fmt.Errorf("masking profiler signal: %w", err))
+	}
+}
+
+// unmaskProfilerSignal removes SIGPROF from the underlying thread's signal
+// mask, allowing it to be interrupted for profiling once again.
+//
+// It also unlocks the current goroutine from its underlying OS thread.
+func unmaskProfilerSignal() {
+	defer runtime.UnlockOSThread()
+
+	if err := unix.PthreadSigmask(unix.SIG_UNBLOCK, &profSet, nil); err != nil {
+		panic(fmt.Errorf("unmasking profiler signal: %w", err))
+	}
+}
+
+const (
+	wordBytes = int(unsafe.Sizeof(unix.Sigset_t{}.Val[0]))
+	wordBits  = wordBytes * 8
+
+	setBytes = int(unsafe.Sizeof(unix.Sigset_t{}))
+	setBits  = setBytes * 8
+)
+
+// sigsetAdd adds signal to set.
+//
+// Note: Sigset_t.Val's value type is uint32 or uint64 depending on the arch.
+// This function must be able to deal with both and so must avoid any direct
+// references to u32 or u64 types.
+func sigsetAdd(set *unix.Sigset_t, signal unix.Signal) error {
+	if signal < 1 {
+		return fmt.Errorf("signal %d must be larger than 0", signal)
+	}
+	if int(signal) > setBits {
+		return fmt.Errorf("signal %d does not fit within unix.Sigset_t", signal)
+	}
+
+	// For amd64, runtime.sigaddset() performs the following operation:
+	// set[(signal-1)/32] |= 1 << ((uint32(signal) - 1) & 31)
+	//
+	// This trick depends on sigset being two u32's, causing a signal in the the
+	// bottom 31 bits to be written to the low word if bit 32 is low, or the high
+	// word if bit 32 is high.
+
+	// Signal is the nth bit in the bitfield.
+	bit := int(signal - 1)
+	// Word within the sigset the bit needs to be written to.
+	word := bit / wordBits
+
+	// Write the signal bit into its corresponding word at the corrected offset.
+	set.Val[word] |= 1 << (bit % wordBits)
+
+	return nil
+}
diff --git a/internal/sys/syscall.go b/internal/sys/syscall.go
index 6dde230..3205ede 100644
--- a/internal/sys/syscall.go
+++ b/internal/sys/syscall.go
@@ -17,6 +17,13 @@ var ENOTSUPP = syscall.Errno(524)
 //
 // Any pointers contained in attr must use the Pointer type from this package.
 func BPF(cmd Cmd, attr unsafe.Pointer, size uintptr) (uintptr, error) {
+	// Prevent the Go profiler from repeatedly interrupting the verifier,
+	// which could otherwise lead to a livelock due to receiving EAGAIN.
+	if cmd == BPF_PROG_LOAD {
+		maskProfilerSignal()
+		defer unmaskProfilerSignal()
+	}
+
 	for {
 		r1, _, errNo := unix.Syscall(unix.SYS_BPF, uintptr(cmd), uintptr(attr), size)
 		runtime.KeepAlive(attr)
diff --git a/internal/unix/signals_generic.go b/internal/unix/signals_generic.go
new file mode 100644
index 0000000..01b9d75
--- /dev/null
+++ b/internal/unix/signals_generic.go
@@ -0,0 +1,8 @@
+//go:build linux && !mips && !mipsle && !mips64 && !mips64le
+
+package unix
+
+const (
+	SIG_BLOCK   = 0
+	SIG_UNBLOCK = 1
+)
diff --git a/internal/unix/signals_mips.go b/internal/unix/signals_mips.go
new file mode 100644
index 0000000..9d86290
--- /dev/null
+++ b/internal/unix/signals_mips.go
@@ -0,0 +1,8 @@
+//go:build linux && (mips || mipsle || mips64 || mips64le)
+
+package unix
+
+const (
+	SIG_BLOCK   = 1
+	SIG_UNBLOCK = 2
+)
diff --git a/internal/unix/types_linux.go b/internal/unix/types_linux.go
index 9cb2537..2241634 100644
--- a/internal/unix/types_linux.go
+++ b/internal/unix/types_linux.go
@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package unix
 
@@ -70,21 +69,25 @@ const (
 	SO_ATTACH_BPF             = linux.SO_ATTACH_BPF
 	SO_DETACH_BPF             = linux.SO_DETACH_BPF
 	SOL_SOCKET                = linux.SOL_SOCKET
+	SIGPROF                   = linux.SIGPROF
 )
 
-// Statfs_t is a wrapper
 type Statfs_t = linux.Statfs_t
-
 type Stat_t = linux.Stat_t
-
-// Rlimit is a wrapper
 type Rlimit = linux.Rlimit
+type Signal = linux.Signal
+type Sigset_t = linux.Sigset_t
 
 // Syscall is a wrapper
 func Syscall(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err syscall.Errno) {
 	return linux.Syscall(trap, a1, a2, a3)
 }
 
+// PthreadSigmask is a wrapper
+func PthreadSigmask(how int, set, oldset *Sigset_t) error {
+	return linux.PthreadSigmask(how, set, oldset)
+}
+
 // FcntlInt is a wrapper
 func FcntlInt(fd uintptr, cmd, arg int) (int, error) {
 	return linux.FcntlInt(fd, cmd, arg)
diff --git a/internal/unix/types_other.go b/internal/unix/types_other.go
index be3fdeb..7ee0a74 100644
--- a/internal/unix/types_other.go
+++ b/internal/unix/types_other.go
@@ -1,5 +1,4 @@
 //go:build !linux
-// +build !linux
 
 package unix
 
@@ -72,9 +71,12 @@ const (
 	SO_ATTACH_BPF             = 0x32
 	SO_DETACH_BPF             = 0x1b
 	SOL_SOCKET                = 0x1
+	SIGPROF                   = 0
+
+	SIG_BLOCK   = 0
+	SIG_UNBLOCK = 0
 )
 
-// Statfs_t is a wrapper
 type Statfs_t struct {
 	Type    int64
 	Bsize   int64
@@ -92,17 +94,27 @@ type Statfs_t struct {
 
 type Stat_t struct{}
 
-// Rlimit is a wrapper
 type Rlimit struct {
 	Cur uint64
 	Max uint64
 }
 
+type Signal int
+
+type Sigset_t struct {
+	Val [4]uint64
+}
+
 // Syscall is a wrapper
 func Syscall(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err syscall.Errno) {
 	return 0, 0, syscall.Errno(1)
 }
 
+// PthreadSigmask is a wrapper
+func PthreadSigmask(how int, set, oldset *Sigset_t) error {
+	return errNonLinux
+}
+
 // FcntlInt is a wrapper
 func FcntlInt(fd uintptr, cmd, arg int) (int, error) {
 	return -1, errNonLinux


Test patch:
diff --git a/internal/sys/signals_test.go b/internal/sys/signals_test.go
new file mode 100644
index 0000000..ea59306
--- /dev/null
+++ b/internal/sys/signals_test.go
@@ -0,0 +1,61 @@
+package sys
+
+import (
+	"runtime"
+	"testing"
+
+	"github.com/cilium/ebpf/internal/unix"
+)
+
+func TestSigset(t *testing.T) {
+	// Type-infer a sigset word. This is a typed uint of 32 or 64 bits depending
+	// on the target architecture, so we can't use an untyped uint.
+	zero := unix.Sigset_t{}.Val[0]
+	words := len(unix.Sigset_t{}.Val)
+
+	var want, got unix.Sigset_t
+	// Flip the first bit of the first word.
+	if err := sigsetAdd(&got, 1); err != nil {
+		t.Fatal(err)
+	}
+	want.Val[0] = 1
+	if want != got {
+		t.Fatalf("expected first word to be 0x%x, got: 0x%x", want, got)
+	}
+
+	// And the last bit of the last word.
+	if err := sigsetAdd(&got, unix.Signal(setBits)); err != nil {
+		t.Fatal(err)
+	}
+	want.Val[words-1] = ^(^zero >> 1)
+	if want != got {
+		t.Fatalf("expected last word to be 0x%x, got: 0x%x", want, got)
+	}
+
+	if err := sigsetAdd(&got, unix.Signal(setBits+1)); err == nil {
+		t.Fatal("expected out-of-bounds add to be rejected")
+	}
+	if err := sigsetAdd(&got, -1); err == nil {
+		t.Fatal("expected negative signal to be rejected")
+	}
+}
+
+func TestProfilerSignal(t *testing.T) {
+	// Additional goroutine lock to make the PthreadSigmask below execute on the
+	// same OS thread as the functions under test. UnlockOSThread needs to be
+	// called as many times as LockOSThread to unlock the goroutine.
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
+	maskProfilerSignal()
+	unmaskProfilerSignal()
+
+	var old unix.Sigset_t
+	if err := unix.PthreadSigmask(0, nil, &old); err != nil {
+		t.Fatal("getting old sigmask:", err)
+	}
+	var want unix.Sigset_t
+	if old != want {
+		t.Fatal("unmask operation didn't result in empty signal mask")
+	}
+}