Skip to content

Commit

Permalink
service: Skip upsert of service for disabled IP family
Browse files Browse the repository at this point in the history
The following panic can happen when trying to upsert e.g. an IPv6
service in the datapath when IPv6 is disabled in the agent. The
corresponding IPv6 lbmap doesn't exist so we get a nil pointer
reference.

    panic: runtime error: invalid memory address or nil pointer dereference
    [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1cd5900]

    goroutine 147 [running]:
    github.com/cilium/cilium/pkg/bpf.(*Map).OpenOrCreate(0x0, 0x0, 0x0, 0x0)
            /go/src/github.com/cilium/cilium/pkg/bpf/map_linux.go:464 +0x40
    github.com/cilium/cilium/pkg/maps/lbmap.updateRevNatLocked(0x2b1b5a0, 0xc000b365fc, 0x2b07380, 0xc000ae7e80, 0xc000b365f0, 0x1)
            /go/src/github.com/cilium/cilium/pkg/maps/lbmap/lbmap.go:331 +0x68
    github.com/cilium/cilium/pkg/maps/lbmap.(*LBBPFMap).UpsertService(0xc0009f5040, 0xc000b3a1e0, 0x0, 0xc000b47470)
            /go/src/github.com/cilium/cilium/pkg/maps/lbmap/lbmap.go:130 +0x5b7
    github.com/cilium/cilium/pkg/service.(*Service).upsertServiceIntoLBMaps(0xc00065a280, 0xc00066b420, 0x421e500, 0x0, 0x421e540, 0x0, 0x0, 0x421e540, 0x0, 0x0, ...)
            /go/src/github.com/cilium/cilium/pkg/service/service.go:749 +0x3de
    github.com/cilium/cilium/pkg/service.(*Service).UpsertService(0xc00065a280, 0xc000661a40, 0x0, 0x0, 0x0)
            /go/src/github.com/cilium/cilium/pkg/service/service.go:324 +0xc85
    github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).addK8sSVCs(0xc000974480, 0xc0009a34c0, 0x1d, 0xc0009b2a96, 0x7, 0x0, 0xc000b22f30, 0xc00012be48, 0x10, 0x7ffff7fb9108)
            /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:743 +0x47b
    github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).k8sServiceHandler.func1(0x0, 0xc0009a34c0, 0x1d, 0xc0009b2a96, 0x7, 0xc000b22f30, 0x0, 0xc00012be48, 0xc0009b0390)
            /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:447 +0xbc7
    github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).k8sServiceHandler(0xc000974480)
            /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:490 +0x95
    created by github.com/cilium/cilium/pkg/k8s/watchers.(*K8sWatcher).RunK8sServiceHandler
            /go/src/github.com/cilium/cilium/pkg/k8s/watchers/watcher.go:495 +0x3f

This commit fixes it by exiting early from UpsertService if trying to
upsert a service for an IP family that is disabled in the agent.

Fixes: #15000
Fixes: #14607
Signed-off-by: Paul Chaignon <paul@cilium.io>
  • Loading branch information
pchaigno authored and aanm committed Mar 4, 2021
1 parent 6536f5f commit ad61343
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 0 deletions.
10 changes: 10 additions & 0 deletions pkg/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,16 @@ func (s *Service) UpsertService(params *lb.SVC) (bool, lb.ID, error) {
option.EnableSVCSourceRangeCheck)
}

ipv6Svc := params.Frontend.IsIPv6()
if ipv6Svc && !option.Config.EnableIPv6 {
err := fmt.Errorf("Unable to upsert service %s as IPv6 is disabled", params.Frontend.L3n4Addr.String())
return false, lb.ID(0), err
}
if !ipv6Svc && !option.Config.EnableIPv4 {
err := fmt.Errorf("Unable to upsert service %s as IPv4 is disabled", params.Frontend.L3n4Addr.String())
return false, lb.ID(0), err
}

// In case we do DSR + IPIP, then it's required that the backends use
// the same destination port as the frontend service.
if option.Config.NodePortMode == option.NodePortModeDSR &&
Expand Down
56 changes: 56 additions & 0 deletions pkg/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type ManagerTestSuite struct {
prevOptionLBSourceRanges bool
prevOptionNPAlgo string
prevOptionDPMode string
ipv6 bool
}

var _ = Suite(&ManagerTestSuite{})
Expand All @@ -62,6 +63,8 @@ func (m *ManagerTestSuite) SetUpTest(c *C) {

m.prevOptionNPAlgo = option.Config.NodePortAlg
m.prevOptionDPMode = option.Config.DatapathMode

m.ipv6 = option.Config.EnableIPv6
}

