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

bpf: lb: un-break terminating backends for service without backend #31840

Merged

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Apr 8, 2024

Discussing with @borkmann we realized that service LB for established connections no longer works, if the service loses the last of its active backends. This was broken by #22388, which introduced the svc->count check too early in the code path.

Fix service connection to terminating backend, when the service has no more backends available.

Continue to forward traffic for established connections, even when a
service loses its last active backends.

This needs a small adjustment in a BPF test that was relying on this
behaviour.

Fixes: 1835011 ("bpf: drop SVC traffic if no backend is available")
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Once a LB connection has been established, we expect to continue using
its CT entry to obtain the backend. Even if the backend is in terminating
state, and the service has lost all of its backends.

Keeping this separate from the fix, in case we can't easily backport.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann julianwiedmann added kind/bug This is a bug in the Cilium logic. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/bug This PR fixes an issue in a previous release of Cilium. area/loadbalancing Impacts load-balancing and Kubernetes service implementations needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Apr 8, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.15.4 Apr 8, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.10 Apr 8, 2024
Copy link
Member

@borkmann borkmann left a comment

Choose a reason for hiding this comment

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

Awesome, and thx for adding a unit test!

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann marked this pull request as ready for review April 8, 2024 23:18
@julianwiedmann julianwiedmann requested a review from a team as a code owner April 8, 2024 23:18
@julianwiedmann julianwiedmann added this pull request to the merge queue Apr 8, 2024
Merged via the queue into cilium:main with commit 7ece278 Apr 8, 2024
62 of 63 checks passed
@julianwiedmann julianwiedmann deleted the 1.16-bpf-lb-terminating-backend branch April 8, 2024 23:26
@julianwiedmann julianwiedmann added the backport/author The backport will be carried out by the author of the PR. label Apr 10, 2024
@asauber asauber added this to Needs backport from main in 1.15.5 Apr 11, 2024
@asauber asauber removed this from Needs backport from main in 1.15.4 Apr 11, 2024
@asauber asauber added this to Needs backport from main in 1.14.11 Apr 11, 2024
@asauber asauber removed this from Needs backport from main in 1.14.10 Apr 11, 2024
@julianwiedmann julianwiedmann removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Apr 19, 2024
@julianwiedmann julianwiedmann added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. affects/v1.13 This issue affects v1.13 branch labels Apr 19, 2024
@github-actions github-actions bot added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. labels May 7, 2024
@nebril nebril added this to Backport pending to v1.14 in 1.14.12 May 10, 2024
@nebril nebril removed this from Needs backport from main in 1.14.11 May 10, 2024
@nebril nebril moved this from Needs backport from main to Backport done to v1.15 in 1.15.5 May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/v1.13 This issue affects v1.13 branch area/loadbalancing Impacts load-balancing and Kubernetes service implementations backport/author The backport will be carried out by the author of the PR. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. 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
1.14.12
Backport pending to v1.14
1.15.5
Backport done to v1.15
Development

Successfully merging this pull request may close these issues.

None yet

2 participants