Add missing API to utilize map of maps #343

geyslan · 2023-07-20T21:56:01Z

Close: #93
Context: #213

commit 108b24a
Author: Geyslan Gregório geyslan@gmail.com
Date: Mon Jul 24 14:06:23 2023 -0300

chore: improve create-map selftest

commit 7c0bb3e
Author: Geyslan Gregório geyslan@gmail.com
Date: Mon Jul 24 13:46:11 2023 -0300

feat: add missing API to utilize map of maps

In this commit, the libbpfgo.go file undergoes significant changes,
primarily focused on introducing and updating functions related to BPF
maps. The main feature added is the capability to utilize "map of maps",
which involves creating, managing, and interacting with nested maps.

Added Functions:

    Reload
    GetMapByID
    GetBTFFDByID
    SetInnerMap
    MaxEntries (Note: GetMaxEntries is deprecated)

Updated Functions (to deal with maps returned by CreateMap):

    CreateMap
    Name
    Type
    SetType
    KeySize
    ValueSize
    SetValueSize
    Pin
    Unpin
    SetPinPath
    PinPath
    IsPinned
    FileDescriptor
    GetValue
    GetValueFlags
    GetValueReadInto
    Update
    Resize
    GetValueBatch
    GetValueAndDeleteBatch

Miscellaneous:

Aside from the map-related features, this commit also includes various
minor updates and improves error handling, all of which contribute to
the overall robustness of the library.

Co-authored-by: Kemal Akkoyun <kakkoyun@gmail.com>

commit 470b7d2
Author: Geyslan Gregório geyslan@gmail.com
Date: Mon Jul 24 12:24:42 2023 -0300

fix: fix selftests for arm64

Despite the fact that selftests were handling the correct function
names for arm64, some are still not working due to the fact that arm64
still does not support generic attaching `bpf_program__attach()`.

```
libbpf: prog 'mmap_fentry': failed to attach: ERROR: strerror_r(-524)=22
failed to attach program: errno 524
[!] ERROR: test error
make[1]: *** [Makefile:81: run-static] Error 4
make[1]: Leaving directory '/vagrant/selftest/percpu'
```

`#define ENOTSUPP       524     /* Operation is not supported */`

commit f061588
Author: Geyslan Gregório geyslan@gmail.com
Date: Mon Jul 24 12:22:07 2023 -0300

chore: increase timeout for selftests

geyslan · 2023-07-20T21:58:17Z

I just rebased the conflicted PR. Next, I'll fix any pending issue and bring in some local implementations related to this effort.

libbpfgo.go

selftest/map-of-maps/main.go

geyslan · 2023-07-24T17:23:08Z

@rafaeldtinoco Tracee local tests were green.

geyslan · 2023-07-24T19:58:46Z

The commit 470b7d2 only fixed a part of the selftests (on arm64). See new errors (operation not supported):

#define ENOTSUPP	524	/* Operation is not supported */

https://elixir.bootlin.com/linux/latest/source/include/linux/errno.h#L27

libbpf: map 1 is "main.rodata"
libbpf: sec '.relfentry/__arm64_sys_mmap': collecting relocation for section(3) 'fentry/__arm64_sys_mmap'
libbpf: sec '.relfentry/__arm64_sys_mmap': relo #0: insn #4 against 'percpu_hash'
libbpf: prog 'mmap_fentry': found map 0 (percpu_hash, sec 5, off 0) for insn #4
libbpf: sec '.relfentry/__arm64_sys_mmap': relo #1: insn #12 against '.rodata'
libbpf: prog 'mmap_fentry': found data map 1 (main.rodata, sec 6, off 0) for insn 12
libbpf: sec '.relfentry/__arm64_sys_mmap': relo #2: insn #17 against '.rodata'
libbpf: prog 'mmap_fentry': found data map 1 (main.rodata, sec 6, off 0) for insn 17
libbpf: loading kernel BTF '/sys/kernel/btf/vmlinux': 0
libbpf: map 'percpu_hash': created successfully, fd=8
libbpf: map 'main.rodata': created successfully, fd=9
libbpf: prog 'mmap_fentry': failed to attach: ERROR: strerror_r(-524)=22
failed to attach program: errno 524
[!] ERROR: test error
make[1]: *** [Makefile:81: run-static] Error 4
make[1]: Leaving directory '/vagrant/selftest/percpu'



