[v1.14] re-introduce ICMPv6 NS responder on from-netdev #31186
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maintainer-s-little-helper
bot
added
backport/1.14
jschwinger233
force-pushed
the
gray/v1.14/icmp6-fix
branch
3 times, most recently
from
add0132 to
cdd43e7
Compare
|
/test-backport-1.14 |
jschwinger233
added
sig/datapath
|
Had some 👀 already, looks good! One note re backporting 44fcc5f would be that we could also add the offending test to |
[ upstream commit: 283b8b7 ] [ backporter's notes: minor conflicts in wireguard.h ] Under the hood this uses ctx_load_bytes(), which can fail. Return such an error to the caller. Signed-off-by: Julian Wiedmann <jwi@isovalent.com> Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
[ upstream commit: ffbd7af ] Right now the helper takes a L3 offset, and assumes a packet without extension headers. Change it to a L4 offset, so that callers can pass this when available. Signed-off-by: Julian Wiedmann <jwi@isovalent.com> Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
[ upstream commit: a28f0fc ] Replace open-coded variants of the helper. Signed-off-by: Julian Wiedmann <jwi@isovalent.com> Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
jschwinger233
force-pushed
the
gray/v1.14/icmp6-fix
branch
from
cdd43e7 to
fb2639e
Compare
[ upstream commit: 60c5e76 ] [ backporter's notes: adds tc_nodeport_l3_dev.o into NOCOVER_PATTERN to avoid verifier issue as v1.14 still has bpf coverage test. ] SKIP_ICMPV6_NS_HANDLING was there to pass bpf coverage test, which is gone by cilium#28090. In the meantime, removing SKIP_ICMPV6_NS_HANDLING from tc_nodeport_l3_dev.c prevents "potential missed tailcall" errors introduced by cilium#30467, as tail_icmp6_handle_ns() doesn't exist when SKIP_ICMPV6_NS_HANDLING is defined, but still gets tail-called by icmp6_handle_ns(). Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
[ upstream commit: dc9dfd7 ] [ backporter's notes: in v1.14 icmp6_handle_ns() doesn't have ext_err parameter, so icmp6_host_handle() doesn't need to accept it either. ] This reverts commit 6580714, to fix the breakage of "IPv6 NS responder for pod" introduced by cilium#12086 (bpf: Reply NA when recv ND for local IPv6 endpoints). 6580714 was merged to solve cilium#14509. To not revive cilium#14509, this commit also passes through ICMPv6 NS if the target is native node IP (eth0's addr). By letting stack take care of those NS-for-node-IP packets, we managed to: 1. Solve cilium#14509 again, but in a way keeping NS responder. The cause of cilium#14509 was NS responder always generates ND whose source IP is "router_ip" (cilium_internal_ip) rather than "node_ip". Once we pass those NS-for-node-IP packets to stack, the ND response would naturally have "node_ip" as source. 2. Avoid the fib_lookup failure mentioned at cilium#30837 (comment). icmp6_host_handle() also has a new parameter `handle_ns` to control if we want NS responder to be active. If it is called from `to-netdev` code path, handle_ns is set to false. This is suggested by julianwiedmann. Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com> Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
[ upstream commit: 8d4db89 ] [ backporter's notes: minor changes due to lack of cilium#30467 and cilium#27134 ] This commit adds bpf/tests/ipv6_ndp_from_netdev_test.c to cover two scenarios: 1. from_netdev receives IPv6 NS for a pod IP on the same host 2. from_netdev receives IPv6 NS for the node IP (eth0's addr) For case 1, from_netdev should return a NA on behalf of the target pod to avoid cilium#30926. for case 2, it must return the NS to stack to address cilium#14509. Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com> Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
[ upstream commit: 475a194 ] [ backporter's notes: minor conflict due to v1.15 icmp6_host_handle() doesn't have ext_err parameter. ] The ICMPv6 handling in handle_ipv6() is only required for the HostFW or by from-netdev. Exclude it otherwise. This is a minor optimization for dc9dfd7 ("bpf: Re-introduce ICMPv6 NS responder on from-netdev"). Signed-off-by: Julian Wiedmann <jwi@isovalent.com> Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
jschwinger233
force-pushed
the
gray/v1.14/icmp6-fix
branch
from
fb2639e to
8ec8bf7
Compare
|
/test-backport-1.14 |
aanm
approved these changes
Mar 7, 2024
brlbil
approved these changes
Mar 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With a lot of conflicts with new verifier issues, this PR ends up with one additional PR #26563 to get backport easier.
v1.14 still has bpf coverage test, and doesn't have some bpf test helpers (e.g.
endpoint_v6_add_entry). Some pkt_gen helpers also had verifier issues(e.g.pktgen__push_rawhdr). I made some efforts to pass the new icmpv6 bpf test.Once this PR is merged, a GitHub action will update the labels of these PRs: