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

proxy: fix rule deletion if protocol family is unsupported #30299

Merged
merged 1 commit into from Jan 24, 2024

Conversation

rgo3
Copy link
Contributor

@rgo3 rgo3 commented Jan 17, 2024

Currently, we try to remove IPv6 proxy rules if the IPv6 option is disabled. This is to clean up those rules if a previously running agent has installed them but was restarted with a configuration change. This can fail if the underlying kernel has no IPv6 support. This commit fixes this, by allowing the necessary netlink syscall to fail with EAFNOSUPPORT.

Fixes: #29965

Allow unsupported protocol family errors when deleting IPv6 proxy routing rules

@rgo3 rgo3 added kind/bug This is a bug in the Cilium logic. area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. release-note/bug This PR fixes an issue in a previous release of Cilium. needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Jan 17, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in v1.15.0-rc.1 Jan 17, 2024
@rgo3
Copy link
Contributor Author

rgo3 commented Jan 17, 2024

/test

Copy link
Contributor

@danehans danehans left a comment

Choose a reason for hiding this comment

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

Consider adding a test case for this by using SocketDisableIPv6.

@rgo3
Copy link
Contributor Author

rgo3 commented Jan 17, 2024

Nice suggestion @danehans, left the PR as a draft because I didn't come up with a regression test yet. I'll give it a shot tomorrow!

@rgo3
Copy link
Contributor Author

rgo3 commented Jan 18, 2024

Using SocketDisableIPv6 actually doesn't work here since we are not creating an IPv6 socket but a netlink socket, meaning we don't fail on the socket creation but later on during the syscall that handles the netlink request.

@danehans
Copy link
Contributor

@rgo3 thanks for looking into this and providing feedback.

/LGTM

Currently we try to remove IPv6 proxy rules if the IPv6 option is
disabled. This is to clean up those rules if a previously running agent
has installed them but was restarted with a configuration change. This
can fail if the underlying kernel has no IPv6 support. This commit fixes
this, by allowing the necessary netlink syscall to fail with
EAFNOSUPPORT.

Fixes: cilium#29965

Signed-off-by: Robin Gögge <r.goegge@isovalent.com>
@rgo3
Copy link
Contributor Author

rgo3 commented Jan 19, 2024

/test

@rgo3 rgo3 marked this pull request as ready for review January 19, 2024 11:51
@rgo3 rgo3 requested a review from a team as a code owner January 19, 2024 11:51
@danehans danehans self-requested a review January 19, 2024 16:32
@joestringer joestringer added the release-blocker/1.15 This issue will prevent the release of the next version of Cilium. label Jan 23, 2024
@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 Jan 24, 2024
@squeed squeed added this pull request to the merge queue Jan 24, 2024
Merged via the queue into cilium:main with commit 945ad0c Jan 24, 2024
62 checks passed
@joamaki joamaki mentioned this pull request Jan 30, 2024
28 tasks
@joamaki joamaki added backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Jan 30, 2024
@aanm aanm added this to Needs backport from main in 1.15.1 Jan 31, 2024
@aanm aanm removed this from Needs backport from main in v1.15.0-rc.1 Jan 31, 2024
@aanm aanm 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 Jan 31, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport done to v1.15 in v1.15.0-rc.1 Jan 31, 2024
@michi-covalent michi-covalent moved this from Needs backport from main to Backport done to v1.15 in 1.15.1 Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.15 This issue will prevent the release of the next version of Cilium. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.15.1
Backport done to v1.15
v1.15.0-rc.1
Backport done to v1.15
Development

Successfully merging this pull request may close these issues.

cilium 1.15.0-rc.0 fails to start in IPv4 only environment
7 participants