Add `golang` bindings to `libbcc` functions #4449

eiffel-fl · 2023-01-24T15:25:55Z

Hi.

This PR moves the golang bindings from iovisor/gobpf to this repository.
I slightly modified the files themselves but they provide the same features they did in their original repository.

You can test it by trying the golang example of execsnoop:

$ cd examples/golang/execsnoop
$ go build ./...
...
$ sudo ./execsnoop
# Some warnings messages are printed about values being defined several times.
...
include/linux/compiler-clang.h:43:9: warning: '__HAVE_BUILTIN_BSWAP16__' macro redefined [-Wmacro-redefined]
#define __HAVE_BUILTIN_BSWAP16__
        ^
<command line>:3:9: note: previous definition is here
#define __HAVE_BUILTIN_BSWAP16__ 1
        ^
3 warnings generated
PCOMM            PID    PPID   RET ARGS
ls               33188  10404    0 /bin/ls
ls               33189  10404    0 /bin/ls
ls               33190  10404    0 /bin/ls -al

The goal of this PR is to move these files here so it will be easier to maintain them as iovisor/gobpf is slightly maintained as stated in iovisor/gobpf#304.

If you see any way to improve this PR, feel free to share (@alban, @yaxiong-zhao)!

Best regards and thank you in advance.

nascentcore-eng · 2023-01-25T16:16:02Z

Are there tests to ensure the APIs are consistent with BCC C&C++ APIs?

At least this PR does not have such tests.
Questions to answer:

Is there such tests in iovisor/gobpf? I dont think so
What's the state of testing for python & lua binding of BCC in iovisor/bcc (this repo)?

eiffel-fl · 2023-01-25T16:39:42Z

Are there tests to ensure the APIs are consistent with BCC C&C++ APIs?

Up to my knowledge, sadly no.

At least this PR does not have such tests. Questions to answer:
1. Is there such tests in iovisor/gobpf? I dont think so

Actually there were some integration tests in bpf_test.go.
I can take a look at this file and import it here to cover only the bcc parts (i.e. not the elf one).

2. What's the state of testing for python & lua binding of BCC in iovisor/bcc (this repo)?

It seems, at least, the LUA one are tested.

eiffel-fl · 2023-01-25T18:06:56Z

I normally transferred the existing tests in gobpf to this PR.
I am not sure they will be run in the CI, as go is not installed in any of the container, but I think this is the same for lua.

nascentcore-eng · 2023-01-31T09:07:56Z

@yonghong-song @chenhengqi
Or some other BCC maintainer, what's your take on this pull request?

src/golang/module.go

yonghong-song · 2023-02-03T22:24:50Z

Have you looked at other alternatives like libbpf-go: https://github.com/aquasecurity/libbpfgo which seems more actively than bcc-golang? cilium also has go binding (https://github.com/cilium/ebpf) and maybe worthwhile to take a look as well.

If bcc/golang is preferred approach, I am okay to merge it into bcc repo since this makes development easier (similar to python/lua). But @eiffel-fl @yaxiong-zhao you guys might need step up to do related code reviews as @davemarchevsky and I am not an expert in go. Not sure whether @chenhengqi is familiar with go or not.

I checked the test result. The 'go' test is not triggered. The container needs to be updated to download the go package. cc @davemarchevsky.

nascentcore-eng · 2023-02-04T01:14:06Z

Have you looked at other alternatives like libbpf-go: https://github.com/aquasecurity/libbpfgo which seems more actively than bcc-golang? cilium also has go binding (https://github.com/cilium/ebpf) and maybe worthwhile to take a look as well.

If bcc/golang is preferred approach, I am okay to merge it into bcc repo since this makes development easier (similar to python/lua). But @eiffel-fl @yaxiong-zhao you guys might need step up to do related code reviews as @davemarchevsky and I am not an expert in go. Not sure whether @chenhengqi is familiar with go or not.

