Skip to content

Commit d22d1b8

Browse files
dylandreimerinkdylandreimerink
committed
bpf: Replace invoke_tailcall_if with C if-else
We currently use the `invoke_tailcall_if` macro to conditionally perform a tail call or inline the same function based on configuration. This macro will only work with compile time constants. So since we are preparing to move to load time configuration, we need to replace these macros with regular if-else statements. At present all conditionals are still compile time constants, so the result should be the same. We simply rely on the compilers constant propagation to optimize away the dead branches. The big difference will be when conditionals are migrated to load time configuration. We already have unused tail call pruning based on load time configuration in place, so after this we should just be able to migrate the conditionals to load time configuration one by one. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
1 parent 1b6b86a commit d22d1b8

File tree

5 files changed

+87
-105
lines changed

5 files changed

+87
-105
lines changed

‎bpf/bpf_host.c‎

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -455,19 +455,22 @@ tail_handle_ipv6(struct __ctx_buff *ctx, __u32 ipcache_srcid, const bool from_ho
455455
return ret;
456456

457457
ctx_store_meta(ctx, CB_SRC_LABEL, src_sec_identity);
458-
if (from_host)
459-
ret = invoke_tailcall_if(is_defined(ENABLE_HOST_FIREWALL),
460-
CILIUM_CALL_IPV6_CONT_FROM_HOST,
461-
tail_handle_ipv6_cont_from_host,
462-
&ext_err);
463-
else
464-
ret = invoke_tailcall_if(is_defined(ENABLE_HOST_FIREWALL),
465-
CILIUM_CALL_IPV6_CONT_FROM_NETDEV,
466-
tail_handle_ipv6_cont_from_netdev,
467-
&ext_err);
458+
if (from_host) {
459+
if (is_defined(ENABLE_HOST_FIREWALL))
460+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV6_CONT_FROM_HOST,
461+
&ext_err);
462+
else
463+
ret = tail_handle_ipv6_cont_from_host(ctx);
464+
} else {
465+
if (is_defined(ENABLE_HOST_FIREWALL))
466+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV6_CONT_FROM_NETDEV,
467+
&ext_err);
468+
else
469+
ret = tail_handle_ipv6_cont_from_netdev(ctx);
470+
}
468471
}
469472

470-
/* Catch errors from both handle_ipv6 and invoke_tailcall_if here. */
473+
/* Catch errors from both handle_ipv6 and tail_call_internal here. */
471474
if (IS_ERR(ret))
472475
return send_drop_notify_error_ext(ctx, src_sec_identity, ret, ext_err,
473476
METRIC_INGRESS);
@@ -911,19 +914,22 @@ tail_handle_ipv4(struct __ctx_buff *ctx, __u32 ipcache_srcid, const bool from_ho
911914
return ret;
912915

913916
ctx_store_meta(ctx, CB_SRC_LABEL, src_sec_identity);
914-
if (from_host)
915-
ret = invoke_tailcall_if(is_defined(ENABLE_HOST_FIREWALL),
916-
CILIUM_CALL_IPV4_CONT_FROM_HOST,
917-
tail_handle_ipv4_cont_from_host,
918-
&ext_err);
919-
else
920-
ret = invoke_tailcall_if(is_defined(ENABLE_HOST_FIREWALL),
921-
CILIUM_CALL_IPV4_CONT_FROM_NETDEV,
922-
tail_handle_ipv4_cont_from_netdev,
923-
&ext_err);
917+
if (from_host) {
918+
if (is_defined(ENABLE_HOST_FIREWALL))
919+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV4_CONT_FROM_HOST,
920+
&ext_err);
921+
else
922+
ret = tail_handle_ipv4_cont_from_host(ctx);
923+
} else {
924+
if (is_defined(ENABLE_HOST_FIREWALL))
925+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV4_CONT_FROM_NETDEV,
926+
&ext_err);
927+
else
928+
ret = tail_handle_ipv4_cont_from_netdev(ctx);
929+
}
924930
}
925931

