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: require BPF masq to enable --install-no-conntrack-iptables-rules #16085

Merged
merged 1 commit into from May 17, 2021

Conversation

jibi
Copy link
Member

@jibi jibi commented May 11, 2021

It's currently possible to enable the no CT Iptables rules together with
Iptables masquerading, which results in Iptables failing to masquerade
traffic.

With this commit, when this setup is detected, we return a fatal error.

Fixes: #16046

Signed-off-by: Gilberto Bertin gilberto@isovalent.com

It's currently possible to enable the no CT Iptables rules together with
Iptables masquerading, which results in Iptables failing to masquerade
traffic.

With this commit, when this setup is detected, we return a fatal error.

Fixes: #16046

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
@jibi jibi added area/daemon Impacts operation of the Cilium daemon. release-note/bug This PR fixes an issue in a previous release of Cilium. needs-backport/1.10 labels May 11, 2021
@jibi jibi requested review from brb, pchaigno and a team May 11, 2021 17:25
@jibi jibi requested a review from a team as a code owner May 11, 2021 17:25
@jibi jibi requested a review from qmonnet May 11, 2021 17:25
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.0-rc2 May 11, 2021
@jibi
Copy link
Member Author

jibi commented May 11, 2021

test-me-please

@joestringer
Copy link
Member

Per the issue this is fixing, this is considered a subfeature that can be addressed after v1.10.0, so removing release-blocker:
#16046 (comment)

If you really think this should be a release blocker, please respond with a description why and create a thread in the Slack #launchpad channel to bring attention to it so we can reconsider its blocking status.

@pchaigno
Copy link
Member

I marked as a release blocker for 1.10 because it's a fairly trivial (so going to land soon) and will break masquerading in a non-obvious way for users running with KPR enabled and BPF masquerading disabled.

But honestly, I'm not sure why we're spending time discussing that label when this PR is ready to merge anyway. k8s-1.16-kernel-netnext failed with a clearly unrelated issue (I filed #16127 for it), other tests are passing, and reviews are in. Marking appropriately.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 12, 2021
@aanm aanm added this to Needs backport from master in 1.10.0-rc3 May 17, 2021
@aanm aanm removed this from Needs backport from master in 1.10.0-rc2 May 17, 2021
@tklauser tklauser merged commit 8aed9d9 into master May 17, 2021
@tklauser tklauser deleted the pr/jibi/require-bpf-masq-for-no-conntrack-rules branch May 17, 2021 10:28
@aanm aanm moved this from Needs backport from master to Backport done to v1.10 in 1.10.0-rc3 May 19, 2021
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. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.10.0-rc3
Backport done to v1.10
Development

Successfully merging this pull request may close these issues.

--install-no-conntrack-iptables-rules prevents ipv4 masquerading
8 participants