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.

Sign up for GitHub

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

Jump to bottom

datapath: Add a new option to skip socket lb when in pod ns #17154

Merged
merged 1 commit into from Sep 30, 2021
Merged

datapath: Add a new option to skip socket lb when in pod ns #17154

merged 1 commit into from Sep 30, 2021

Conversation

brb
Copy link
Member

@brb brb commented Aug 12, 2021 •
edited

Diff from previous PR (cannot push to the fork's branch, so opening a new PR instead):

  • Explicitly enable bpf_lxc LB if the bypass is enabled (previously, the ifdef was not checking whether the bypass macro was defined).
  • Rename the bypass macro to ENABLE_SOCKET_LB_HOST_ONLY.
  • Rename the helm var from loadBalancer.hostNamespaceOnly to hostServices.hostNamespaceOnly.
  • Minor doc improvements.

Supersedes #13882.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 12, 2021
@brb
Copy link
Member Author

brb commented Aug 12, 2021

test-net-next

@brb brb force-pushed the pr/brb/fix-istio-disable-socket-lb branch from 8233676 to f7d126b Compare August 13, 2021 10:13
@brb
Copy link
Member Author

brb commented Aug 13, 2021

test-me-please

@brb brb mentioned this pull request Aug 13, 2021
Closed
@brb
Copy link
Member Author

brb commented Aug 13, 2021

test-runtime

@brb brb force-pushed the pr/brb/fix-istio-disable-socket-lb branch from f7d126b to 490607a Compare August 24, 2021 08:19
@brb brb changed the title WIP: fix istio socket-lb datapath: Adds a new option to skip socket lb when in pod ns Aug 24, 2021
@brb brb added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/loadbalancing labels Aug 24, 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 Aug 24, 2021
@brb brb force-pushed the pr/brb/fix-istio-disable-socket-lb branch from 490607a to 6d54696 Compare August 24, 2021 08:35
@brb brb changed the title datapath: Adds a new option to skip socket lb when in pod ns datapath: Add a new option to skip socket lb when in pod ns Aug 24, 2021
@brb brb requested a review from Weil0ng August 24, 2021 08:35
@brb
Copy link
Member Author

brb commented Aug 24, 2021 •
edited by maintainer-s-little-helper bot

test-me-please

Job 'Cilium-PR-K8s-1.16-net-next' has 3 failures but they might be new flakes since it also hit 1 known flakes: #17060 (88.47)

@brb brb marked this pull request as ready for review August 24, 2021 08:48
@brb brb requested review from a team August 24, 2021 08:48
@brb brb requested review from a team as code owners August 24, 2021 08:48
@brb brb requested review from a team August 24, 2021 08:48
@brb brb requested a review from a team as a code owner August 24, 2021 08:48
@brb
Copy link
Member Author

brb commented Sep 30, 2021

net-next hit #12511. Marking as ready to merge.

@brb brb added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Sep 30, 2021
@glibsm glibsm merged commit f7303af into master Sep 30, 2021
@glibsm glibsm deleted the pr/brb/fix-istio-disable-socket-lb branch September 30, 2021 16:12
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.5 Oct 4, 2021
@glibsm glibsm mentioned this pull request Oct 4, 2021
Merged
@brb brb mentioned this pull request Oct 11, 2021
Closed
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.5 Oct 13, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.5 Oct 13, 2021
@joestringer joestringer mentioned this pull request Oct 13, 2021
Merged
@brb brb mentioned this pull request Oct 18, 2021
Closed
@brb brb mentioned this pull request Nov 10, 2021
Closed
@brb brb mentioned this pull request Jan 4, 2022
Closed
@cortopy cortopy mentioned this pull request Jan 14, 2022
Closed
1 task
@brb brb mentioned this pull request Feb 22, 2022
Closed
2 tasks
jrajahalme added a commit that referenced this pull request Mar 8, 2022
@jrajahalme
bpf: Fix conditional compilation
ba12881 
Use the same condition in defining the label 'skip_policy_enforcement' as
for jumping to it. Otherwise we could jump to a nonexisting label given a
suitable datapath configuration. This does not seem possible now, as
ENABLE_HOST_SERVICES_FULL should not be defined if
ENABLE_SOCKET_LB_HOST_ONLY is defined. However, as the conditions for
defining ENABLE_PER_PACKET_LB evolve, this would become a compilation
bug.

Fixes: #17154
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme mentioned this pull request Mar 10, 2022
Merged
jrajahalme added a commit to jrajahalme/cilium that referenced this pull request Mar 14, 2022
@jrajahalme
datapath: Fix conditional compilation
3c666e9 
Use the same condition in defining the label 'skip_policy_enforcement' as
for jumping to it. Otherwise we could jump to a nonexisting label given a
suitable datapath configuration. This does not seem possible now, as
ENABLE_HOST_SERVICES_FULL should not be defined if
ENABLE_SOCKET_LB_HOST_ONLY is defined. However, as the conditions for
defining ENABLE_PER_PACKET_LB evolve, this would become a compilation
bug.

Fixes: cilium#17154
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
joestringer pushed a commit that referenced this pull request Mar 15, 2022
@jrajahalme @joestringer
datapath: Fix conditional compilation
6e2ce4a 
Use the same condition in defining the label 'skip_policy_enforcement' as
for jumping to it. Otherwise we could jump to a nonexisting label given a
suitable datapath configuration. This does not seem possible now, as
ENABLE_HOST_SERVICES_FULL should not be defined if
ENABLE_SOCKET_LB_HOST_ONLY is defined. However, as the conditions for
defining ENABLE_PER_PACKET_LB evolve, this would become a compilation
bug.

Fixes: #17154
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
qmonnet pushed a commit that referenced this pull request Mar 30, 2022
@jrajahalme @qmonnet
datapath: Fix conditional compilation
a9dbbe2 
[ upstream commit 6e2ce4a ]

Use the same condition in defining the label 'skip_policy_enforcement' as
for jumping to it. Otherwise we could jump to a nonexisting label given a
suitable datapath configuration. This does not seem possible now, as
ENABLE_HOST_SERVICES_FULL should not be defined if
ENABLE_SOCKET_LB_HOST_ONLY is defined. However, as the conditions for
defining ENABLE_PER_PACKET_LB evolve, this would become a compilation
bug.

Fixes: #17154
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
qmonnet pushed a commit that referenced this pull request Apr 4, 2022
@jrajahalme @qmonnet
datapath: Fix conditional compilation
710f0eb 
[ upstream commit 6e2ce4a ]

Use the same condition in defining the label 'skip_policy_enforcement' as
for jumping to it. Otherwise we could jump to a nonexisting label given a
suitable datapath configuration. This does not seem possible now, as
ENABLE_HOST_SERVICES_FULL should not be defined if
ENABLE_SOCKET_LB_HOST_ONLY is defined. However, as the conditions for
defining ENABLE_PER_PACKET_LB evolve, this would become a compilation
bug.

Fixes: #17154
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
pchaigno pushed a commit that referenced this pull request Apr 4, 2022
@jrajahalme @pchaigno
datapath: Fix conditional compilation
be5ba5e 
[ upstream commit 6e2ce4a ]

Use the same condition in defining the label 'skip_policy_enforcement' as
for jumping to it. Otherwise we could jump to a nonexisting label given a
suitable datapath configuration. This does not seem possible now, as
ENABLE_HOST_SERVICES_FULL should not be defined if
ENABLE_SOCKET_LB_HOST_ONLY is defined. However, as the conditions for
defining ENABLE_PER_PACKET_LB evolve, this would become a compilation
bug.

Fixes: #17154
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@mateiidavid mateiidavid mentioned this pull request Oct 24, 2022
Closed
@lwj5 lwj5 mentioned this pull request Sep 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@borkmann borkmann borkmann left review comments

@errordeveloper errordeveloper errordeveloper approved these changes

@tklauser tklauser tklauser approved these changes

@joamaki joamaki joamaki approved these changes

@pchaigno pchaigno pchaigno approved these changes

@Weil0ng Weil0ng Weil0ng approved these changes

@qmonnet qmonnet qmonnet approved these changes

Assignees
No one assigned
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.
Projects
No open projects
1.10.5
Backport done to v1.10
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet