Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

daemon, lbmap: Avoid premature initialization of BPF maps #14607

Merged

Conversation

christarazi
Copy link
Member

@christarazi christarazi commented Jan 13, 2021

There is a flag called --bpf-lb-map-max that controls the maximum
number of entries in BPF LB maps. When this is set, it updates
lbmap.MaxEntries which is a package-level variable. All LB maps use
this variable to configure their maximum size.

However, due to bpf.NewMap() being called at the package-level (to
initialize "cilium_lb4_services_v2", "cilium_lb4_backends", etc.)
because it's in a var() block, lbmap.MaxEntries has not been updated
yet with the value passed from the aforementioned flag.

Ultimately, this means that the flag was never respected, meaning the
user was never able to change the maximum size of their BPF LB maps.

The reason lbmap.MaxEntries wasn't updated is because Golang will
initialize variables defined in the var() block as soon as the package
is imported, aka before main() [1]. In this case, whenever the lbmap
package was first imported, then the call to bpf.NewMap() was made
with the yet-to-be updated lbmap.MaxEntries. Only after this has
happened (main() begins execution), then Cilium will eventually get to
reading the flag value and updating lbmap.MaxEntries.

This commit avoids the initialization at the var() block and instead
defines explicit initialization functions to be called when appropriate,
in this case, after reading the flag and updating lbmap.MaxEntries,
inside lbmap.Init().

This was found by running scale tests against the service code, e.g.
creating 1000+ backends per service. The following error was observed
and has been fixed with this commit:

[PUT /service/{id}][500] putServiceIdFailure  Unable to update service entry 1.1.0.131:36895 => 2871 (37415) [FLAGS: 0x0]: Unable to update element for map with file descriptor 14: argument list too long

[1]: See below for excerpt. Source:
https://golang.org/ref/spec#Program_initialization_and_execution

Within a package, package-level variable initialization proceeds
stepwise, with each step selecting the variable earliest in
declaration order which has no dependencies on uninitialized
variables.

[...]

A package with no imports is initialized by assigning initial values
to all its package-level variables followed by calling all init
functions in the order they appear in the source, possibly in
multiple files, as presented to the compiler. If a package has
imports, the imported packages are initialized before initializing
the package itself. If multiple packages import a package, the
imported package will be initialized only once. The importing of
packages, by construction, guarantees that there can be no cyclic
initialization dependencies.

Package initialization—variable initialization and the invocation of
init functions—happens in a single goroutine, sequentially, one
package at a time. An init function may launch other goroutines,
which can run concurrently with the initialization code. However,
initialization always sequences the init functions: it will not
invoke the next one until the previous one has returned.
Fix bug where Cilium did not respect `--bpf-lb-map-max` and wouldn't update the maximum size of BPF LB maps

@christarazi christarazi requested review from a team January 13, 2021 21:12
@christarazi christarazi added kind/bug This is a bug in the Cilium logic. needs-backport/1.9 release-note/bug This PR fixes an issue in a previous release of Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. labels Jan 13, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jan 13, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Jan 13, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.2 Jan 13, 2021
@christarazi christarazi force-pushed the pr/christarazi/fix-bpf-max-entries branch from de13440 to a074d11 Compare January 13, 2021 21:13
@christarazi

This comment has been minimized.

@christarazi christarazi requested a review from brb January 13, 2021 21:15
@christarazi christarazi added the area/daemon Impacts operation of the Cilium daemon. label Jan 13, 2021
@christarazi christarazi force-pushed the pr/christarazi/fix-bpf-max-entries branch from a074d11 to bc0d522 Compare January 14, 2021 00:05
@christarazi

This comment has been minimized.

@christarazi christarazi changed the title lbmap, service: Avoid premature init of BPF maps lbmap: Avoid premature initialization of BPF maps Jan 14, 2021
@christarazi christarazi force-pushed the pr/christarazi/fix-bpf-max-entries branch from bc0d522 to 17bbaf8 Compare January 14, 2021 02:22
@christarazi christarazi requested a review from a team as a code owner January 14, 2021 02:22
@christarazi
Copy link
Member Author

test-me-please

@christarazi
Copy link
Member Author

CI has passed. Taking this opportunity to fix typos in commit msg and because the GH Actions never got triggered, to push again.

@christarazi christarazi force-pushed the pr/christarazi/fix-bpf-max-entries branch 2 times, most recently from d749fa0 to d4b58e4 Compare January 14, 2021 07:11
@brb
Copy link
Member

brb commented Jan 27, 2021