libbpf: map 1 is "main.bss"
libbpf: sec '.relfentry': collecting relocation for section(3) 'fentry'
libbpf: sec '.relfentry': relo #0: insn #0 against 'ringbuffer_flags'
libbpf: prog 'foobar': found data map 1 (main.bss, sec 5, off 0) for insn 0
libbpf: sec '.relfentry': relo #1: insn #3 against 'events'
libbpf: prog 'foobar': found map 0 (events, sec 7, off 0) for insn #3
libbpf: loading kernel BTF '/sys/kernel/btf/vmlinux': 0
libbpf: map 'events': created successfully, fd=8
libbpf: map 'main.bss': created successfully, fd=9
libbpf: prog 'foobar': failed to attach: ERROR: strerror_r(-524)=22
failed to attach program: errno 524
[!] ERROR: test error
make[1]: *** [Makefile:81: run-static] Error 4
make[1]: Leaving directory '/vagrant/selftest/set-attach'



libbpf: sec '.relfentry/__arm64_sys_mmap': collecting relocation for section(3) 'fentry/__arm64_sys_mmap'
libbpf: sec '.relfentry/__arm64_sys_mmap': relo #0: insn #0 against '.rodata'
libbpf: prog 'mmap_fentry': found data map 1 (second.rodata, sec 5, off 0) for insn 0
libbpf: sec '.relfentry/__arm64_sys_mmap': relo #1: insn #4 against 'events'
libbpf: prog 'mmap_fentry': found map 0 (events, sec 6, off 0) for insn #4
libbpf: reused pinned map at '/sys/fs/bpf/events'
libbpf: map 'events': skipping creation (preset fd=11)
libbpf: loading kernel BTF '/sys/kernel/btf/vmlinux': 0
libbpf: reused pinned map at '/sys/fs/bpf/events'
libbpf: map 'events': skipping creation (preset fd=12)
libbpf: map 'first.rodata': created successfully, fd=10
libbpf: loading kernel BTF '/sys/kernel/btf/vmlinux': 0
libbpf: reused pinned map at '/sys/fs/bpf/events'
libbpf: map 'events': skipping creation (preset fd=15)
libbpf: map 'second.rodata': created successfully, fd=14
libbpf: prog 'openat_fentry': failed to attach: ERROR: strerror_r(-524)=22
couldn't attach prog
[!] ERROR: test error
make[1]: *** [Makefile:90: run-static] Error 4
make[1]: Leaving directory '/vagrant/selftest/multiple-objects'



libbpf: sec '.relfentry/__arm64_sys_mmap': collecting relocation for section(3) 'fentry/__arm64_sys_mmap'
libbpf: sec '.relfentry/__arm64_sys_mmap': relo #0: insn #4 against 'counter_hash_map'
libbpf: prog 'mmap_fentry': found map 0 (counter_hash_map, sec 6, off 0) for insn #4
libbpf: sec '.relfentry/__arm64_sys_mmap': relo #1: insn #9 against 'ringbuffer_flags'
libbpf: prog 'mmap_fentry': found data map 2 (main.bss, sec 5, off 0) for insn 9
libbpf: sec '.relfentry/__arm64_sys_mmap': relo #2: insn #12 against 'events'
libbpf: prog 'mmap_fentry': found map 1 (events, sec 6, off 32) for insn #12
libbpf: loading kernel BTF '/sys/kernel/btf/vmlinux': 0
libbpf: map 'counter_hash_map': created successfully, fd=8
libbpf: map 'events': created successfully, fd=9
libbpf: map 'main.bss': created successfully, fd=10
libbpf: prog 'mmap_fentry': failed to attach: ERROR: strerror_r(-524)=22
failed to attach program: errno 524
[!] ERROR: test error
make[1]: *** [Makefile:81: run-static] Error 4
make[1]: Leaving directory '/vagrant/selftest/spinlocks'



