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

kpr: Make it possible to run bpf_sock on Kind #14951

Merged
merged 2 commits into from
Feb 22, 2021

Conversation

brb
Copy link
Member

@brb brb commented Feb 12, 2021

Please see commit msgs.

Marked to be backported to v1.9, because this will resolve the BPF complexity issues when the host-reachable svc is disabled.

On lack of unit tests - unfortunately, we cannot control whether the unit tests will run on a host with or without cgroup v1 which can influence the results. Anyway, I'm planning to enable kpr test suite on Kind.

Support kube-proxy-replacement when running on Kind

@brb brb added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. needs-backport/1.9 labels Feb 12, 2021
@brb brb requested a review from a team February 12, 2021 10:18
@brb brb requested review from a team as code owners February 12, 2021 10:18
@brb brb requested a review from a team February 12, 2021 10:18
@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. labels Feb 12, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Feb 12, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.5 Feb 12, 2021
@brb brb added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Feb 12, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 12, 2021
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

A few nits on the comments, but looks good to me otherwise.

pkg/cgroups/cgroups.go Outdated Show resolved Hide resolved
pkg/cgroups/cgroups.go Outdated Show resolved Hide resolved
pkg/cgroups/cgroups.go Outdated Show resolved Hide resolved
pkg/cgroups/cgroups.go Outdated Show resolved Hide resolved
Documentation/gettingstarted/kind.rst Show resolved Hide resolved
@rolinh
Copy link
Member

rolinh commented Feb 12, 2021

@brb On newer kernels (at least with 5.10), Kind is broken on v1.9 with the recommended options (ie: --set hostServices.enabled=false). Should we add a note about it (only in v1.9 docs)?

Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@qmonnet qmonnet removed their assignment Feb 12, 2021
@brb
Copy link
Member Author

brb commented Feb 12, 2021

On newer kernels (at least with 5.10), Kind is broken on v1.9 with the recommended options (ie: --set hostServices.enabled=false). Should we add a note about it (only in v1.9 docs)?

@rolinh Good point, I will create a separate PR to update the docs. Meanwhile, could you create an issue for it so that I could link it in the docs?

@brb
Copy link
Member Author

brb commented Feb 12, 2021

test-me-please

daemon/cmd/daemon.go Outdated Show resolved Hide resolved
pkg/k8s/node.go Outdated Show resolved Hide resolved
When running on Kind, multiple cilium-agent instances running on the
same host will try to attach to the same cgroup v2 root bpf_sock
programs. As the LB BPF maps cannot be shared among them, in-cluster LB
won't work.

To fix this, we try to derive a cgroup v2 sub-hierarchy from a cilium-agent
process cgroup v2. This guarantees, that each bpf_sock will be attached
to a different cgroup.

The net_cls and net_prio discrepancy when running with cgroup v2 was
spotted by Daniel Borkmann.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Martynas Pumputis <m@lambda.lt>
@brb
Copy link
Member Author

brb commented Feb 22, 2021

test-me-please

@brb
Copy link
Member Author

brb commented Feb 22, 2021

test-net-next

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 22, 2021
@christarazi christarazi merged commit 1c40e0a into master Feb 22, 2021
1.10.0 automation moved this from In progress to Done Feb 22, 2021
@christarazi christarazi deleted the pr/brb/bpf-sock-on-kind branch February 22, 2021 23:13
This was referenced Feb 24, 2021
@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.5 Feb 24, 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.5 Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
1.9.5
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

10 participants