maps: Handle GetValueAndDeleteBatch return code appropiately

Long story short, some libbpf APIs that don't return pointers, such as the
batch APIs might return `-1` when there's a 'partial success'. In the case
of these APIs, with errno=ENOENT, we know that we read and deleted less than
`count` entries, but it was still overall sucessful.

There's more background in the inline comments). We are unfortunately
breaking the API but I think it's needed. Happy to hear your feedback! :D

This PR fixes this for `GetValueAndDeleteBatch` only. We aren't addressing
all the other batch calls yet as we haven't used them
yet.

Thanks!

libbpfgo.go

@@ -694,20 +694,20 @@ func (b *BPFMap) GetValueBatch(keys unsafe.Pointer, startKey, nextKey unsafe.Poi
 
 // GetValueAndDeleteBatch allows for batch lookup and deletion of elements where each element is deleted after being retrieved from the map.
 //
-// The first argument, keys. is a pointer to an array or slice of keys which will be populated with the keys returned from this operation.
+// The first argument, keys, is a pointer to an array or slice of keys which will be populated with the keys returned from this operation.
 // It returns the associated values as a slice of slices of bytes.
 //
 // This API allows for batch lookups and deletion of multiple keys, potentially in steps over multiple iterations. For example,
 // you provide the last key seen (or nil) for the startKey, and the first key to start the next iteration with in nextKey.
 // Once the first iteration is complete you can provide the last key seen in the previous iteration as the startKey for the next iteration
 // and repeat until nextKey is nil.
 //
-// The last argument, count, is the number of keys to lookup, and delete. 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.
+// The last argument, count, is the number of keys to lookup and delete. The kernel will update it with the count of the elements that were
+// retrieved and deleted.
 //
-// The API can return partial results even though an error is returned.
-// In this case the keys that were successfully retrieved and deleted will be in the result slice.
-func (b *BPFMap) GetValueAndDeleteBatch(keys, startKey, nextKey unsafe.Pointer, count uint32) ([][]byte, error) {
+// The API can return partial results even though an -1 is returned. In this case, errno will be set to `ENOENT` and the values slice and count
+// will be filled in with the elements that were read. See the comment below for more context.
+func (b *BPFMap) GetValueAndDeleteBatch(keys, startKey, nextKey unsafe.Pointer, count uint32) ([][]byte, uint32, uint32, error) {
 	var (
 		values    = make([]byte, b.ValueSize()*int(count))
 		valuesPtr = unsafe.Pointer(&values[0])
@@ -720,19 +720,48 @@ func (b *BPFMap) GetValueAndDeleteBatch(keys, startKey, nextKey unsafe.Pointer,
 		Flags:     C.BPF_ANY,
 	}
 
-	errC := C.bpf_map_lookup_and_delete_batch(b.fd, startKey, nextKey, keys, valuesPtr, &countC, bpfMapBatchOptsToC(opts))
-	if errC != 0 {
-		sc := syscall.Errno(-errC)
-		if sc != syscall.EFAULT {
-			if uint32(countC) != count {
-				return collectBatchValues(values, uint32(countC), b.ValueSize()),
-					fmt.Errorf("failed to retrieve and delete ALL elements in map %s, deleted (%d/%d): %w", b.name, uint32(countC), count, sc)
+	// Before libbpf 1.0 (without LIBBPF_STRICT_DIRECT_ERRS), the return value
+	// and errno not modified [1]. On error, we will get a return value of -1
+	// and errno will be set accordingly with most BPF calls.
+	//
+	// The batch APIs are a bit different in which they can return an error, but
+	// depending on the errno code, it might mean a complete error (nothing was
+	// done) or a partial success (some elements were processed).
+	//
+	// On complete sucess, it will return 0, and errno won't be set.
+	//
+	// [1] https://github.com/libbpf/libbpf/blob/b69f8ee93ef6aa3518f8fbfd9d1df6c2c84fd08f/src/libbpf_internal.h#L496
+	ret, errC := C.bpf_map_lookup_and_delete_batch(
+		b.fd,
+		startKey,
+		nextKey,
+		keys,
+		valuesPtr,
+		&countC,
+		bpfMapBatchOptsToC(opts))
+
+	requested, processed := count, uint32(countC)
+
+	if ret != 0 {
+		if errC == syscall.ENOENT {
+			if requested != processed {
+				// We are reading and returning fewer entries that were requested.
+				parsedVals := collectBatchValues(values, processed, b.ValueSize())
+				return parsedVals, requested, processed, nil
+			} else {
+				// We were not able to process any entries.
+				return nil, requested, processed, fmt.Errorf("failed to batch lookup and delete values %v in map %s: ret %d (err: %s)", keys, b.name, ret, errC)
 			}
+
 		}
-		return nil, fmt.Errorf("failed to batch lookup and delete values %v in map %s: %w", keys, b.name, syscall.Errno(-errC))
+		// Any other errno is a real error.
+		return nil, requested, processed, fmt.Errorf("failed to batch lookup and delete values %v in map %s: ret %d (err: %s)", keys, b.name, ret, errC)
+
 	}
 
-	return collectBatchValues(values, count, b.ValueSize()), nil
+	// All the entries were processed.
+	parsedVals := collectBatchValues(values, processed, b.ValueSize())
+	return parsedVals, requested, processed, nil
 }
 
 func collectBatchValues(values []byte, count uint32, valueSize int) [][]byte {
@@ -787,7 +816,7 @@ func (b *BPFMap) DeleteKeyBatch(keys unsafe.Pointer, count uint32) error {
 		ElemFlags: C.BPF_ANY,
 		Flags:     C.BPF_ANY,
 	}
-	
+
 	errC := C.bpf_map_delete_batch(b.fd, keys, &countC, bpfMapBatchOptsToC(opts))
 	if errC != 0 {
 		sc := syscall.Errno(-errC)

selftest/map-batch/main.go

@@ -76,11 +76,15 @@ func main() {
 	// Test batch lookup and delete.
 	deleteKeys := make([]uint32, 3)
 	nextKey = uint32(0)
-	vals, err := testerMap.GetValueAndDeleteBatch(unsafe.Pointer(&deleteKeys[0]), nil, unsafe.Pointer(&nextKey), uint32(len(deleteKeys)))
+	vals, requestedCount, processedCount, err := testerMap.GetValueAndDeleteBatch(unsafe.Pointer(&deleteKeys[0]), nil, unsafe.Pointer(&nextKey), uint32(len(deleteKeys)))
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "testerMap.LookupBatch failed: %v", err)
 		os.Exit(-1)
 	}
+	if requestedCount != processedCount {
+		fmt.Fprintf(os.Stderr, "testerMap.LookupBatch failed: %d!=%d", requestedCount, processedCount)
+		os.Exit(-1)
+	}
 	for i, val := range vals {
 		actual := binary.LittleEndian.Uint32(val)
 		expected := deleteKeys[i] + 1
@@ -94,6 +98,7 @@ func main() {
 		fmt.Fprintf(os.Stderr, "testerMap.UpdateBatch failed: %v", err)
 		os.Exit(-1)
 	}
+
 	// Test batch delete.
 	testerMap.DeleteKeyBatch(unsafe.Pointer(&keys[0]), uint32(len(keys)))
 
@@ -103,4 +108,26 @@ func main() {
 		fmt.Fprintf(os.Stderr, "testerMap.GetValue was expected to fail, but succeeded")
 		os.Exit(-1)
 	}
+
+	// Test batch lookup and delete when requesting more elements than are in the map.
+	deleteKeys = make([]uint32, 3)
+	nextKey = uint32(0)
+	vals, requestedCount, processedCount, err = testerMap.GetValueAndDeleteBatch(unsafe.Pointer(&deleteKeys[0]), nil, unsafe.Pointer(&nextKey), 5)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "testerMap.LookupBatch failed: %v", err)
+		os.Exit(-1)
+	}
+	if requestedCount != 5 {
+		fmt.Fprintf(os.Stderr, "testerMap.LookupBatch failed: requestedCount=%d", requestedCount)
+		os.Exit(-1)
+	}
+	if processedCount != 0 {
+		fmt.Fprintf(os.Stderr, "testerMap.LookupBatch failed: processedCount=%d", processedCount)
+		os.Exit(-1)
+	}
+
+	if len(vals) != 0 {
+		fmt.Fprintf(os.Stderr, "testerMap.LookupBatch failed: vals=%d", vals)
+		os.Exit(-1)
+	}
 }