func (m *ManagerTestSuite) TearDownTest(c *C) {
Expand All @@ -71,11 +74,13 @@ func (m *ManagerTestSuite) TearDownTest(c *C) {
option.Config.EnableSVCSourceRangeCheck = m.prevOptionLBSourceRanges
option.Config.NodePortAlg = m.prevOptionNPAlgo
option.Config.DatapathMode = m.prevOptionDPMode
option.Config.EnableIPv6 = m.ipv6
}

var (
frontend1 = *lb.NewL3n4AddrID(lb.TCP, net.ParseIP("1.1.1.1"), 80, lb.ScopeExternal, 0)
frontend2 = *lb.NewL3n4AddrID(lb.TCP, net.ParseIP("1.1.1.2"), 80, lb.ScopeExternal, 0)
frontend3 = *lb.NewL3n4AddrID(lb.TCP, net.ParseIP("f00d::1"), 80, lb.ScopeExternal, 0)
backends1 = []lb.Backend{
*lb.NewBackend(0, lb.TCP, net.ParseIP("10.0.0.1"), 8080),
*lb.NewBackend(0, lb.TCP, net.ParseIP("10.0.0.2"), 8080),
Expand All @@ -84,9 +89,22 @@ var (
*lb.NewBackend(0, lb.TCP, net.ParseIP("10.0.0.2"), 8080),
*lb.NewBackend(0, lb.TCP, net.ParseIP("10.0.0.3"), 8080),
}
backends3 = []lb.Backend{
*lb.NewBackend(0, lb.TCP, net.ParseIP("fd00::2"), 8080),
*lb.NewBackend(0, lb.TCP, net.ParseIP("fd00::3"), 8080),
}
)

func (m *ManagerTestSuite) TestUpsertAndDeleteService(c *C) {
m.testUpsertAndDeleteService(c)
}

func (m *ManagerTestSuite) TestUpsertAndDeleteServiceWithoutIPv6(c *C) {
option.Config.EnableIPv6 = false
m.testUpsertAndDeleteService(c)
}

func (m *ManagerTestSuite) testUpsertAndDeleteService(c *C) {
// Should create a new service with two backends and session affinity
p := &lb.SVC{
Frontend: frontend1,
Expand Down Expand Up @@ -178,6 +196,44 @@ func (m *ManagerTestSuite) TestUpsertAndDeleteService(c *C) {
c.Assert(len(m.lbmap.AffinityMatch[uint16(id2)]), Equals, 2)
c.Assert(len(m.lbmap.SourceRanges[uint16(id2)]), Equals, 2)

// Should add IPv6 service only if IPv6 is enabled
c.Assert(err, IsNil)
cidr1, err = cidr.ParseCIDR("fd00::/8")
c.Assert(err, IsNil)
p3 := &lb.SVC{
Frontend: frontend3,
Backends: backends3,
Type: lb.SVCTypeLoadBalancer,
TrafficPolicy: lb.SVCTrafficPolicyCluster,
SessionAffinity: true,
SessionAffinityTimeoutSec: 300,
Name: "svc3",
Namespace: "ns3",
LoadBalancerSourceRanges: []*cidr.CIDR{cidr1},
}
created, id3, err := m.svc.UpsertService(p3)
if option.Config.EnableIPv6 {
c.Assert(err, IsNil)
c.Assert(created, Equals, true)
c.Assert(id3, Equals, lb.ID(3))
c.Assert(len(m.lbmap.ServiceByID[uint16(id3)].Backends), Equals, 2)
c.Assert(len(m.lbmap.BackendByID), Equals, 4)
c.Assert(m.svc.svcByID[id3].svcName, Equals, "svc3")
c.Assert(m.svc.svcByID[id3].svcNamespace, Equals, "ns3")
c.Assert(len(m.lbmap.AffinityMatch[uint16(id3)]), Equals, 2)
c.Assert(len(m.lbmap.SourceRanges[uint16(id3)]), Equals, 1)

// Should remove the IPv6 service
found, err := m.svc.DeleteServiceByID(lb.ServiceID(id3))
c.Assert(err, IsNil)
c.Assert(found, Equals, true)
} else {
c.Assert(err, ErrorMatches, "Unable to upsert service .+ as IPv6 is disabled")
c.Assert(created, Equals, false)
}
c.Assert(len(m.lbmap.ServiceByID), Equals, 2)
c.Assert(len(m.lbmap.BackendByID), Equals, 2)

// Should remove the service and the backend, but keep another service and
// its backends. Also, should remove the affinity match.
found, err := m.svc.DeleteServiceByID(lb.ServiceID(id1))
Expand Down

0 comments on commit ad61343

Please sign in to comment.