I checked the test result. The 'go' test is not triggered. The container needs to be updated to download the go package. cc @davemarchevsky.

I can verify that this go binding code is same as iovisor/gobpf repo, and they are mostly in decent state.

We have found a few issues in this code in our own use, and would later contribute the fixes back after this PR is merged. But will not ask for changes in this PR.

In general we'll not ask for revision on the code in this PR to avoid dragging the process. Since this PR's main objective of improving BCC go binding's promptness of maintenance, is well understood and meaningful.

I think the current maintainer of iovisor/BCC should provide strick guidance on the CI CD requirements and make sure such requirements are met before merging. Like making sure enough tests are added, and triggered properly.

yonghong-song · 2023-02-04T04:36:06Z

@yaxiong-zhao We will add 'go' package to the test distro and do the 'golang' test before the merge.

eiffel-fl · 2023-02-04T07:20:54Z

But @eiffel-fl @yaxiong-zhao you guys might need step up to do related code reviews

I will do my best for it as I totally understand your point of view and I do not want to add extra work on you and other iovisor/bcc maintainers by merging this!

The container needs to be updated to download the go package.

I will take a look at it!

chenhengqi · 2023-02-06T15:36:37Z

I am OK with this change, will test it locally.

chenhengqi

I think the golang module name should be succinct.
Personally, I perfer just github.com/iovisor/bcc

From 578199dc774a860469bec7a5d5b9c6e0f32439c5 Mon Sep 17 00:00:00 2001
From: Hengqi Chen <[email protected]>
Date: Tue, 7 Feb 2023 18:12:24 +0800
Subject: [PATCH] bcc/golang: Rename package to github.com/iovisor/bcc

Signed-off-by: Hengqi Chen <[email protected]>
---
 examples/golang/execsnoop/execsnoop.go    | 2 +-
 examples/golang/execsnoop/go.mod          | 4 ++--
 examples/golang/execsnoop/output.go       | 4 ++--
 src/golang/go.mod                         | 3 +++
 src/golang/module.go                      | 3 +--
 src/golang/perf.go                        | 3 ++-
 src/golang/pkg/cpuonline/cpuonline.go     | 4 ++--
 src/golang/pkg/cpupossible/cpupossible.go | 4 ++--
 src/golang/pkg/cpurange/cpu_range.go      | 2 +-
 src/golang/pkg/cpurange/cpu_range_test.go | 2 +-
 src/golang/table.go                       | 2 +-
 11 files changed, 18 insertions(+), 15 deletions(-)
 create mode 100644 src/golang/go.mod

diff --git a/examples/golang/execsnoop/execsnoop.go b/examples/golang/execsnoop/execsnoop.go
index 18ad63a8..ee756196 100644
--- a/examples/golang/execsnoop/execsnoop.go
+++ b/examples/golang/execsnoop/execsnoop.go
@@ -24,7 +24,7 @@ import (
 	"strings"
 	"unsafe"
 
-	bpf "github.com/iovisor/bcc/src/golang"
+	bpf "github.com/iovisor/bcc"
 )
 
 import "C"
diff --git a/examples/golang/execsnoop/go.mod b/examples/golang/execsnoop/go.mod
index 1f56b111..b687638d 100644
--- a/examples/golang/execsnoop/go.mod
+++ b/examples/golang/execsnoop/go.mod
@@ -3,7 +3,7 @@ module github.com/iovisor/bcc/examples/golang/execsnoop
 go 1.18
 
 require (
-	github.com/iovisor/bcc/src/golang v1.0.0
+	github.com/iovisor/bcc v1.0.0
 )
 