926-
/* Catch errors from both handle_ipv4 and invoke_tailcall_if here. */
932+
/* Catch errors from both handle_ipv4 and tail_call_internal here. */
927933
if (IS_ERR(ret))
928934
return send_drop_notify_error_ext(ctx, src_sec_identity, ret, ext_err,
929935
METRIC_INGRESS);
@@ -1805,24 +1811,22 @@ to_host_from_lxc(struct __ctx_buff *ctx)
18051811
case bpf_htons(ETH_P_IPV6):
18061812
ctx_store_meta(ctx, CB_SRC_LABEL, 0);
18071813
ctx_store_meta(ctx, CB_TRACED, 1);
1808-
ret = invoke_tailcall_if(__or(__and(is_defined(ENABLE_IPV4),
1809-
is_defined(ENABLE_IPV6)),
1810-
is_defined(DEBUG)),
1811-
CILIUM_CALL_IPV6_TO_HOST_POLICY_ONLY,
1812-
tail_ipv6_host_policy_ingress,
1813-
&ext_err);
1814+
if ((is_defined(ENABLE_IPV4) && is_defined(ENABLE_IPV6)) || is_defined(DEBUG))
1815+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV6_TO_HOST_POLICY_ONLY,
1816+
&ext_err);
1817+
else
1818+
ret = tail_ipv6_host_policy_ingress(ctx);
18141819
break;
18151820
# endif
18161821
# ifdef ENABLE_IPV4
18171822
case bpf_htons(ETH_P_IP):
18181823
ctx_store_meta(ctx, CB_SRC_LABEL, 0);
18191824
ctx_store_meta(ctx, CB_TRACED, 1);
1820-
ret = invoke_tailcall_if(__or(__and(is_defined(ENABLE_IPV4),
1821-
is_defined(ENABLE_IPV6)),
1822-
is_defined(DEBUG)),
1823-
CILIUM_CALL_IPV4_TO_HOST_POLICY_ONLY,
1824-
tail_ipv4_host_policy_ingress,
1825-
&ext_err);
1825+
if ((is_defined(ENABLE_IPV4) && is_defined(ENABLE_IPV6)) || is_defined(DEBUG))
1826+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV4_TO_HOST_POLICY_ONLY,
1827+
&ext_err);
1828+
else
1829+
ret = tail_ipv4_host_policy_ingress(ctx);
18261830
break;
18271831
# endif
18281832
default:

‎bpf/bpf_lxc.c‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,11 @@ int NAME(struct __ctx_buff *ctx) \
351351
return drop_for_direction(ctx, DIR, DROP_INVALID_TC_BUFFER, \
352352
ext_err); \
353353
\
354-
ret = invoke_tailcall_if(CONDITION, TARGET_ID, TARGET_NAME, &ext_err); \
354+
if (CONDITION) \
355+
ret = tail_call_internal(ctx, TARGET_ID, &ext_err); \
356+
else \
357+
ret = TARGET_NAME(ctx); \
358+
\
355359
if (IS_ERR(ret)) \
356360
return drop_for_direction(ctx, DIR, ret, ext_err); \
357361
\
@@ -413,7 +417,11 @@ int NAME(struct __ctx_buff *ctx) \
413417
return drop_for_direction(ctx, DIR, DROP_INVALID_TC_BUFFER, \
414418
ext_err); \
415419
\
416-
ret = invoke_tailcall_if(CONDITION, TARGET_ID, TARGET_NAME, &ext_err); \
420+
if (CONDITION) \
421+
ret = tail_call_internal(ctx, TARGET_ID, &ext_err); \
422+
else \
423+
ret = TARGET_NAME(ctx); \
424+
\
417425
if (IS_ERR(ret)) \
418426
return drop_for_direction(ctx, DIR, ret, ext_err); \
419427
\
@@ -1916,7 +1924,7 @@ int tail_ipv6_to_endpoint(struct __ctx_buff *ctx)
19161924

19171925
TAIL_CT_LOOKUP6(CILIUM_CALL_IPV6_CT_INGRESS_POLICY_ONLY,
19181926
tail_ipv6_ct_ingress_policy_only, CT_INGRESS,
1919-
__and(is_defined(ENABLE_IPV4), is_defined(ENABLE_IPV6)),
1927+
(is_defined(ENABLE_IPV4) && is_defined(ENABLE_IPV6)),
19201928
CILIUM_CALL_IPV6_TO_LXC_POLICY_ONLY, tail_ipv6_policy)
19211929

