Skip to content

Respond with ICMP reply for traffic to services without backends #28157

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.

Sign up for GitHub

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

Merged
dylandreimerink merged 10 commits into from
Nov 23, 2023
Merged

Respond with ICMP reply for traffic to services without backends #28157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7c6c662
option: Add `--service-no-backend-response` option
dylandreimerink Sep 8, 2023
ece129f
bpf: Differentiate `DROP_NO_SERVICE` and `DROP_UNSUPP_SERVICE_PROTO`
dylandreimerink Sep 8, 2023
901c3ed
bpf: Allow uneven lengths in `__bpf_memzero`
dylandreimerink Sep 8, 2023
3ba7ee6
bpf: Send ICMP unreachable when no service backends are available
dylandreimerink Sep 8, 2023
ca4d6bc
bpf,tests: Add tests for the ICMP response for services without backend
dylandreimerink Sep 8, 2023
1da5af5
complexity-tests: Add no service backend response to complexity tests
dylandreimerink Sep 8, 2023
840f385
test,l4lb: Change expected error in L4LB test
dylandreimerink Sep 11, 2023
306297e
probes: Added probe for bpf_skb_adjust_room + `BPF_ADJ_ROOM_MAC` mode
dylandreimerink Sep 12, 2023
54b2890
test,conformance-ingress: Change sanity check to deal with ICMP reply
dylandreimerink Sep 14, 2023
6c2ae70
bpf: Add ratelimiting feature and use it for ICMPv6 responses
dylandreimerink Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/conformance-ingress.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,10 @@ jobs:
- name: Run Sanity check (external)
timeout-minutes: 5
run: |
lb=$(kubectl get ingress basic-ingress -o jsonpath='{.status.loadBalancer.ingress[0].ip}')
curl -s -v --connect-timeout 5 --max-time 20 --retry 3 --fail -- http://"$lb"
lb=$(kubectl get ingress basic-ingress -o jsonpath='{.status.loadBalancer.ingress[0].ip}')
curl -s -v --connect-timeout 5 --max-time 20 --retry 3 --retry-all-errors --retry-delay 5 --fail -- http://"$lb"

# By now the service should be up, no need to do the manual retries for the second request
curl -s -v --connect-timeout 5 --max-time 20 --retry 3 --fail -- http://"$lb"/details/1

- name: Run Sanity check (internal to NodePort)
Expand Down
1 change: 1 addition & 0 deletions Documentation/cmdref/cilium-agent.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Documentation/helm-values.rst
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions api/v1/flow/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,7 @@ here.
| UNENCRYPTED_TRAFFIC | 195 | |
| TTL_EXCEEDED | 196 | |
| NO_NODE_ID | 197 | |
| DROP_RATE_LIMITED | 198 | |



Expand Down
354 changes: 179 additions & 175 deletions api/v1/flow/flow.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions api/v1/flow/flow.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ enum DropReason {
UNENCRYPTED_TRAFFIC = 195;
TTL_EXCEEDED = 196;
NO_NODE_ID = 197;
DROP_RATE_LIMITED = 198;
}

