datapath: pass IPv6 NDP traffic to stack without policy check

[jschwinger233]

Previously, our policy check for IPv6 NDP traffic caused issues such as #23852 and #23910 because this traffic was identified as WORLD_ID, which would be given a verdict of drop when CiliumNetworkPolicy is applied for per-endpoint routing.

To resolve this issue, we pass all IPv6 NDP traffic to the stack without policy check.

This change aligns with how we handle IPv4 ARP: the cilium bpf never performs policy check for ARP, regardless of whether we enable ENABLE_ARP_PASSTHROUGH or ENABLE_ARP_RESPONDER.

Fixes: #23852
Fixes: #23910

Signed-off-by: Zhichuan Liang gray.liang@isovalent.com

Bypassing policy check for IPv6 NDP to fix broken pod-to-pod connectivity when per-endpoint route is enabled with policy.

[jschwinger233]

@brb We don't really need to abandon ICMPv6 ND responder to resolve #23852 and #23910, just simply let the ND traffic pass to stack without policy check. Do you think this is an acceptable solution?

[brb]

We don't really need to abandon ICMPv6 ND responder

SGTM! Just for what packets the built-in responder would be still used?

[jschwinger233]

Just for what packets the built-in responder would be still used?

All the ICMPv6 NS packets from containers are handled by responder, and the NS in the opposite direction, from host to container, are on kernel. The bpf program will lookup the endpoints map by NS target IPv6, then send back the ICMPv6 ND after amending the packet payload in place. Because container's IPv6 routing gateway is set to host router IPv6, almost all NS packets from container are targeting host router IPv6, and responder composes ND with lxc L2 address.

As mentioned in #23852 (comment), simply removing the NS responder doesn't make kernel capable of handling these NS packets. I managed to remove responder in a draft PR https://github.com/cilium/cilium/pull/24778/files by changing IPv6 gateway address inside each container during CNI runtime, it worked, in spite of breaking the Docker CNM plugin, but my main concern is, we must introduce a new option like ENABLE_IPV6_ND_RESPONDER, while I can't answer the question "for what scenario we should enable this option". So I think maybe just keep it simple.

[jschwinger233]

/test

[jschwinger233]

Just for what packets the built-in responder would be still used?

Hold on, there are more places using ND responder, for example, ingress of eth0 when nodport / wireguard / host firewall is enabled.

That causes #14509 due to our responder setting the NA packet's source ipv6 to router ip, and some users complained about this behavior because their firewall/anti-spoofing filter doesn't recognize router ip and drops these NA packets as a result.

I think I still need to remove ND responder to address that issue.

But this PR could be independent to that. This PR is adequate to address #23910 and #23852, I'll open a new PR to remove responder.

[brb]

Thanks!

[github-actions]

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

[jschwinger233]

/test

[jschwinger233]

/ci-ginkgo

[jschwinger233]

😿 can't trigger Conformance ginkgo (ci-ginkgo)
working now

[tamilmani1989]

can this fix be backported to 1.13?

PS: I tried to cherry-pick this PR and this one #24921 on top on 1.13.10 and its not direct and have to resolve a conflict and couple of minor fixes. wanted to see if we can still backport this to 1.13 with those changes?

[jschwinger233]

@tamilmani1989 I'm not sure, because all those works are based on #23445, which is not likely to be backported to 1.13.