test-4.19

pkg/maps/lbmap/lbmap.go Outdated Show resolved Hide resolved
There is a flag called `--bpf-lb-map-max` that controls the maximum
number of entries in BPF LB maps. When this is set, it updates
`lbmap.MaxEntries` which is a package-level variable. All LB maps use
this variable to configure their maximum size.

However, due to `bpf.NewMap()` being called at the package-level (to
initialize "cilium_lb4_services_v2", "cilium_lb4_backends", etc.)
because it's in a `var()` block, `lbmap.MaxEntries` has not been updated
yet with the value passed from the aforementioned flag.

Ultimately, this means that the flag was never respected, meaning the
user was never able to change the maximum size of their BPF LB maps.

The reason `lbmap.MaxEntries` wasn't updated is because Golang will
initialize variables defined in the `var()` block as soon as the package
is imported, aka before `main()` [1]. In this case, whenever the `lbmap`
package was first imported, then the call to `bpf.NewMap()` was made
with the yet-to-be updated `lbmap.MaxEntries`. Only after this has
happened (`main()` begins execution), then Cilium will eventually get to
reading the flag value and updating `lbmap.MaxEntries`.

This commit avoids the initialization at the `var()` block and instead
defines explicit initialization functions to be called when appropriate,
in this case, after reading the flag and updating `lbmap.MaxEntries`,
inside `lbmap.Init()`.

This was found by running scale tests against the service code, e.g.
creating 1000+ backends per service. The following error was observed
and has been fixed with this commit:

```
[PUT /service/{id}][500] putServiceIdFailure  Unable to update service entry 1.1.0.131:36895 => 2871 (37415) [FLAGS: 0x0]: Unable to update element for map with file descriptor 14: argument list too long
```

[1]: See below for excerpt. Source:
https://golang.org/ref/spec#Program_initialization_and_execution

    Within a package, package-level variable initialization proceeds
    stepwise, with each step selecting the variable earliest in
    declaration order which has no dependencies on uninitialized
    variables.

    [...]

    A package with no imports is initialized by assigning initial values
    to all its package-level variables followed by calling all init
    functions in the order they appear in the source, possibly in
    multiple files, as presented to the compiler. If a package has
    imports, the imported packages are initialized before initializing
    the package itself. If multiple packages import a package, the
    imported package will be initialized only once. The importing of
    packages, by construction, guarantees that there can be no cyclic
    initialization dependencies.

    Package initialization—variable initialization and the invocation of
    init functions—happens in a single goroutine, sequentially, one
    package at a time. An init function may launch other goroutines,
    which can run concurrently with the initialization code. However,
    initialization always sequences the init functions: it will not
    invoke the next one until the previous one has returned.

Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi christarazi force-pushed the pr/christarazi/fix-bpf-max-entries branch from b7fef1d to 4c30fc7 Compare January 27, 2021 20:23
@christarazi
Copy link
Member Author

test-me-please

@christarazi christarazi requested a review from brb January 27, 2021 20:24
@brb
Copy link
Member

brb commented Jan 28, 2021

test-gke

Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! As a follow-up we can get rid of more globals (e.g. s/MaxEntries/params.MaxEntries/).

@borkmann borkmann merged commit d9fa628 into cilium:master Jan 28, 2021
@christarazi christarazi deleted the pr/christarazi/fix-bpf-max-entries branch January 28, 2021 17:33
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.4 Jan 29, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.4 Feb 3, 2021
pchaigno added a commit that referenced this pull request Feb 18, 2021
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>
pchaigno added a commit to pchaigno/cilium that referenced this pull request Mar 1, 2021
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: cilium#15000
Fixes: cilium#14607
Signed-off-by: Paul Chaignon <paul@cilium.io>
pchaigno added a commit that referenced this pull request Mar 1, 2021
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>
aanm pushed a commit that referenced this pull request Mar 4, 2021
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>
lyveng pushed a commit to lyveng/cilium that referenced this pull request Mar 4, 2021
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: cilium#15000
Fixes: cilium#14607
Signed-off-by: Paul Chaignon <paul@cilium.io>
tklauser pushed a commit to tklauser/cilium that referenced this pull request Mar 8, 2021
[ upstream commit ad61343 ]

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: cilium#15000
Fixes: cilium#14607
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
joestringer pushed a commit that referenced this pull request Mar 8, 2021
[ upstream commit ad61343 ]

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>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/daemon Impacts operation of the Cilium daemon. kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
1.9.4
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

7 participants