enum TrafficDirection {
Expand Down
1 change: 1 addition & 0 deletions api/v1/observer/observer.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 18 additions & 2 deletions bpf/bpf_lxc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ static __always_inline int __per_packet_lb_svc_xlate_4(void *ctx, struct iphdr *

ret = lb4_extract_tuple(ctx, ip4, ETH_HLEN, &l4_off, &tuple);
if (IS_ERR(ret)) {
if (ret == DROP_NO_SERVICE || ret == DROP_UNKNOWN_L4)
if (ret == DROP_UNSUPP_SERVICE_PROTO || ret == DROP_UNKNOWN_L4)
goto skip_service_lookup;
else
return ret;
Expand All @@ -108,6 +108,14 @@ static __always_inline int __per_packet_lb_svc_xlate_4(void *ctx, struct iphdr *
ret = lb4_local(get_ct_map4(&tuple), ctx, ipv4_is_fragment(ip4),
ETH_HLEN, l4_off, &key, &tuple, svc, &ct_state_new,
has_l4_header, false, &cluster_id, ext_err);

#ifdef SERVICE_NO_BACKEND_RESPONSE
if (ret == DROP_NO_SERVICE) {
ep_tail_call(ctx, CILIUM_CALL_IPV4_NO_SERVICE);
return DROP_MISSED_TAIL_CALL;
}
#endif

Comment thread
julianwiedmann marked this conversation as resolved.
if (IS_ERR(ret))
return ret;
}
Expand All @@ -133,7 +141,7 @@ static __always_inline int __per_packet_lb_svc_xlate_6(void *ctx, struct ipv6hdr

ret = lb6_extract_tuple(ctx, ip6, ETH_HLEN, &l4_off, &tuple);
if (IS_ERR(ret)) {
if (ret == DROP_NO_SERVICE || ret == DROP_UNKNOWN_L4)
if (ret == DROP_UNSUPP_SERVICE_PROTO || ret == DROP_UNKNOWN_L4)
goto skip_service_lookup;
else
return ret;
Expand All @@ -158,6 +166,14 @@ static __always_inline int __per_packet_lb_svc_xlate_6(void *ctx, struct ipv6hdr
#endif /* ENABLE_L7_LB */
ret = lb6_local(get_ct_map6(&tuple), ctx, ETH_HLEN, l4_off,
&key, &tuple, svc, &ct_state_new, false, ext_err);

#ifdef SERVICE_NO_BACKEND_RESPONSE
if (ret == DROP_NO_SERVICE) {
ep_tail_call(ctx, CILIUM_CALL_IPV6_NO_SERVICE);
return DROP_MISSED_TAIL_CALL;
}
#endif

if (IS_ERR(ret))
return ret;
}
Expand Down
1 change: 1 addition & 0 deletions bpf/complexity-tests/419/bpf_lxc/4.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@
-DENABLE_IPV4_FRAGMENTS=1
-DENCAP_IFINDEX=1
-DTUNNEL_MODE=1
-DSERVICE_NO_BACKEND_RESPONSE=1
Comment thread
dylandreimerink marked this conversation as resolved.
1 change: 1 addition & 0 deletions bpf/complexity-tests/510/bpf_host/4.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/510/bpf_host/5.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/510/bpf_host/6.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/510/bpf_lxc/4.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/510/bpf_lxc/5.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/510/bpf_lxc/6.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/54/bpf_host/4.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/54/bpf_host/5.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/54/bpf_host/6.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/54/bpf_lxc/4.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DENABLE_CLUSTER_AWARE_ADDRESSING=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/54/bpf_lxc/5.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/54/bpf_lxc/6.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@
-DENABLE_EGRESS_GATEWAY=1
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/netnext/bpf_host/4.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DENABLE_HIGH_SCALE_IPCACHE=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/netnext/bpf_host/5.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DENABLE_HIGH_SCALE_IPCACHE=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/netnext/bpf_host/6.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DENABLE_HIGH_SCALE_IPCACHE=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/netnext/bpf_lxc/4.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@
-DENABLE_SCTP=1
-DENABLE_CLUSTER_AWARE_ADDRESSING=1
-DENABLE_HIGH_SCALE_IPCACHE=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/netnext/bpf_lxc/5.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DENABLE_HIGH_SCALE_IPCACHE=1
-DSERVICE_NO_BACKEND_RESPONSE=1
1 change: 1 addition & 0 deletions bpf/complexity-tests/netnext/bpf_lxc/6.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@
-DENABLE_VTEP=1
-DENABLE_SCTP=1
-DENABLE_HIGH_SCALE_IPCACHE=1
-DSERVICE_NO_BACKEND_RESPONSE=1
5 changes: 5 additions & 0 deletions bpf/include/bpf/builtins.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ static __always_inline void __bpf_memzero(void *d, __u64 len)

d += len;

if (len > 1 && len % 2 == 1) {
__it_set(d, 8);
len -= 1;
}

switch (len) {
case 96: __it_set(d, 64); fallthrough;
case 88: jmp_88: __it_set(d, 64); fallthrough;
Expand Down
7 changes: 5 additions & 2 deletions bpf/lib/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,9 @@ enum {
#define CILIUM_CALL_IPV4_CONT_FROM_NETDEV 44
#define CILIUM_CALL_IPV6_CONT_FROM_HOST 45
#define CILIUM_CALL_IPV6_CONT_FROM_NETDEV 46
#define CILIUM_CALL_SIZE 47
#define CILIUM_CALL_IPV4_NO_SERVICE 47
#define CILIUM_CALL_IPV6_NO_SERVICE 48
#define CILIUM_CALL_SIZE 49

typedef __u64 mac_t;

Expand Down Expand Up @@ -597,7 +599,7 @@ enum {
#define DROP_INVALID_EXTHDR -156
#define DROP_FRAG_NOSUPPORT -157
#define DROP_NO_SERVICE -158
#define DROP_UNUSED8 -159 /* unused */
Copy link
Copy Markdown
Contributor

@ti-mo ti-mo Jan 5, 2024

Choose a reason for hiding this comment

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

@dylandreimerink This drop reason wasn't added in flow.proto and drop.go.

Under normal circumstances, we shouldn't reuse any of these, since renaming a proto field/type causes a backwards-incompatible change. We're lucky that in this case, drop reason 159 is actually missing from the proto as well as from drop.go. 😅

    SERVICE_BACKEND_NOT_FOUND = 158;
    NO_TUNNEL_OR_ENCAPSULATION_ENDPOINT = 160;

In any case, I'm marking all unused ones as deprecated in #29482.

cc @rolinh

#define DROP_UNSUPP_SERVICE_PROTO -159
#define DROP_NO_TUNNEL_ENDPOINT -160
#define DROP_NAT_46X64_DISABLED -161
#define DROP_EDT_HORIZON -162
Expand Down Expand Up @@ -636,6 +638,7 @@ enum {
#define DROP_UNENCRYPTED_TRAFFIC -195
#define DROP_TTL_EXCEEDED -196
#define DROP_NO_NODE_ID -197
#define DROP_RATE_LIMITED -198

#define NAT_PUNT_TO_STACK DROP_NAT_NOT_NEEDED
#define NAT_NEEDED CTX_ACT_OK
Expand Down
Loading