[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1f403e4e-d790-7818-5728-2f79d7c1b051@mojatatu.com>
Date: Mon, 5 Dec 2022 15:45:58 -0300
From: Pedro Tammela <pctammela@...atatu.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
pabeni@...hat.com, jhs@...atatu.com, xiyou.wangcong@...il.com,
jiri@...nulli.us, kuniyu@...zon.com,
Victor Nogueira <victor@...atatu.com>
Subject: Re: [PATCH net-next v3 2/4] net/sched: add retpoline wrapper for tc
On 05/12/2022 14:23, Eric Dumazet wrote:
> On Mon, Dec 5, 2022 at 6:16 PM Pedro Tammela <pctammela@...atatu.com> wrote:
>>
>> On kernels compiled with CONFIG_RETPOLINE and CONFIG_NET_TC_INDIRECT_WRAPPER,
>> optimize actions and filters that are compiled as built-ins into a direct call.
>> The calls are ordered according to relevance. Testing data shows that
>> the pps difference between first and last is between 0.5%-1.0%.
>>
>> On subsequent patches we expose the classifiers and actions functions
>> and wire up the wrapper into tc.
>>
>> Signed-off-by: Pedro Tammela <pctammela@...atatu.com>
>> Reviewed-by: Jamal Hadi Salim <jhs@...atatu.com>
>> Reviewed-by: Victor Nogueira <victor@...atatu.com>
>> ---
>> include/net/tc_wrapper.h | 226 +++++++++++++++++++++++++++++++++++++++
>> net/sched/Kconfig | 13 +++
>> 2 files changed, 239 insertions(+)
>> create mode 100644 include/net/tc_wrapper.h
>>
>> diff --git a/include/net/tc_wrapper.h b/include/net/tc_wrapper.h
>> new file mode 100644
>> index 000000000000..3bdebbfdf9d2
>> --- /dev/null
>> +++ b/include/net/tc_wrapper.h
>> @@ -0,0 +1,226 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef __NET_TC_WRAPPER_H
>> +#define __NET_TC_WRAPPER_H
>> +
>> +#include <linux/indirect_call_wrapper.h>
>> +#include <net/pkt_cls.h>
>> +
>> +#if IS_ENABLED(CONFIG_NET_TC_INDIRECT_WRAPPER)
>> +
>> +#define TC_INDIRECT_SCOPE
>> +
>> +/* TC Actions */
>> +#ifdef CONFIG_NET_CLS_ACT
>> +
>> +#define TC_INDIRECT_ACTION_DECLARE(fname) \
>> + INDIRECT_CALLABLE_DECLARE(int fname(struct sk_buff *skb, \
>> + const struct tc_action *a, \
>> + struct tcf_result *res))
>> +
>> +TC_INDIRECT_ACTION_DECLARE(tcf_bpf_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_connmark_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_csum_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_ct_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_ctinfo_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_gact_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_gate_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_ife_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_ipt_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_mirred_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_mpls_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_nat_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_pedit_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_police_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_sample_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_simp_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_skbedit_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_skbmod_act);
>> +TC_INDIRECT_ACTION_DECLARE(tcf_vlan_act);
>> +TC_INDIRECT_ACTION_DECLARE(tunnel_key_act);
>> +
>> +static inline int tc_act(struct sk_buff *skb, const struct tc_action *a,
>> + struct tcf_result *res)
>> +{
>
> Perhaps you could add a static key to enable this retpoline avoidance only
> on cpus without hardware support. (IBRS enabled cpus would basically
> use a jump to
> directly go to the
>
> return a->ops->act(skb, a, res);
Something like this you have in mind? Not tested, just compiled:
diff --git a/include/net/tc_wrapper.h b/include/net/tc_wrapper.h
index 3bdebbfdf9d2..8a74bcf4a2e0 100644
--- a/include/net/tc_wrapper.h
+++ b/include/net/tc_wrapper.h
@@ -2,13 +2,19 @@
#ifndef __NET_TC_WRAPPER_H
#define __NET_TC_WRAPPER_H
-#include <linux/indirect_call_wrapper.h>
#include <net/pkt_cls.h>
-#if IS_ENABLED(CONFIG_NET_TC_INDIRECT_WRAPPER)
+#if IS_ENABLED(CONFIG_RETPOLINE)
+
+#include <asm/cpufeature.h>
+
+#include <linux/static_key.h>
+#include <linux/indirect_call_wrapper.h>
#define TC_INDIRECT_SCOPE
+static DEFINE_STATIC_KEY_FALSE(tc_skip_wrapper);
+
/* TC Actions */
#ifdef CONFIG_NET_CLS_ACT
@@ -41,6 +47,9 @@ TC_INDIRECT_ACTION_DECLARE(tunnel_key_act);
static inline int tc_act(struct sk_buff *skb, const struct tc_action *a,
struct tcf_result *res)
{
+ if (static_branch_unlikely(&tc_skip_wrapper))
+ goto skip;
+
#if IS_BUILTIN(CONFIG_NET_ACT_GACT)
if (a->ops->act == tcf_gact_act)
return tcf_gact_act(skb, a, res);
@@ -122,6 +131,7 @@ static inline int tc_act(struct sk_buff *skb, const
struct tc_action *a,
return tcf_sample_act(skb, a, res);
#endif
+skip:
return a->ops->act(skb, a, res);
}
@@ -151,6 +161,9 @@ TC_INDIRECT_FILTER_DECLARE(u32_classify);
static inline int tc_classify(struct sk_buff *skb, const struct
tcf_proto *tp,
struct tcf_result *res)
{
+ if (static_branch_unlikely(&tc_skip_wrapper))
+ goto skip;
+
#if IS_BUILTIN(CONFIG_NET_CLS_BPF)
if (tp->classify == cls_bpf_classify)
return cls_bpf_classify(skb, tp, res);
@@ -200,9 +213,16 @@ static inline int tc_classify(struct sk_buff *skb,
const struct tcf_proto *tp,
return tcindex_classify(skb, tp, res);
#endif
+skip:
return tp->classify(skb, tp, res);
}
+static inline void tc_wrapper_init(void)
+{
+ if (boot_cpu_has(X86_FEATURE_IBRS))
+ static_branch_enable(&tc_skip_wrapper);
+}
+
#endif /* CONFIG_NET_CLS */
#else
@@ -221,6 +241,10 @@ static inline int tc_classify(struct sk_buff *skb,
const struct tcf_proto *tp,
return tp->classify(skb, tp, res);
}
+static inline void tc_wrapper_init(void)
+{
+}
+
#endif
#endif /* __NET_TC_WRAPPER_H */
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 9bc055f8013e..1e8ab4749c6c 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -1021,19 +1021,6 @@ config NET_TC_SKB_EXT
Say N here if you won't be using tc<->ovs offload or tc
chains offload.
-config NET_TC_INDIRECT_WRAPPER
- bool "TC indirect call wrapper"
- depends on NET_SCHED
- depends on RETPOLINE
-
- help
- Say Y here to skip indirect calls in the TC datapath for known
- builtin classifiers/actions under CONFIG_RETPOLINE kernels.
-
- TC may run slower on CPUs with hardware based mitigations.
-
- If unsure, say N.
-
endif # NET_SCHED
config NET_SCH_FIFO
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 5b3c0ac495be..44d4b1e4e18e 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -2179,6 +2179,8 @@ static int __init tc_action_init(void)
rtnl_register(PF_UNSPEC, RTM_GETACTION, tc_ctl_action,
tc_dump_action,
0);
+ tc_wrapper_init();
+
return 0;
}
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 668130f08903..39b6f6331dee 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3765,6 +3765,8 @@ static int __init tc_filter_init(void)
rtnl_register(PF_UNSPEC, RTM_GETCHAIN, tc_ctl_chain,
tc_dump_chain, 0);
+ tc_wrapper_init();
+
return 0;
err_register_pernet_subsys:
Powered by blists - more mailing lists