Skip to content

Commit

Permalink
Improves the error logs during the bpf maps updating
Browse files Browse the repository at this point in the history 
Fixes: #15864
Signed-off-by: El-Fadel Bonfoh <elfadel@accuknox.com>
  • Loading branch information
@elfadel
elfadel authored and BONFOH El Fadel INNOV/NET committed Jul 11, 2021
1 parent cb9ecdc commit d0b5226
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 19 deletions.
17 changes: 13 additions & 4 deletions pkg/bpf/bpf_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ type bpfAttrMapOpElem struct {
}

// UpdateElementFromPointers updates the map in fd with the given value in the given key.
func UpdateElementFromPointers(fd int, structPtr unsafe.Pointer, sizeOfStruct uintptr) error {
func UpdateElementFromPointers(fd int, mapName string, structPtr unsafe.Pointer, sizeOfStruct uintptr) error {
var duration *spanstat.SpanStat
if option.Config.MetricsConfig.BPFSyscallDurationEnabled {
duration = spanstat.Start()
Expand All @@ -113,7 +113,16 @@ func UpdateElementFromPointers(fd int, structPtr unsafe.Pointer, sizeOfStruct ui
}

if ret != 0 || err != 0 {
return fmt.Errorf("Unable to update element for map with file descriptor %d: %s", fd, err)
switch err {
case unix.E2BIG:
return fmt.Errorf("Unable to update element for %s map with file descriptor %d: the map is full, please consider resizing it. %w", mapName, fd, err)
case unix.EEXIST:
return fmt.Errorf("Unable to update element for %s map with file descriptor %d: specified key already exists. %w", mapName, fd, err)
case unix.ENOENT:
return fmt.Errorf("Unable to update element for %s map with file descriptor %d: key does not exist. %w", mapName, fd, err)
default:
return fmt.Errorf("Unable to update element for %s map with file descriptor %d: %w", mapName, fd, err)
}
}

return nil
Expand All @@ -125,15 +134,15 @@ func UpdateElementFromPointers(fd int, structPtr unsafe.Pointer, sizeOfStruct ui
// bpf.BPF_NOEXIST to create new element if it didn't exist;
// bpf.BPF_EXIST to update existing element.
// Deprecated, use UpdateElementFromPointers
func UpdateElement(fd int, key, value unsafe.Pointer, flags uint64) error {
func UpdateElement(fd int, mapName string, key, value unsafe.Pointer, flags uint64) error {
uba := bpfAttrMapOpElem{
mapFd: uint32(fd),
key: uint64(uintptr(key)),
value: uint64(uintptr(value)),
flags: uint64(flags),
}

ret := UpdateElementFromPointers(fd, unsafe.Pointer(&uba), unsafe.Sizeof(uba))
ret := UpdateElementFromPointers(fd, mapName, unsafe.Pointer(&uba), unsafe.Sizeof(uba))
runtime.KeepAlive(key)
runtime.KeepAlive(value)
return ret
Expand Down
4 changes: 2 additions & 2 deletions pkg/bpf/map_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -921,7 +921,7 @@ func (m *Map) Update(key MapKey, value MapValue) error {
return err
}

err = UpdateElement(m.fd, key.GetKeyPtr(), value.GetValuePtr(), 0)
err = UpdateElement(m.fd, m.name, key.GetKeyPtr(), value.GetValuePtr(), 0)
if option.Config.MetricsConfig.BPFMapOps {
metrics.BPFMapOps.WithLabelValues(m.commonName(), metricOpUpdate, metrics.Error2Outcome(err)).Inc()
}
Expand Down Expand Up @@ -1162,7 +1162,7 @@ func (m *Map) resolveErrors(ctx context.Context) error {
switch e.DesiredAction {
case OK:
case Insert:
err := UpdateElement(m.fd, e.Key.GetKeyPtr(), e.Value.GetValuePtr(), 0)
err := UpdateElement(m.fd, m.name, e.Key.GetKeyPtr(), e.Value.GetValuePtr(), 0)
if option.Config.MetricsConfig.BPFMapOps {
metrics.BPFMapOps.WithLabelValues(m.commonName(), metricOpUpdate, metrics.Error2Outcome(err)).Inc()
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/maps/cidrmap/cidrmap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (cm *CIDRMap) InsertCIDR(cidr net.IPNet) error {
return err
}
log.WithField(logfields.Path, cm.path).Debugf("Inserting CIDR entry %s", cidr.String())
return bpf.UpdateElement(cm.Fd, unsafe.Pointer(&key), unsafe.Pointer(&entry), 0)
return bpf.UpdateElement(cm.Fd, cm.path, unsafe.Pointer(&key), unsafe.Pointer(&entry), 0)
}

// DeleteCIDR deletes an entry from 'cm' with key 'cidr'.
Expand Down
18 changes: 9 additions & 9 deletions pkg/maps/ctmap/ctmap_privileged_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (k *CTMapTestSuite) Benchmark_MapUpdate(c *C) {
for i := 0; i < c.N; i++ {
key.DestPort = uint16(i % 0xFFFF)
key.SourcePort = uint16(i / 0xFFFF)
err := bpf.UpdateElement(m.Map.GetFd(), unsafe.Pointer(key), unsafe.Pointer(value), 0)
err := bpf.UpdateElement(m.Map.GetFd(), m.Map.Name(), unsafe.Pointer(key), unsafe.Pointer(value), 0)
c.Assert(err, IsNil)
}

Expand Down Expand Up @@ -147,7 +147,7 @@ func (k *CTMapTestSuite) TestCtGcIcmp(c *C) {
TxBytes: 216,
Lifetime: 37459,
}
err = bpf.UpdateElement(ctMap.Map.GetFd(), unsafe.Pointer(ctKey),
err = bpf.UpdateElement(ctMap.Map.GetFd(), ctMap.Map.Name(), unsafe.Pointer(ctKey),
unsafe.Pointer(ctVal), 0)
c.Assert(err, IsNil)

Expand All @@ -169,7 +169,7 @@ func (k *CTMapTestSuite) TestCtGcIcmp(c *C) {
Addr: types.IPv4{192, 168, 34, 11},
Port: 0x3195,
}
err = bpf.UpdateElement(natMap.Map.GetFd(), unsafe.Pointer(natKey),
err = bpf.UpdateElement(natMap.Map.GetFd(), natMap.Map.Name(), unsafe.Pointer(natKey),
unsafe.Pointer(natVal), 0)
c.Assert(err, IsNil)
natKey = &nat.NatKey4{
Expand All @@ -190,7 +190,7 @@ func (k *CTMapTestSuite) TestCtGcIcmp(c *C) {
Addr: types.IPv4{192, 168, 34, 11},
Port: 0x3195,
}
err = bpf.UpdateElement(natMap.Map.GetFd(), unsafe.Pointer(natKey),
err = bpf.UpdateElement(natMap.Map.GetFd(), natMap.Map.Name(), unsafe.Pointer(natKey),
unsafe.Pointer(natVal), 0)
c.Assert(err, IsNil)

Expand Down Expand Up @@ -283,7 +283,7 @@ func (k *CTMapTestSuite) TestOrphanNatGC(c *C) {
TxBytes: 216,
Lifetime: 37459,
}
err = bpf.UpdateElement(ctMapAny.Map.GetFd(), unsafe.Pointer(ctKey),
err = bpf.UpdateElement(ctMapAny.Map.GetFd(), ctMapAny.Map.Name(), unsafe.Pointer(ctKey),
unsafe.Pointer(ctVal), 0)
c.Assert(err, IsNil)

Expand All @@ -305,7 +305,7 @@ func (k *CTMapTestSuite) TestOrphanNatGC(c *C) {
Addr: types.IPv4{10, 23, 32, 45},
Port: 0x51d6,
}
err = bpf.UpdateElement(natMap.Map.GetFd(), unsafe.Pointer(natKey),
err = bpf.UpdateElement(natMap.Map.GetFd(), natMap.Map.Name(), unsafe.Pointer(natKey),
unsafe.Pointer(natVal), 0)
c.Assert(err, IsNil)
natKey = &nat.NatKey4{
Expand All @@ -326,7 +326,7 @@ func (k *CTMapTestSuite) TestOrphanNatGC(c *C) {
Addr: types.IPv4{10, 23, 32, 45},
Port: 0x50d6,
}
err = bpf.UpdateElement(natMap.Map.GetFd(), unsafe.Pointer(natKey),
err = bpf.UpdateElement(natMap.Map.GetFd(), natMap.Map.Name(), unsafe.Pointer(natKey),
unsafe.Pointer(natVal), 0)
c.Assert(err, IsNil)

Expand Down Expand Up @@ -354,7 +354,7 @@ func (k *CTMapTestSuite) TestOrphanNatGC(c *C) {
c.Assert(len(buf), Equals, 0)

// Create only CT_INGRESS NAT entry which should be removed
err = bpf.UpdateElement(natMap.Map.GetFd(), unsafe.Pointer(natKey),
err = bpf.UpdateElement(natMap.Map.GetFd(), natMap.Map.Name(), unsafe.Pointer(natKey),
unsafe.Pointer(natVal), 0)
c.Assert(err, IsNil)

Expand Down Expand Up @@ -409,7 +409,7 @@ func (k *CTMapTestSuite) TestOrphanNatGC(c *C) {
Addr: types.IPv6{2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2},
Port: 0x51d6,
}
err = bpf.UpdateElement(natMapV6.Map.GetFd(), unsafe.Pointer(natKeyV6),
err = bpf.UpdateElement(natMapV6.Map.GetFd(), natMapV6.Map.Name(), unsafe.Pointer(natKeyV6),
unsafe.Pointer(natValV6), 0)
c.Assert(err, IsNil)

Expand Down
13 changes: 11 additions & 2 deletions pkg/maps/lbmap/lbmap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package lbmap

import (
"errors"
"fmt"
"net"
"sort"
Expand All @@ -30,6 +31,7 @@ import (
"github.com/cilium/cilium/pkg/u8proto"

"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
)

var log = logging.DefaultLogger.WithField(logfields.LogSubsys, "map-lb")
Expand Down Expand Up @@ -117,8 +119,15 @@ func (lbmap *LBBPFMap) UpsertService(p *UpsertServiceParams) error {
svcVal.SetRevNat(int(p.ID))
svcKey.SetBackendSlot(slot)
if err := updateServiceEndpoint(svcKey, svcVal); err != nil {
return fmt.Errorf("Unable to update service entry %+v => %+v: %s",
svcKey, svcVal, err)
if errors.Is(err, unix.E2BIG) {
return fmt.Errorf("Unable to update service entry %+v => %+v: "+
"Unable to update element for LB bpf map: "+
"You can resize it with the flag \"--%s\". "+
"The resizing might break existing connections to services",
svcKey, svcVal, option.LBMapEntriesName)
}

return fmt.Errorf("Unable to update service entry %+v => %+v: %w", svcKey, svcVal, err)
}
slot++
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/probe/probe.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func HaveFullLPM() bool {
if err != nil {
return
}
err = bpf.UpdateElement(m.GetFd(), unsafe.Pointer(&probeKey{}),
err = bpf.UpdateElement(m.GetFd(), m.Name(), unsafe.Pointer(&probeKey{}),
unsafe.Pointer(&probeValue{}), bpf.BPF_ANY)
if err != nil {
return
Expand Down

0 comments on commit d0b5226

Please sign in to comment.