[BPF] do not check value size in DeleteMapEntry

Passing value size is unnecessary and is incorrectly checked when debug mode is turned on as at the place of the check we do not know whether a map is per-cpu or not.
projectcalico · Jun 16, 2023 · 23a8fd6 · 23a8fd6
1 parent 232174b
commit 23a8fd6
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 15 deletions.
diff --git a/felix/bpf/maps/maps.go b/felix/bpf/maps/maps.go
@@ -360,7 +360,7 @@ func (b *PinnedMap) Iter(f IterCallback) error {
 		if action == IterDelete {
 			// The previous iteration asked us to delete its key; do that now before we check for the end of
 			// the iteration.
-			err := DeleteMapEntry(b.MapFD(), keyToDelete, valueSize)
+			err := DeleteMapEntry(b.MapFD(), keyToDelete)
 			if err != nil && !IsNotExists(err) {
 				return fmt.Errorf("failed to delete map entry: %w", err)
 			}
@@ -416,12 +416,7 @@ func (b *PinnedMap) Get(k []byte) ([]byte, error) {
 }
 
 func (b *PinnedMap) Delete(k []byte) error {
-	valueSize := b.ValueSize
-	if b.perCPU {
-		valueSize = b.ValueSize * NumPossibleCPUs()
-		log.Debugf("Set value size to %v for deleting an entry from Per-CPU map", valueSize)
-	}
-	return DeleteMapEntry(b.fd, k, valueSize)
+	return DeleteMapEntry(b.fd, k)
 }
 
 func (b *PinnedMap) DeleteIfExists(k []byte) error {

diff --git a/felix/bpf/maps/syscall.go b/felix/bpf/maps/syscall.go
@@ -125,11 +125,11 @@ func checkMapIfDebug(mapFD FD, keySize, valueSize int) error {
 	case unix.BPF_MAP_TYPE_PERCPU_HASH, unix.BPF_MAP_TYPE_PERCPU_ARRAY, unix.BPF_MAP_TYPE_LRU_PERCPU_HASH, unix.BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE:
 		// The actual size of per cpu maps is equal to the value size * number of cpu
 		ncpus := NumPossibleCPUs()
-		if valueSize >= 0 && valueSize != mapInfo.ValueSize*ncpus {
+		if valueSize > 0 && valueSize != mapInfo.ValueSize*ncpus {
 			log.WithField("mapInfo", mapInfo).WithField("valueLen", valueSize).Panic("Incorrect value length for per-CPU map")
 		}
 	default:
-		if valueSize >= 0 && valueSize != mapInfo.ValueSize {
+		if valueSize > 0 && valueSize != mapInfo.ValueSize {
 			log.WithField("mapInfo", mapInfo).WithField("valueLen", valueSize).Panic("Incorrect value length")
 		}
 	}
@@ -156,10 +156,10 @@ func GetMapInfo(fd FD) (*MapInfo, error) {
 	}, nil
 }
 
-func DeleteMapEntry(mapFD FD, k []byte, valueSize int) error {
-	log.Debugf("DeleteMapEntry(%v, %v, %v)", mapFD, k, valueSize)
+func DeleteMapEntry(mapFD FD, k []byte) error {
+	log.Debugf("DeleteMapEntry(%v, %v)", mapFD, k)
 
-	err := checkMapIfDebug(mapFD, len(k), valueSize)
+	err := checkMapIfDebug(mapFD, len(k), 0)
 	if err != nil {
 		return err
 	}
@@ -174,7 +174,7 @@ func DeleteMapEntry(mapFD FD, k []byte, valueSize int) error {
 }
 
 func DeleteMapEntryIfExists(mapFD FD, k []byte, valueSize int) error {
-	err := DeleteMapEntry(mapFD, k, valueSize)
+	err := DeleteMapEntry(mapFD, k)
 	if err == unix.ENOENT {
 		// Delete failed because entry did not exist.
 		err = nil

diff --git a/felix/bpf/ut/bpf_prog_test.go b/felix/bpf/ut/bpf_prog_test.go
@@ -1510,7 +1510,7 @@ func TestJumpMap(t *testing.T) {
 	err = maps.UpdateMapEntry(jumpMapFD, k, v)
 	Expect(err).NotTo(HaveOccurred())
 
-	err = maps.DeleteMapEntry(jumpMapFD, k, 4)
+	err = maps.DeleteMapEntry(jumpMapFD, k)
 	Expect(err).NotTo(HaveOccurred())
 
 	err = maps.UpdateMapEntry(jumpMapFD, k, v)
@@ -1522,6 +1522,6 @@ func TestJumpMap(t *testing.T) {
 	err = maps.DeleteMapEntryIfExists(jumpMapFD, k, 4)
 	Expect(err).NotTo(HaveOccurred())
 
-	err = maps.DeleteMapEntry(jumpMapFD, k, 4)
+	err = maps.DeleteMapEntry(jumpMapFD, k)
 	Expect(err).To(HaveOccurred())
 }