19221930
TAIL_CT_LOOKUP6(CILIUM_CALL_IPV6_CT_INGRESS, tail_ipv6_ct_ingress, CT_INGRESS,
@@ -2267,7 +2275,7 @@ int tail_ipv4_to_endpoint(struct __ctx_buff *ctx)
22672275

22682276
TAIL_CT_LOOKUP4(CILIUM_CALL_IPV4_CT_INGRESS_POLICY_ONLY,
22692277
tail_ipv4_ct_ingress_policy_only, CT_INGRESS,
2270-
__and(is_defined(ENABLE_IPV4), is_defined(ENABLE_IPV6)),
2278+
(is_defined(ENABLE_IPV4) && is_defined(ENABLE_IPV6)),
22712279
CILIUM_CALL_IPV4_TO_LXC_POLICY_ONLY, tail_ipv4_policy)
22722280

22732281
TAIL_CT_LOOKUP4(CILIUM_CALL_IPV4_CT_INGRESS, tail_ipv4_ct_ingress, CT_INGRESS,
@@ -2302,17 +2310,21 @@ int cil_lxc_policy(struct __ctx_buff *ctx)
23022310
switch (proto) {
23032311
#ifdef ENABLE_IPV6
23042312
case bpf_htons(ETH_P_IPV6):
2305-
ret = invoke_tailcall_if(__and(is_defined(ENABLE_IPV4), is_defined(ENABLE_IPV6)),
2306-
CILIUM_CALL_IPV6_CT_INGRESS_POLICY_ONLY,
2307-
tail_ipv6_ct_ingress_policy_only, &ext_err);
2313+
if (is_defined(ENABLE_IPV4) && is_defined(ENABLE_IPV6))
2314+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV6_CT_INGRESS_POLICY_ONLY,
2315+
&ext_err);
2316+
else
2317+
ret = tail_ipv6_ct_ingress_policy_only(ctx);
23082318
sec_label = SECLABEL_IPV6;
23092319
break;
23102320
#endif /* ENABLE_IPV6 */
23112321
#ifdef ENABLE_IPV4
23122322
case bpf_htons(ETH_P_IP):
2313-
ret = invoke_tailcall_if(__and(is_defined(ENABLE_IPV4), is_defined(ENABLE_IPV6)),
2314-
CILIUM_CALL_IPV4_CT_INGRESS_POLICY_ONLY,
2315-
tail_ipv4_ct_ingress_policy_only, &ext_err);
2323+
if (is_defined(ENABLE_IPV4) && is_defined(ENABLE_IPV6))
2324+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV4_CT_INGRESS_POLICY_ONLY,
2325+
&ext_err);
2326+
else
2327+
ret = tail_ipv4_ct_ingress_policy_only(ctx);
23162328
sec_label = SECLABEL_IPV4;
23172329
break;
23182330
#endif /* ENABLE_IPV4 */

‎bpf/lib/nodeport.h‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,13 +1146,12 @@ int tail_nodeport_nat_ingress_ipv6(struct __ctx_buff *ctx)
11461146
ctx_skip_host_fw_set(ctx);
11471147
# endif
11481148

1149-
ret = invoke_traced_tailcall_if(__or(__and(is_defined(ENABLE_HOST_FIREWALL),
1150-
is_defined(IS_BPF_HOST)),
1151-
__and(is_defined(ENABLE_IPV6_FRAGMENTS),
1152-
is_defined(IS_BPF_XDP))),
1153-
CILIUM_CALL_IPV6_NODEPORT_REVNAT_INGRESS,
1154-
nodeport_rev_dnat_ingress_ipv6,
1155-
&trace, &ext_err);
1149+
if ((is_defined(ENABLE_HOST_FIREWALL) && is_defined(IS_BPF_HOST)) ||
1150+
(is_defined(ENABLE_IPV6_FRAGMENTS) && is_defined(IS_BPF_XDP)))
1151+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV6_NODEPORT_REVNAT_INGRESS, &ext_err);
1152+
else
1153+
ret = nodeport_rev_dnat_ingress_ipv6(ctx, &trace, &ext_err);
1154+
11561155
if (IS_ERR(ret))
11571156
goto drop_err;
11581157

@@ -2486,11 +2485,11 @@ int tail_nodeport_nat_ingress_ipv4(struct __ctx_buff *ctx)
24862485
* Also let nodeport_rev_dnat_ipv4() redirect EgressGW
24872486
* reply traffic into tunnel (see there for details).
24882487
*/
2489-
ret = invoke_traced_tailcall_if(__and(is_defined(ENABLE_HOST_FIREWALL),
2490-
is_defined(IS_BPF_HOST)),
2491-
CILIUM_CALL_IPV4_NODEPORT_REVNAT,
2492-
nodeport_rev_dnat_ipv4,
2493-
&trace, &ext_err);
2488+
if (is_defined(ENABLE_HOST_FIREWALL) && is_defined(IS_BPF_HOST))
2489+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV4_NODEPORT_REVNAT, &ext_err);
2490+
else
2491+
ret = nodeport_rev_dnat_ipv4(ctx, &trace, &ext_err);
2492+
24942493
if (IS_ERR(ret))
24952494
goto drop_err;
24962495