-replace github.com/iovisor/bcc/src/golang => ../../../src/golang
+replace github.com/iovisor/bcc => ../../../src/golang
diff --git a/examples/golang/execsnoop/output.go b/examples/golang/execsnoop/output.go
index a6099102..d7c7a7ab 100644
--- a/examples/golang/execsnoop/output.go
+++ b/examples/golang/execsnoop/output.go
@@ -37,7 +37,7 @@ type tableOutput struct {
 }
 
 func (t tableOutput) PrintHeader() {
-	header := "%-16s %-6s %-6s %3s %s\n"
+	header := "%-16s %-7s %-7s %3s %s\n"
 	args := []interface{}{"PCOMM", "PID", "PPID", "RET", "ARGS"}
 	if t.timestamp {
 		header = "%-8s" + header
@@ -47,7 +47,7 @@ func (t tableOutput) PrintHeader() {
 }
 
 func (t tableOutput) PrintLine(e eventPayload) {
-	header := "%-16s %-6d %-6s %3d %s\n"
+	header := "%-16s %-7d %-7s %3d %s\n"
 	args := []interface{}{e.Comm, e.Pid, e.Ppid, e.RetVal, e.Argv}
 	if t.timestamp {
 		header = "%-8.3f" + header
diff --git a/src/golang/go.mod b/src/golang/go.mod
new file mode 100644
index 00000000..51f9471f
--- /dev/null
+++ b/src/golang/go.mod
@@ -0,0 +1,3 @@
+module github.com/iovisor/bcc
+
+go 1.19
diff --git a/src/golang/module.go b/src/golang/module.go
index 5e0a4963..3fe3e9e6 100644
--- a/src/golang/module.go
+++ b/src/golang/module.go
@@ -24,7 +24,7 @@ import (
 	"syscall"
 	"unsafe"
 
-	"github.com/iovisor/bcc/src/golang/pkg/cpuonline"
+	"github.com/iovisor/bcc/pkg/cpuonline"
 )
 
 // If you want to compile golang bindings with a specific version of libbcc, you
@@ -39,7 +39,6 @@ import (
 //    cd examples/golang/execsnoop
 //    CGO_CPPFLAGS="-DLOCAL=1 -I$PWD/../../../src/cc/ -I$PWD/../../../build/src/cc/" CGO_LDFLAGS="-L$PWD/../../../build/src/cc/ -lbcc" PKG_CONFIG_PATH=$PWD/../../../build/src/cc/ go build
 
-
 /*
 #cgo pkg-config: libbcc
 #ifdef LOCAL
diff --git a/src/golang/perf.go b/src/golang/perf.go
index 7469fa7d..d54577e1 100644
--- a/src/golang/perf.go
+++ b/src/golang/perf.go
@@ -21,7 +21,7 @@ import (
 	"sync"
 	"unsafe"
 
-	"github.com/iovisor/bcc/src/golang/pkg/cpuonline"
+	"github.com/iovisor/bcc/pkg/cpuonline"
 )
 
 /*
@@ -94,6 +94,7 @@ func lookupCallback(i uint64) *callbackData {
 // be written. This is because we can't take the address of a Go
 // function and give that to C-code since the cgo tool will generate a
 // stub in C that should be called."
+//
 //export rawCallback
 func rawCallback(cbCookie unsafe.Pointer, raw unsafe.Pointer, rawSize C.int) {
 	callbackData := lookupCallback(uint64(uintptr(cbCookie)))
diff --git a/src/golang/pkg/cpuonline/cpuonline.go b/src/golang/pkg/cpuonline/cpuonline.go
index 20e6bb6f..69be8cf0 100644
--- a/src/golang/pkg/cpuonline/cpuonline.go
+++ b/src/golang/pkg/cpuonline/cpuonline.go
@@ -1,9 +1,9 @@
-package cpuonline
+package cpuonline // import "github.com/iovisor/bcc/pkg/cpuonline"
 
 import (
 	"io/ioutil"
 
-	"github.com/iovisor/bcc/src/golang/pkg/cpurange"
+	"github.com/iovisor/bcc/pkg/cpurange"
 )
 
 const cpuOnline = "/sys/devices/system/cpu/online"
diff --git a/src/golang/pkg/cpupossible/cpupossible.go b/src/golang/pkg/cpupossible/cpupossible.go
index 6c73a283..376cc3ac 100644
--- a/src/golang/pkg/cpupossible/cpupossible.go
+++ b/src/golang/pkg/cpupossible/cpupossible.go
@@ -1,9 +1,9 @@
-package cpupossible
+package cpupossible // import "github.com/iovisor/bcc/pkg/cpupossible"
 
 import (
 	"io/ioutil"
 
-	"github.com/iovisor/bcc/src/golang/pkg/cpurange"
+	"github.com/iovisor/bcc/pkg/cpurange"
 )
 
 const cpuPossible = "/sys/devices/system/cpu/possible"
diff --git a/src/golang/pkg/cpurange/cpu_range.go b/src/golang/pkg/cpurange/cpu_range.go
index 30319313..da6a5790 100644
--- a/src/golang/pkg/cpurange/cpu_range.go
+++ b/src/golang/pkg/cpurange/cpu_range.go
@@ -1,4 +1,4 @@
-package cpurange
+package cpurange // import "github.com/iovisor/bcc/pkg/cpurange"
 
 import (
 	"strconv"
diff --git a/src/golang/pkg/cpurange/cpu_range_test.go b/src/golang/pkg/cpurange/cpu_range_test.go
index d9040cf8..02067018 100644
--- a/src/golang/pkg/cpurange/cpu_range_test.go
+++ b/src/golang/pkg/cpurange/cpu_range_test.go
@@ -1,4 +1,4 @@
-package cpurange
+package cpurange // import "github.com/iovisor/bcc/pkg/cpurange"
 
 import (
 	"testing"
diff --git a/src/golang/table.go b/src/golang/table.go
index c056851c..d86240a9 100644
--- a/src/golang/table.go
+++ b/src/golang/table.go
@@ -22,7 +22,7 @@ import (
 	"os"
 	"unsafe"
 
-	"github.com/iovisor/bcc/src/golang/pkg/cpupossible"
+	"github.com/iovisor/bcc/pkg/cpupossible"
 )
 
 /*
-- 
2.31.1

nascentcore-eng · 2023-02-07T10:24:54Z

I think the golang module name should be succinct. Personally, I perfer just github.com/iovisor/bcc

github.com/iovisor/bcc
+1

tests/golang/CMakeLists.txt

These bindings were taken from iovisor/gobpf as they were at time of commit 0e5464f03ed2 ("[bcc] Support for new `bcc_func_load` signature."). Signed-off-by: Francis Laniel <[email protected]>

Signed-off-by: Francis Laniel <[email protected]>

It was taken from iovisor/gobpf at time of commit 0af20fb29fcb ("add lost events channel"). Signed-off-by: Francis Laniel <[email protected]>

Signed-off-by: Francis Laniel <[email protected]>

This file was taken from gobpf as it was at time of commit f718482cf42f ("Fix compilation in older kernels"). Signed-off-by: Francis Laniel <[email protected]>

We are only interested in testing bcc. Signed-off-by: Francis Laniel <[email protected]>

Signed-off-by: Francis Laniel <[email protected]>

eiffel-fl · 2023-02-13T15:48:40Z

Personally, I perfer just github.com/iovisor/bcc

I agree too!
Thank you for the patch!

In order to support properly testing ongoing work to incorporate `iovisor/gobpf` code into this repo (#4449), we need to be able to test the bindings in this repository. So try adding `golang-go` to the ubuntu container. Note that we can't use `actions/setup-go@v2` GH action used by `gobpf`'s CI as we're running tests within containers on the CI, not directly on the CI. Marking [DRAFT] for now as I haven't touched the dockerfiles in a while and probably forgot / am misunderstanding something.

eiffel-fl requested review from drzaeus77, goldshtn, yonghong-song, 4ast, brendangregg and davemarchevsky as code owners January 24, 2023 15:25

eiffel-fl force-pushed the francis/golang-bindings branch from e9f9eb4 to 52cacae Compare January 25, 2023 18:04

chenhengqi self-assigned this Jan 31, 2023

chenhengqi reviewed Jan 31, 2023

View reviewed changes

src/golang/module.go Outdated Show resolved Hide resolved

eiffel-fl force-pushed the francis/golang-bindings branch from 52cacae to cb08891 Compare February 1, 2023 11:48

chenhengqi reviewed Feb 7, 2023

View reviewed changes

tests/golang/CMakeLists.txt Show resolved Hide resolved

eiffel-fl added 8 commits February 13, 2023 16:32

Add golang bindings to bcc functions.

3c0b174

These bindings were taken from iovisor/gobpf as they were at time of commit 0e5464f03ed2 ("[bcc] Support for new `bcc_func_load` signature."). Signed-off-by: Francis Laniel <[email protected]>

golang: Permit using both local and installed libbcc.

a2e3011

Signed-off-by: Francis Laniel <[email protected]>

golang: Extend copyright headers.

ca2b90a

Signed-off-by: Francis Laniel <[email protected]>

examples: Add execsnoop using golang bindings.

0691698

It was taken from iovisor/gobpf at time of commit 0af20fb29fcb ("add lost events channel"). Signed-off-by: Francis Laniel <[email protected]>

examples: Enable golang execsnoop.

e7395da

Signed-off-by: Francis Laniel <[email protected]>

tests: Add bpf_test to test golang bindings.

b5b7baa

This file was taken from gobpf as it was at time of commit f718482cf42f ("Fix compilation in older kernels"). Signed-off-by: Francis Laniel <[email protected]>

tests: Remove tests for elf module in golang.

547a495

We are only interested in testing bcc. Signed-off-by: Francis Laniel <[email protected]>

test: Enable golang tests.

8ca4dfc

Signed-off-by: Francis Laniel <[email protected]>

eiffel-fl force-pushed the francis/golang-bindings branch from cb08891 to 8ca4dfc Compare February 13, 2023 15:48

davemarchevsky mentioned this pull request Feb 14, 2023

[DRAFT] Add golang to ubuntu dockerfiles #4481

Open

iaguis mentioned this pull request Oct 31, 2023

add module.GetSymbolByAddr / GetDemangleSymbolByAddr iovisor/gobpf#232

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `golang` bindings to `libbcc` functions #4449

Add `golang` bindings to `libbcc` functions #4449

eiffel-fl commented Jan 24, 2023

nascentcore-eng commented Jan 25, 2023

eiffel-fl commented Jan 25, 2023

eiffel-fl commented Jan 25, 2023

nascentcore-eng commented Jan 31, 2023

yonghong-song commented Feb 3, 2023

nascentcore-eng commented Feb 4, 2023

yonghong-song commented Feb 4, 2023

eiffel-fl commented Feb 4, 2023 •

edited

Loading

chenhengqi commented Feb 6, 2023

chenhengqi left a comment

nascentcore-eng commented Feb 7, 2023

eiffel-fl commented Feb 13, 2023

Add golang bindings to libbcc functions #4449

Are you sure you want to change the base?

Add golang bindings to libbcc functions #4449

Conversation

eiffel-fl commented Jan 24, 2023

nascentcore-eng commented Jan 25, 2023

eiffel-fl commented Jan 25, 2023

eiffel-fl commented Jan 25, 2023

nascentcore-eng commented Jan 31, 2023

yonghong-song commented Feb 3, 2023

nascentcore-eng commented Feb 4, 2023

yonghong-song commented Feb 4, 2023

eiffel-fl commented Feb 4, 2023 • edited Loading

chenhengqi commented Feb 6, 2023

chenhengqi left a comment

Choose a reason for hiding this comment

nascentcore-eng commented Feb 7, 2023

eiffel-fl commented Feb 13, 2023

Add `golang` bindings to `libbcc` functions #4449

Add `golang` bindings to `libbcc` functions #4449

eiffel-fl commented Feb 4, 2023 •

edited

Loading