Skip to content

Commit

Permalink
[BPF] do not check value size in DeleteMapEntry
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tomastigera committed Jun 19, 2023
1 parent 232174b commit 4a8a595
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 22 deletions.
16 changes: 3 additions & 13 deletions felix/bpf/maps/maps.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -416,21 +416,11 @@ 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 {
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 DeleteMapEntryIfExists(b.fd, k, valueSize)
return DeleteMapEntryIfExists(b.fd, k)
}

func (b *PinnedMap) updateDeltaEntries() error {
Expand Down
10 changes: 5 additions & 5 deletions felix/bpf/maps/syscall.go
Original file line number Diff line number Diff line change
Expand Up @@ -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), -1)
if err != nil {
return err
}
Expand All @@ -173,8 +173,8 @@ func DeleteMapEntry(mapFD FD, k []byte, valueSize int) error {
return nil
}

func DeleteMapEntryIfExists(mapFD FD, k []byte, valueSize int) error {
err := DeleteMapEntry(mapFD, k, valueSize)
func DeleteMapEntryIfExists(mapFD FD, k []byte) error {
err := DeleteMapEntry(mapFD, k)
if err == unix.ENOENT {
// Delete failed because entry did not exist.
err = nil
Expand Down
8 changes: 4 additions & 4 deletions felix/bpf/ut/bpf_prog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1510,18 +1510,18 @@ 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)
Expect(err).NotTo(HaveOccurred())

err = maps.DeleteMapEntryIfExists(jumpMapFD, k, 4)
err = maps.DeleteMapEntryIfExists(jumpMapFD, k)
Expect(err).NotTo(HaveOccurred())

err = maps.DeleteMapEntryIfExists(jumpMapFD, k, 4)
err = maps.DeleteMapEntryIfExists(jumpMapFD, k)
Expect(err).NotTo(HaveOccurred())

err = maps.DeleteMapEntry(jumpMapFD, k, 4)
err = maps.DeleteMapEntry(jumpMapFD, k)
Expect(err).To(HaveOccurred())
}

0 comments on commit 4a8a595

Please sign in to comment.