Skip to content

Commit

Permalink
bpf: Remove ICMPv6 NS Responder on bpf_host
Browse files Browse the repository at this point in the history
This commit removes the ICMPv6 NS responder from from-netdev, to-netdev,
and from-host.

Let me explain why this removal won't break anything.

First we need to know NS responder handles packets targeting local
router or one of the local endpoints, and here is the responding steps
in details:

1. if NS is targeting the local router;
2. or if NS is targeting a local endpoint;
3. transform the packet into an ICMPv6 NA with cilium_host's MAC address
   as the response;
4. modify the packet L3, set the source IPv6 to the router IPv6;
5. redirect_self;

From-netdev is on the native devices handling ingress traffic from other
nodes. The ingress traffic should be underlay traffic that doesn't
expose overlay addresses, like pod or router addresses. Therefore, NS
traffic reaching from-netdev can only target native IPv6, and we don't
need the responder to deal with that.

To-netdev and from-host are in the same host network namespace, one is
attached to native devices, and the other is on cilium_host. Prior to
issue cilium#23445, we needed NS responder because no host device had that
router IPv6. Since cilium#23445 has been resolved, now curling from host to a
local pod or local router doesn't require bpf NS responder anymore.

The removal of the responder on from-netdev also fixes a known issue
 cilium#14509, which is caused by forementioned step 4: NS responder sets
responding packet's source IPv6 to router IPv6 even if the NS comes from
a native device.

Fixes: cilium#14509

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
  • Loading branch information
jschwinger233 authored and pchaigno committed May 10, 2023
1 parent 14fcc11 commit 6580714
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 9 deletions.
9 changes: 7 additions & 2 deletions bpf/bpf_host.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ handle_ipv6(struct __ctx_buff *ctx, __u32 secctx __maybe_unused,
struct ct_buffer6 __maybe_unused ct_buffer = {};
void *data, *data_end;
struct ipv6hdr *ip6;
int ret, hdrlen;
int __maybe_unused ret;
int hdrlen;
__u8 nexthdr;
#ifdef ENABLE_HOST_FIREWALL
bool need_hostfw = false;
Expand All @@ -164,13 +165,15 @@ handle_ipv6(struct __ctx_buff *ctx, __u32 secctx __maybe_unused,
if (hdrlen < 0)
return hdrlen;

#ifdef ENABLE_HOST_FIREWALL
if (likely(nexthdr == IPPROTO_ICMPV6)) {
ret = icmp6_host_handle(ctx);
if (ret == SKIP_HOST_FIREWALL)
goto skip_host_firewall;
if (IS_ERR(ret))
return ret;
}
#endif /* ENABLE_HOST_FIREWALL */

#ifdef ENABLE_NODEPORT
if (!from_host) {
Expand Down Expand Up @@ -214,8 +217,8 @@ handle_ipv6(struct __ctx_buff *ctx, __u32 secctx __maybe_unused,
}
#endif /* ENABLE_HOST_FIREWALL */

skip_host_firewall:
#ifdef ENABLE_HOST_FIREWALL
skip_host_firewall:
ctx_store_meta(ctx, CB_FROM_HOST,
(need_hostfw ? FROM_HOST_FLAG_NEED_HOSTFW : 0));
#endif /* ENABLE_HOST_FIREWALL */
Expand Down Expand Up @@ -476,13 +479,15 @@ handle_to_netdev_ipv6(struct __ctx_buff *ctx, struct trace_ctx *trace, __s8 *ext
if (hdrlen < 0)
return hdrlen;

#ifdef ENABLE_HOST_FIREWALL
if (likely(nexthdr == IPPROTO_ICMPV6)) {
ret = icmp6_host_handle(ctx);
if (ret == SKIP_HOST_FIREWALL)
return CTX_ACT_OK;
if (IS_ERR(ret))
return ret;
}
#endif /* ENABLE_HOST_FIREWALL */

if ((ctx->mark & MARK_MAGIC_HOST_MASK) == MARK_MAGIC_HOST)
src_id = HOST_ID;
Expand Down
11 changes: 4 additions & 7 deletions bpf/lib/icmp6.h
Original file line number Diff line number Diff line change
Expand Up @@ -397,10 +397,7 @@ icmp6_host_handle(struct __ctx_buff *ctx __maybe_unused)
__u8 type __maybe_unused;

type = icmp6_load_type(ctx, ETH_HLEN);
if (type == ICMP6_NS_MSG_TYPE)
return icmp6_handle_ns(ctx, ETH_HLEN, METRIC_INGRESS);

#ifdef ENABLE_HOST_FIREWALL
/* When the host firewall is enabled, we drop and allow ICMPv6 messages
* according to RFC4890, except for echo request and reply messages which
* are handled by host policies and can be dropped.
Expand All @@ -420,7 +417,7 @@ icmp6_host_handle(struct __ctx_buff *ctx __maybe_unused)
* | ICMPv6-mult-list-done | CTX_ACT_OK | 132 |
* | ICMPv6-router-solici | CTX_ACT_OK | 133 |
* | ICMPv6-router-advert | CTX_ACT_OK | 134 |
* | ICMPv6-neighbor-solicit | icmp6_handle_ns | 135 |
* | ICMPv6-neighbor-solicit | CTX_ACT_OK | 135 |
* | ICMPv6-neighbor-advert | CTX_ACT_OK | 136 |
* | ICMPv6-redirect-message | CTX_ACT_DROP | 137 |
* | ICMPv6-router-renumber | CTX_ACT_OK | 138 |
Expand Down Expand Up @@ -448,6 +445,9 @@ icmp6_host_handle(struct __ctx_buff *ctx __maybe_unused)
* | ICMPv6-unassigned | CTX_ACT_DROP | |
*/

if (type == ICMP6_NS_MSG_TYPE)
return CTX_ACT_OK;

if (type == ICMP6_ECHO_REQUEST_MSG_TYPE || type == ICMP6_ECHO_REPLY_MSG_TYPE)
/* Decision is deferred to the host policies. */
return CTX_ACT_OK;
Expand All @@ -459,9 +459,6 @@ icmp6_host_handle(struct __ctx_buff *ctx __maybe_unused)
(ICMP6_MULT_RA_MSG_TYPE <= type && type <= ICMP6_MULT_RT_MSG_TYPE))
return SKIP_HOST_FIREWALL;
return DROP_FORBIDDEN_ICMP6;
#else
return CTX_ACT_OK;
#endif /* ENABLE_HOST_FIREWALL */
}

#endif

0 comments on commit 6580714

Please sign in to comment.