‎bpf/lib/nodeport_egress.h‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -682,28 +682,26 @@ handle_nat_fwd(struct __ctx_buff *ctx, __u32 cluster_id, __u32 src_id,
682682
switch (proto) {
683683
#ifdef ENABLE_IPV4
684684
case bpf_htons(ETH_P_IP):
685-
ret = invoke_traced_tailcall_if(__or4(__and(is_defined(ENABLE_IPV4),
686-
is_defined(ENABLE_IPV6)),
687-
__and(is_defined(ENABLE_HOST_FIREWALL),
688-
is_defined(IS_BPF_HOST)),
689-
__and(is_defined(ENABLE_CLUSTER_AWARE_ADDRESSING),
690-
is_defined(ENABLE_INTER_CLUSTER_SNAT)),
691-
__and(is_defined(ENABLE_EGRESS_GATEWAY_COMMON),
692-
is_defined(IS_BPF_HOST))),
693-
CILIUM_CALL_IPV4_NODEPORT_NAT_FWD,
694-
handle_nat_fwd_ipv4, trace, ext_err);
685+
if ((is_defined(ENABLE_IPV4) && is_defined(ENABLE_IPV6)) ||
686+
(is_defined(ENABLE_HOST_FIREWALL) && is_defined(IS_BPF_HOST)) ||
687+
(is_defined(ENABLE_CLUSTER_AWARE_ADDRESSING) &&
688+
is_defined(ENABLE_INTER_CLUSTER_SNAT)) ||
689+
(is_defined(ENABLE_EGRESS_GATEWAY_COMMON) && is_defined(IS_BPF_HOST)))
690+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV4_NODEPORT_NAT_FWD,
691+
ext_err);
692+
else
693+
ret = handle_nat_fwd_ipv4(ctx, trace, ext_err);
695694
break;
696695
#endif /* ENABLE_IPV4 */
697696
#ifdef ENABLE_IPV6
698697
case bpf_htons(ETH_P_IPV6):
699-
ret = invoke_traced_tailcall_if(__or3(__and(is_defined(ENABLE_IPV4),
700-
is_defined(ENABLE_IPV6)),
701-
__and(is_defined(ENABLE_HOST_FIREWALL),
702-
is_defined(IS_BPF_HOST)),
703-
__and(is_defined(ENABLE_EGRESS_GATEWAY_COMMON),
704-
is_defined(IS_BPF_HOST))),
705-
CILIUM_CALL_IPV6_NODEPORT_NAT_FWD,
706-
handle_nat_fwd_ipv6, trace, ext_err);
698+
if ((is_defined(ENABLE_IPV4) && is_defined(ENABLE_IPV6)) ||
699+
(is_defined(ENABLE_HOST_FIREWALL) && is_defined(IS_BPF_HOST)) ||
700+
(is_defined(ENABLE_EGRESS_GATEWAY_COMMON) && is_defined(IS_BPF_HOST)))
701+
ret = tail_call_internal(ctx, CILIUM_CALL_IPV6_NODEPORT_NAT_FWD,
702+
ext_err);
703+
else
704+
ret = handle_nat_fwd_ipv6(ctx, trace, ext_err);
707705
break;
708706
#endif /* ENABLE_IPV6 */
709707
default:

‎bpf/lib/tailcall.h‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -135,34 +135,3 @@ tail_call_internal(struct __ctx_buff *ctx, const __u32 index, __s8 *ext_err)
135135
*ext_err = (__s8)index;
136136
return DROP_MISSED_TAIL_CALL;
137137
}
138-
139-
/* invoke_tailcall_if() is a helper which based on COND either selects to emit
140-
* a tail call for the underlying function when true or emits it as inlined
141-
* when false. COND can be selected by one or multiple compile time flags.
142-
*
143-
* [...]
144-
* invoke_tailcall_if(__and(is_defined(ENABLE_IPV4), is_defined(ENABLE_IPV6)),
145-
* CILIUM_CALL_FOO, foo_fn);
146-
* [...]
147-
*
148-
* The loader will only load tail calls if they are invoked at least once.
149-
*/
150-
151-
#define __invoke_tailcall_if_0(NAME, FUNC, EXT_ERR) \
152-
FUNC(ctx)
153-
#define __invoke_tailcall_if_1(NAME, FUNC, EXT_ERR) \
154-
({ \
155-
tail_call_internal(ctx, NAME, EXT_ERR); \
156-
})
157-
#define invoke_tailcall_if(COND, NAME, FUNC, EXT_ERR) \
158-
__eval(__invoke_tailcall_if_, COND)(NAME, FUNC, EXT_ERR)
159-
160-
#define __invoke_traced_tailcall_if_0(NAME, FUNC, TRACE, EXT_ERR) \
161-
FUNC(ctx, TRACE, EXT_ERR)
162-
#define __invoke_traced_tailcall_if_1(NAME, FUNC, TRACE, EXT_ERR) \
163-
({ \
164-
tail_call_internal(ctx, NAME, EXT_ERR); \
165-
})
166-
#define invoke_traced_tailcall_if(COND, NAME, FUNC, TRACE, EXT_ERR) \
167-
__eval(__invoke_traced_tailcall_if_, COND)(NAME, FUNC, TRACE, \
168-
EXT_ERR)

0 commit comments

Comments
 (0)