libbpf: sec '.relfentry/__arm64_sys_mmap': collecting relocation for section(3) 'fentry/__arm64_sys_mmap'
libbpf: sec '.relfentry/__arm64_sys_mmap': relo #0: insn #0 against 'ringbuffer_flags'
libbpf: prog 'mmap_fentry': found data map 1 (main.bss, sec 5, off 0) for insn 0
libbpf: sec '.relfentry/__arm64_sys_mmap': relo #1: insn #3 against 'events'
libbpf: prog 'mmap_fentry': found map 0 (events, sec 6, off 0) for insn #3
libbpf: loading kernel BTF '/sys/kernel/btf/vmlinux': 0
libbpf: map 'events': created successfully, fd=8
libbpf: map 'main.bss': created successfully, fd=9
libbpf: prog 'mmap_fentry': failed to attach: ERROR: strerror_r(-524)=22
failed to attach program: errno 524
[!] ERROR: test error
make[1]: *** [Makefile:81: run-static] Error 4
make[1]: Leaving directory '/vagrant/selftest/tracing'

Despite the fact that selftests were handling the correct function names for arm64, some are still not working due to the fact that arm64 still does not support generic attaching `bpf_program__attach()`. ``` libbpf: prog 'mmap_fentry': failed to attach: ERROR: strerror_r(-524)=22 failed to attach program: errno 524 [!] ERROR: test error make[1]: *** [Makefile:81: run-static] Error 4 make[1]: Leaving directory '/vagrant/selftest/percpu' ``` `#define ENOTSUPP 524 /* Operation is not supported */`

libbpfgo.go

selftest/map-of-maps/main.go

In this commit, the libbpfgo.go file undergoes significant changes, primarily focused on introducing and updating functions related to BPF maps. The main feature added is the capability to utilize "map of maps", which involves creating, managing, and interacting with nested maps. Added Functions: Reload GetMapByID GetBTFFDByID SetInnerMap MaxEntries (Note: GetMaxEntries is deprecated) Updated Functions (to deal with maps returned by CreateMap): CreateMap Name Type SetType KeySize ValueSize SetValueSize Pin Unpin SetPinPath PinPath IsPinned FileDescriptor GetValue GetValueFlags GetValueReadInto Update Resize GetValueBatch GetValueAndDeleteBatch Miscellaneous: Aside from the map-related features, this commit also includes various minor updates and improves error handling, all of which contribute to the overall robustness of the library. Co-authored-by: Kemal Akkoyun <kakkoyun@gmail.com>

rafaeldtinoco · 2023-08-01T20:02:22Z

I just spoke to @geyslan that we have a few big problems in this approach, not coming from what Geyslan is proposing, but before. We started mixing libbpf functions that are "managed" by libbpf (always related to a BPF program, BPF map or BPF link) with low level functions that aren't supposed to be managed by libbpf (despite being exported). One example of this is bpf_map_create(). It provides a way to call BPF syscall and create a map but the resulting map is totally un-managed by libbpf (thus libbpfgo).

After couple of hours discussing we end up agreeing that we should have a new type "Bpf MapOfMaps" that will do all the abstraction of using the libbpf un-managed functions but making the type fully managed by libbpfgo.

This will be more clear in his next PR, which we agreed to have:

