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: Fatal on BPF masquerade + IPv6 masquerade #17906

Merged
merged 1 commit into from Nov 22, 2021

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented Nov 16, 2021

BPF masquerading for IPv6 isn't supported yet, so we should fatal early if the user asks for both BPF and IPv6 masquerade. They can use iptables-based masquerading for IPv6 instead.

Since we enable BPF-based masquerading in all tests with 4.19+ kernels, we also need to disable IPv6 masquerading there. That should be fine since we rarely rely on IPv6 masquerading anyway.

Fix bug where the agents would silently skip all IPv6 masquerading due to an incorrect configuration.

@pchaigno pchaigno 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. 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 Nov 16, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.6 Nov 16, 2021
@borkmann
Copy link
Member

What is missing for full support? Rather than just rejecting such config and deferring to later, I would rather be in favor of fixing it.

We have snat_v4_needed() and snat_v6_needed(), is it mainly to extend the latter in order to cover all cases in similar way, @brb ? Maybe I can take a look at it. Our BPF NAT engine definitely supports v6.

@brb
Copy link
Member

brb commented Nov 17, 2021

We have snat_v4_needed() and snat_v6_needed(), is it mainly to extend the latter in order to cover all cases in similar way,

It's basically copy-pasting snat_v4_needed to snat_v6_needed (minus egress gw and ip masq agent functionality).

BPF masquerading for IPv6 isn't supported yet, so we should fatal early
if the user asks for both BPF and IPv6 masquerade. They can use
iptables-based masquerading for IPv6 instead.

Since we enable BPF-based masquerading in all tests with 4.19+ kernels,
we also need to disable IPv6 masquerading there. That should be fine
since we rarely rely on IPv6 masquerading anyway.

Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno
Copy link
Member Author

/test

@pchaigno pchaigno marked this pull request as ready for review November 18, 2021 09:59
@pchaigno pchaigno requested a review from a team November 18, 2021 09:59
@pchaigno pchaigno requested a review from a team as a code owner November 18, 2021 09:59
@maintainer-s-little-helper maintainer-s-little-helper bot assigned jibi and nbusseneau and unassigned jibi Nov 18, 2021
Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

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

Thanks.

@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 Nov 19, 2021
@gandro gandro merged commit d60cdef into cilium:master Nov 22, 2021
@pchaigno pchaigno deleted the fatal-ipv6-bpf-masquerading branch November 22, 2021 10:19
@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.6 Nov 23, 2021
@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.6 Nov 23, 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.6 Nov 30, 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.6 Nov 30, 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. 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-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
No open projects
1.10.6
Backport done to v1.10
Development

Successfully merging this pull request may close these issues.

None yet

7 participants