From a78da8fb64dd73613e1f0c2a4bebace11b578733 Mon Sep 17 00:00:00 2001 From: Francisco Javier Honduvilla Coto Date: Wed, 3 Aug 2022 11:16:26 +0100 Subject: [PATCH] batch: Fix DeleteKeyBatch when dealing with partial deletion, more context on https://github.com/aquasecurity/libbpfgo/issues/162 Signed-off-by: Francisco Javier Honduvilla Coto --- libbpfgo.go | 17 +++++++---------- selftest/map-batch/main.go | 21 ++++++++++++++++++++- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/libbpfgo.go b/libbpfgo.go index 9074d030..f382945e 100644 --- a/libbpfgo.go +++ b/libbpfgo.go @@ -880,7 +880,8 @@ func (b *BPFMap) UpdateBatch(keys, values unsafe.Pointer, count uint32) error { // DeleteKeyBatch allows for batch deletion of multiple elements in the map. // // `count` number of keys will be deleted from the map. Passing an argument that greater than the number of keys -// in the map will cause the function to return a syscall.EPERM as an error. +// in the map will cause the function to delete fewer keys than requested. See the inline comment in +// `GetValueAndDeleteBatch` for more context. func (b *BPFMap) DeleteKeyBatch(keys unsafe.Pointer, count uint32) error { countC := C.uint(count) @@ -890,17 +891,13 @@ func (b *BPFMap) DeleteKeyBatch(keys unsafe.Pointer, count uint32) error { Flags: C.BPF_ANY, } - errC := C.bpf_map_delete_batch(b.fd, keys, &countC, bpfMapBatchOptsToC(opts)) - if errC != 0 { - sc := syscall.Errno(-errC) - if sc != syscall.EFAULT { - if uint32(countC) != count { - return fmt.Errorf("failed to batch delete ALL keys from map %s, deleted (%d/%d): %w", b.name, uint32(countC), count, sc) - } - } - return fmt.Errorf("failed to batch delete keys from map %s: %w", b.name, syscall.Errno(-errC)) + ret, errC := C.bpf_map_delete_batch(b.fd, keys, &countC, bpfMapBatchOptsToC(opts)) + + if ret != 0 && errC != syscall.ENOENT { + return fmt.Errorf("failed to batch delete keys %v in map %s: ret %d (err: %s)", keys, b.name, ret, errC) } + // ret = -1 && errno == syscall.ENOENT indicates a partial deletion. return nil } diff --git a/selftest/map-batch/main.go b/selftest/map-batch/main.go index d3abade6..e764b605 100644 --- a/selftest/map-batch/main.go +++ b/selftest/map-batch/main.go @@ -103,7 +103,12 @@ func main() { } // Test batch delete. - testerMap.DeleteKeyBatch(unsafe.Pointer(&keys[0]), uint32(len(keys)-1)) + // Trying to delete more keys than we have. + err = testerMap.DeleteKeyBatch(unsafe.Pointer(&keys[0]), uint32(len(keys)+100)) + if err != nil { + fmt.Fprintf(os.Stderr, "testerMap.DeleteKeyBatch was expected to not fail") + os.Exit(-1) + } // Ensure value is no longer there. _, err = testerMap.GetValue(unsafe.Pointer(&keys[0])) @@ -112,6 +117,20 @@ func main() { os.Exit(-1) } + // Re-add deleted entries. + if err := testerMap.UpdateBatch(unsafe.Pointer(&keys[0]), unsafe.Pointer(&values[0]), uint32(len(keys))); err != nil { + fmt.Fprintf(os.Stderr, "testerMap.UpdateBatch failed: %v", err) + os.Exit(-1) + } + + // Test batch delete. + // Trying to delete fewer or equal keys than we have. + err = testerMap.DeleteKeyBatch(unsafe.Pointer(&keys[0]), uint32(len(keys)-1)) + if err != nil { + fmt.Fprintf(os.Stderr, "testerMap.DeleteKeyBatch was expected to not fail") + os.Exit(-1) + } + // Test batch lookup and delete when requesting more elements than are in the map. deleteKeys = make([]uint32, 3) nextKey = uint32(0)