un-managed functions package (will contain exported by libbpf functions that can be used by external players but dont cause libbpf to manage resources).
all the rest of libbpfgo should rely in the stable API with exported functions that are managed (and all of these functions contain __ in their names.

With that said, I'll be the one reviewing a new PR soon to come after our talks.

Thanks @geyslan and I think we should seek for any un-managed libbpf function being exported by libbpfgo and move them to this un-managed package (so users are aware that using those functions are their full responsibility).

Moved C definitions to 'libbpfgo.c', leaving 'libbpfgo.h' only with declarations to avoid multiple declaration errors when importing 'libbpfgo.h' from different Go files. Introduce 'libbpfgo_bpf_map_batch_opts_new' and 'libbpfgo_bpf_map_batch_opts_free' struct helpers to manage the C struct lifecycle on the C side. These are replacements for 'BPFMapBatchOpts' and 'bpfMapBatchOptsToC()'. This avoids problems with structs which may contain bitfields. See aquasecurity#244. Prefixed all C helpers used in Go with 'libbpfgo'. Restructured libbpf BPF map logic into separate files: - 'map-common.go' contains generic BPF map types, constants, and helpers like 'GetMapInfoByFD()' and 'GetMapFDByID()'. - 'map-low.go' includes 'BPFMapLow' and low-level helpers like the newly introduced 'GetMapByID()'. - 'map-iterator.go' contains related types and logic. Introduced 'misc.go' as a place for miscellaneous generic helpers and 'btf.go' with the new 'GetBTFFDByID()' function, as a place for BTF related helpers. In the 'BPFMap' struct: - Removed fields like 'fd' and 'name' since they must be retrieved directly from 'libbpf'. - Added 'bpfMapLow' instance for access to low-level methods. - Deprecated 'BPFMap.GetMaxEntries' in favor of using 'MaxEntries'. - Exposed 'InitialValue' and 'SetInitialValue' as public wrappers. Bug Fixes: - Fixed cases where 'BPFMap.ValueSize()', possibly 0, was being passed directly to 'make()' - detected on the map of maps effort aquasecurity#343. - Fixed 'get_internal_map_init_value' which didn't check against NULL. Further Refactoring and Standardization on the map related logic and on lines touched by other changes: - Removed unnecessary assignment to the blank identifier (S1005): 'case _, _ = <-stop:'. - Removed check for 'BPFMap.Name()' against nil, as 'C.GoString(C.NULL)' returns "". - Changed error returns to 'retC' in functions that return error values to avoid confusion with possible use of errno. - Changed returns to 'valueC' in functions that return values or pointers to values. - Suffixed variable names that are C types with 'C'. - Applied the use of 'defer' for better resource management.

Moved C definitions to 'libbpfgo.c', leaving 'libbpfgo.h' only with declarations to avoid multiple declaration errors when importing 'libbpfgo.h' from different Go files. Introduce 'libbpfgo_bpf_map_batch_opts_new' and 'libbpfgo_bpf_map_batch_opts_free' struct helpers to manage the C struct lifecycle on the C side. These are replacements for 'BPFMapBatchOpts' and 'bpfMapBatchOptsToC()'. This avoids problems with structs which may contain bitfields. See aquasecurity#244. Prefixed all C helpers used in Go with 'libbpfgo'. Restructured libbpf BPF map logic into separate files: - 'map-common.go' contains generic BPF map types, constants, and helpers like 'GetMapInfoByFD()' and 'GetMapFDByID()'. - 'map-low.go' includes 'BPFMapLow' and low-level helpers like the newly introduced 'GetMapByID()'. - 'map-iterator.go' contains related types and logic. Introduced 'misc.go' as a place for miscellaneous generic helpers and 'btf.go' with the new 'GetBTFFDByID()' function, as a place for BTF related helpers. In the 'BPFMap' struct: - Removed fields like 'fd' and 'name' since they must be retrieved directly from 'libbpf'. - Added 'bpfMapLow' instance for access to low-level methods. - Deprecated 'BPFMap.GetMaxEntries' in favor of using 'MaxEntries'. - Exposed 'InitialValue' and 'SetInitialValue' as public wrappers. Bug Fixes: - Fixed cases where 'BPFMap.ValueSize()', possibly 0, was being passed directly to 'make()' - detected on the map of maps effort aquasecurity#343. - Fixed 'get_internal_map_init_value' which didn't check against NULL. Further Refactoring and Standardization on the map related logic and on lines touched by other changes: - Removed check for 'BPFMap.Name()' against nil, as 'C.GoString(C.NULL)' returns "". - Changed error returns to 'retC' in functions that return error values to avoid confusion with possible use of errno. - Changed returns to 'valueC' in functions that return values or pointers to values. - Suffixed variable names that are C types with 'C'. - Applied the use of 'defer' for better resource management.

Introduced struct helpers to manage the C struct lifecycle on the C side, which avoids problems with structs which may contain bitfields. See aquasecurity#244. These are replacements for 'BPFMapBatchOpts' and 'bpfMapBatchOptsToC()'. - libbpfgo_bpf_map_batch_opts_new - libbpfgo_bpf_map_batch_opts_free These are replacements for 'bpfMapCreateOptsToC()' - libbpfgo_bpf_map_create_opts_new - libbpfgo_bpf_map_create_opts_free Restructured libbpf BPF map logic into separate files: - 'map-common.go' contains generic BPF map types, constants, and helpers like 'GetMapInfoByFD()'. - 'map-low.go' includes 'BPFMapLow' and respective logic and helpers like 'CreateMap()'. - 'map-iterator.go' contains related types and logic. In the 'BPFMap' struct: - Removed fields like 'fd' and 'name' since they must be retrieved directly from 'libbpf'. - Added 'bpfMapLow' instance for access to low-level methods. - Deprecated 'BPFMap.GetMaxEntries' in favor of using 'MaxEntries'. - Exposed 'InitialValue' and 'SetInitialValue' as public wrappers. Introduced 'misc.go' as a place for miscellaneous generic helpers. Bug Fixes: - Fixed cases where 'BPFMap.ValueSize()', possibly 0, was being passed directly to 'make()' - detected on the map of maps effort aquasecurity#343. Further Refactoring and Standardization on the map related logic and on lines touched by other changes: - Removed check for 'BPFMap.Name()' against nil, as 'C.GoString(C.NULL)' returns "". - Changed error returns to 'retC' in functions that return error values to avoid confusion with possible use of errno. - Changed returns to 'valueC' in functions that return values or pointers to values. - Suffixed variable names that are C types with 'C'. - Applied the use of 'defer' for better resource management.

Introduced struct helpers to manage the C struct lifecycle on the C side, which avoids problems with structs which may contain bitfields. See aquasecurity#244. These are replacements for 'BPFMapBatchOpts' and 'bpfMapBatchOptsToC()'. - libbpfgo_bpf_map_batch_opts_new - libbpfgo_bpf_map_batch_opts_free These are replacements for 'bpfMapCreateOptsToC()' - libbpfgo_bpf_map_create_opts_new - libbpfgo_bpf_map_create_opts_free Restructured libbpf BPF map logic into separate files: - 'map-common.go' contains generic BPF map types, constants, and helpers like 'GetMapInfoByFD()'. - 'map-low.go' includes 'BPFMapLow' and respective logic and helpers like 'CreateMap()'. - 'map-iterator.go' contains related types and logic. In the 'BPFMap' struct: - Removed fields like 'fd' and 'name' since they must be retrieved directly from 'libbpf'. - Added 'bpfMapLow' instance for access to low-level methods. - Deprecated 'BPFMap.GetMaxEntries' in favor of using 'MaxEntries'. - Exposed 'InitialValue' and 'SetInitialValue' as public wrappers. - Introduced MapFlags(), IfIndex() and MapExtra() methods. Introduced 'misc.go' as a place for miscellaneous generic helpers. Bug Fixes: - Fixed cases where 'BPFMap.ValueSize()', possibly 0, was being passed directly to 'make()' - detected on the map of maps effort aquasecurity#343. Further Refactoring and Standardization on the map related logic and on lines touched by other changes: - Removed check for 'BPFMap.Name()' against nil, as 'C.GoString(C.NULL)' returns "". - Changed error returns to 'retC' in functions that return error values to avoid confusion with possible use of errno. - Changed returns to 'valueC' in functions that return values or pointers to values. - Suffixed variable names that are C types with 'C'. - Applied the use of 'defer' for better resource management.

Introduced struct helpers to manage the C struct lifecycle on the C side, which avoids problems with structs which may contain bitfields. See aquasecurity#244. These are replacements for 'BPFMapBatchOpts' and 'bpfMapBatchOptsToC()'. - cgo_bpf_map_batch_opts_new - cgo_bpf_map_batch_opts_free These are replacements for 'bpfMapCreateOptsToC()' - cgo_bpf_map_create_opts_new - cgo_bpf_map_create_opts_free Restructured libbpf BPF map logic into separate files: - 'map-common.go' contains generic BPF map types, constants, and helpers like 'GetMapInfoByFD()'. - 'map-low.go' includes 'BPFMapLow' and respective logic and helpers like 'CreateMap()'. - 'map-iterator.go' contains related types and logic. In the 'BPFMap' struct: - Removed fields like 'fd' and 'name' since they must be retrieved directly from 'libbpf'. - Added 'bpfMapLow' instance for access to low-level methods. - Deprecated 'BPFMap.GetMaxEntries' in favor of using 'MaxEntries'. - Exposed 'InitialValue' and 'SetInitialValue' as public wrappers. - Introduced MapFlags(), IfIndex() and MapExtra() methods. Introduced 'misc.go' as a place for miscellaneous generic helpers. Bug Fixes: - Fixed cases where 'BPFMap.ValueSize()', possibly 0, was being passed directly to 'make()' - detected on the map of maps effort aquasecurity#343. Further Refactoring and Standardization on the map related logic and on lines touched by other changes: - Removed check for 'BPFMap.Name()' against nil, as 'C.GoString(C.NULL)' returns "". - Changed error returns to 'retC' in functions that return error values to avoid confusion with possible use of errno. - Changed returns to 'valueC' in functions that return values or pointers to values. - Suffixed variable names that are C types with 'C'. - Applied the use of 'defer' for better resource management.

Introduced struct helpers to manage the C struct lifecycle on the C side, which avoids problems with structs which may contain bitfields. See #244. These are replacements for 'BPFMapBatchOpts' and 'bpfMapBatchOptsToC()'. - cgo_bpf_map_batch_opts_new - cgo_bpf_map_batch_opts_free These are replacements for 'bpfMapCreateOptsToC()' - cgo_bpf_map_create_opts_new - cgo_bpf_map_create_opts_free Restructured libbpf BPF map logic into separate files: - 'map-common.go' contains generic BPF map types, constants, and helpers like 'GetMapInfoByFD()'. - 'map-low.go' includes 'BPFMapLow' and respective logic and helpers like 'CreateMap()'. - 'map-iterator.go' contains related types and logic. In the 'BPFMap' struct: - Removed fields like 'fd' and 'name' since they must be retrieved directly from 'libbpf'. - Added 'bpfMapLow' instance for access to low-level methods. - Deprecated 'BPFMap.GetMaxEntries' in favor of using 'MaxEntries'. - Exposed 'InitialValue' and 'SetInitialValue' as public wrappers. - Introduced MapFlags(), IfIndex() and MapExtra() methods. Introduced 'misc.go' as a place for miscellaneous generic helpers. Bug Fixes: - Fixed cases where 'BPFMap.ValueSize()', possibly 0, was being passed directly to 'make()' - detected on the map of maps effort #343. Further Refactoring and Standardization on the map related logic and on lines touched by other changes: - Removed check for 'BPFMap.Name()' against nil, as 'C.GoString(C.NULL)' returns "". - Changed error returns to 'retC' in functions that return error values to avoid confusion with possible use of errno. - Changed returns to 'valueC' in functions that return values or pointers to values. - Suffixed variable names that are C types with 'C'. - Applied the use of 'defer' for better resource management.

geyslan added the chore label Jul 20, 2023

geyslan self-assigned this Jul 20, 2023

This comment was marked as outdated.

Sign in to view

geyslan mentioned this pull request Jul 20, 2023

Add APIs to utilize map of maps #213

Closed

1 task

geyslan force-pushed the add-map-in-maps-api branch 4 times, most recently from ee14dad to bae0461 Compare July 21, 2023 18:32