From 4c096ea2d67cad959b5de9f98015e9291c7b7b56 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:20 +0300 Subject: [PATCH 01/14] net/sched: matchall: Take verbose flag into account when logging error messages The verbose flag was added in commit 81c7288b170a ("sched: cls: enable verbose logging") to avoid suppressing logging of error messages that occur "when the rule is not to be exclusively executed by the hardware". However, such error messages are currently suppressed when setup of flow action fails. Take the verbose flag into account to avoid suppressing error messages. This is done by using the extack pointer initialized by tc_cls_common_offload_init(), which performs the necessary checks. Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/cls_matchall.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c index ca5670fd5228ea..37283b30692451 100644 --- a/net/sched/cls_matchall.c +++ b/net/sched/cls_matchall.c @@ -101,12 +101,10 @@ static int mall_replace_hw_filter(struct tcf_proto *tp, if (err) { kfree(cls_mall.rule); mall_destroy_hw_filter(tp, head, cookie, NULL); - if (skip_sw) - NL_SET_ERR_MSG_MOD(extack, "Failed to setup flow action"); - else - err = 0; + NL_SET_ERR_MSG_MOD(cls_mall.common.extack, + "Failed to setup flow action"); - return err; + return skip_sw ? err : 0; } err = tc_setup_cb_add(block, tp, TC_SETUP_CLSMATCHALL, &cls_mall, @@ -305,11 +303,10 @@ static int mall_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb, err = tc_setup_offload_action(&cls_mall.rule->action, &head->exts); if (err) { kfree(cls_mall.rule); - if (add && tc_skip_sw(head->flags)) { - NL_SET_ERR_MSG_MOD(extack, "Failed to setup flow action"); - return err; - } - return 0; + NL_SET_ERR_MSG_MOD(cls_mall.common.extack, + "Failed to setup flow action"); + + return add && tc_skip_sw(head->flags) ? err : 0; } err = tc_setup_cb_reoffload(block, tp, add, cb, TC_SETUP_CLSMATCHALL, From 11c95317bc1a70b305b978ac4bec39e224142094 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:21 +0300 Subject: [PATCH 02/14] net/sched: flower: Take verbose flag into account when logging error messages The verbose flag was added in commit 81c7288b170a ("sched: cls: enable verbose logging") to avoid suppressing logging of error messages that occur "when the rule is not to be exclusively executed by the hardware". However, such error messages are currently suppressed when setup of flow action fails. Take the verbose flag into account to avoid suppressing error messages. This is done by using the extack pointer initialized by tc_cls_common_offload_init(), which performs the necessary checks. Before: # tc filter add dev dummy0 ingress pref 1 proto ip flower dst_ip 198.51.100.1 action police rate 100Mbit burst 10000 # tc filter add dev dummy0 ingress pref 2 proto ip flower verbose dst_ip 198.51.100.1 action police rate 100Mbit burst 10000 After: # tc filter add dev dummy0 ingress pref 1 proto ip flower dst_ip 198.51.100.1 action police rate 100Mbit burst 10000 # tc filter add dev dummy0 ingress pref 2 proto ip flower verbose dst_ip 198.51.100.1 action police rate 100Mbit burst 10000 Warning: cls_flower: Failed to setup flow action. Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/cls_flower.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index c80fc49c0da1c7..70e95ce28ffdd1 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -467,11 +467,10 @@ static int fl_hw_replace_filter(struct tcf_proto *tp, err = tc_setup_offload_action(&cls_flower.rule->action, &f->exts); if (err) { kfree(cls_flower.rule); - if (skip_sw) { - NL_SET_ERR_MSG_MOD(extack, "Failed to setup flow action"); - return err; - } - return 0; + NL_SET_ERR_MSG_MOD(cls_flower.common.extack, + "Failed to setup flow action"); + + return skip_sw ? err : 0; } err = tc_setup_cb_add(block, tp, TC_SETUP_CLSFLOWER, &cls_flower, @@ -2357,8 +2356,9 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb, err = tc_setup_offload_action(&cls_flower.rule->action, &f->exts); if (err) { kfree(cls_flower.rule); + NL_SET_ERR_MSG_MOD(cls_flower.common.extack, + "Failed to setup flow action"); if (tc_skip_sw(f->flags)) { - NL_SET_ERR_MSG_MOD(extack, "Failed to setup flow action"); __fl_put(f); return err; } From c2ccf84ecb715bb81dc7f51e69d680a95bf055ae Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:22 +0300 Subject: [PATCH 03/14] net/sched: act_api: Add extack to offload_act_setup() callback The callback is used by various actions to populate the flow action structure prior to offload. Pass extack to this callback so that the various actions will be able to report accurate error messages to user space. Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- include/net/act_api.h | 3 ++- include/net/pkt_cls.h | 6 ++++-- net/sched/act_api.c | 4 ++-- net/sched/act_csum.c | 3 ++- net/sched/act_ct.c | 3 ++- net/sched/act_gact.c | 3 ++- net/sched/act_gate.c | 3 ++- net/sched/act_mirred.c | 3 ++- net/sched/act_mpls.c | 3 ++- net/sched/act_pedit.c | 3 ++- net/sched/act_police.c | 3 ++- net/sched/act_sample.c | 3 ++- net/sched/act_skbedit.c | 3 ++- net/sched/act_tunnel_key.c | 3 ++- net/sched/act_vlan.c | 3 ++- net/sched/cls_api.c | 16 ++++++++++------ net/sched/cls_flower.c | 6 ++++-- net/sched/cls_matchall.c | 6 ++++-- 18 files changed, 50 insertions(+), 27 deletions(-) diff --git a/include/net/act_api.h b/include/net/act_api.h index 3049cb69c0252b..9cf6870b526eb2 100644 --- a/include/net/act_api.h +++ b/include/net/act_api.h @@ -134,7 +134,8 @@ struct tc_action_ops { (*get_psample_group)(const struct tc_action *a, tc_action_priv_destructor *destructor); int (*offload_act_setup)(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind); + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack); }; struct tc_action_net { diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index a3b57a93228a7f..8cf001aed858dc 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -547,10 +547,12 @@ tcf_match_indev(struct sk_buff *skb, int ifindex) } int tc_setup_offload_action(struct flow_action *flow_action, - const struct tcf_exts *exts); + const struct tcf_exts *exts, + struct netlink_ext_ack *extack); void tc_cleanup_offload_action(struct flow_action *flow_action); int tc_setup_action(struct flow_action *flow_action, - struct tc_action *actions[]); + struct tc_action *actions[], + struct netlink_ext_ack *extack); int tc_setup_cb_call(struct tcf_block *block, enum tc_setup_type type, void *type_data, bool err_stop, bool rtnl_held); diff --git a/net/sched/act_api.c b/net/sched/act_api.c index 4f51094da9dab7..da9733da9868ff 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -195,7 +195,7 @@ static int offload_action_init(struct flow_offload_action *fl_action, if (act->ops->offload_act_setup) { spin_lock_bh(&act->tcfa_lock); err = act->ops->offload_act_setup(act, fl_action, NULL, - false); + false, extack); spin_unlock_bh(&act->tcfa_lock); return err; } @@ -271,7 +271,7 @@ static int tcf_action_offload_add_ex(struct tc_action *action, if (err) goto fl_err; - err = tc_setup_action(&fl_action->action, actions); + err = tc_setup_action(&fl_action->action, actions, extack); if (err) { NL_SET_ERR_MSG_MOD(extack, "Failed to setup tc actions for offload"); diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c index e0f515b774cad2..22847ee009efaf 100644 --- a/net/sched/act_csum.c +++ b/net/sched/act_csum.c @@ -696,7 +696,8 @@ static size_t tcf_csum_get_fill_size(const struct tc_action *act) } static int tcf_csum_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { if (bind) { struct flow_action_entry *entry = entry_data; diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c index b1f502fce59566..8af9d6e5ba61d5 100644 --- a/net/sched/act_ct.c +++ b/net/sched/act_ct.c @@ -1584,7 +1584,8 @@ static void tcf_stats_update(struct tc_action *a, u64 bytes, u64 packets, } static int tcf_ct_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { if (bind) { struct flow_action_entry *entry = entry_data; diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c index bde6a6c01e64cb..db84a0473cc131 100644 --- a/net/sched/act_gact.c +++ b/net/sched/act_gact.c @@ -253,7 +253,8 @@ static size_t tcf_gact_get_fill_size(const struct tc_action *act) } static int tcf_gact_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { if (bind) { struct flow_action_entry *entry = entry_data; diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c index d56e73843a4b7f..fd515527473324 100644 --- a/net/sched/act_gate.c +++ b/net/sched/act_gate.c @@ -619,7 +619,8 @@ static int tcf_gate_get_entries(struct flow_action_entry *entry, } static int tcf_gate_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { int err; diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 39acd1d186098b..70a6a4447e6bd6 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -460,7 +460,8 @@ static void tcf_offload_mirred_get_dev(struct flow_action_entry *entry, } static int tcf_mirred_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { if (bind) { struct flow_action_entry *entry = entry_data; diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c index b9ff3459fdab97..23fcfa5605dfa4 100644 --- a/net/sched/act_mpls.c +++ b/net/sched/act_mpls.c @@ -385,7 +385,8 @@ static int tcf_mpls_search(struct net *net, struct tc_action **a, u32 index) } static int tcf_mpls_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { if (bind) { struct flow_action_entry *entry = entry_data; diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index 31fcd279c17767..dc12d502c4fec2 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -488,7 +488,8 @@ static int tcf_pedit_search(struct net *net, struct tc_action **a, u32 index) } static int tcf_pedit_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { if (bind) { struct flow_action_entry *entry = entry_data; diff --git a/net/sched/act_police.c b/net/sched/act_police.c index f4d91770526390..77c17e9b46d112 100644 --- a/net/sched/act_police.c +++ b/net/sched/act_police.c @@ -442,7 +442,8 @@ static int tcf_police_act_to_flow_act(int tc_act, u32 *extval) } static int tcf_police_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { if (bind) { struct flow_action_entry *entry = entry_data; diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c index 9a22cdda6bbdcd..2f7f5e44d28c98 100644 --- a/net/sched/act_sample.c +++ b/net/sched/act_sample.c @@ -291,7 +291,8 @@ static void tcf_offload_sample_get_group(struct flow_action_entry *entry, } static int tcf_sample_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { if (bind) { struct flow_action_entry *entry = entry_data; diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c index ceba11b198bbab..8cd8e506c9c9b8 100644 --- a/net/sched/act_skbedit.c +++ b/net/sched/act_skbedit.c @@ -328,7 +328,8 @@ static size_t tcf_skbedit_get_fill_size(const struct tc_action *act) } static int tcf_skbedit_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { if (bind) { struct flow_action_entry *entry = entry_data; diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c index 23aba03d26a8d1..3c6f40478c8135 100644 --- a/net/sched/act_tunnel_key.c +++ b/net/sched/act_tunnel_key.c @@ -808,7 +808,8 @@ static int tcf_tunnel_encap_get_tunnel(struct flow_action_entry *entry, static int tcf_tunnel_key_offload_act_setup(struct tc_action *act, void *entry_data, u32 *index_inc, - bool bind) + bool bind, + struct netlink_ext_ack *extack) { int err; diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c index 883454c4f92194..8c89bce99cbd17 100644 --- a/net/sched/act_vlan.c +++ b/net/sched/act_vlan.c @@ -369,7 +369,8 @@ static size_t tcf_vlan_get_fill_size(const struct tc_action *act) } static int tcf_vlan_offload_act_setup(struct tc_action *act, void *entry_data, - u32 *index_inc, bool bind) + u32 *index_inc, bool bind, + struct netlink_ext_ack *extack) { if (bind) { struct flow_action_entry *entry = entry_data; diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 2957f8f5cea759..dd711ae048ff45 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -3513,11 +3513,13 @@ EXPORT_SYMBOL(tc_cleanup_offload_action); static int tc_setup_offload_act(struct tc_action *act, struct flow_action_entry *entry, - u32 *index_inc) + u32 *index_inc, + struct netlink_ext_ack *extack) { #ifdef CONFIG_NET_CLS_ACT if (act->ops->offload_act_setup) - return act->ops->offload_act_setup(act, entry, index_inc, true); + return act->ops->offload_act_setup(act, entry, index_inc, true, + extack); else return -EOPNOTSUPP; #else @@ -3526,7 +3528,8 @@ static int tc_setup_offload_act(struct tc_action *act, } int tc_setup_action(struct flow_action *flow_action, - struct tc_action *actions[]) + struct tc_action *actions[], + struct netlink_ext_ack *extack) { int i, j, index, err = 0; struct tc_action *act; @@ -3551,7 +3554,7 @@ int tc_setup_action(struct flow_action *flow_action, entry->hw_stats = tc_act_hw_stats(act->hw_stats); entry->hw_index = act->tcfa_index; index = 0; - err = tc_setup_offload_act(act, entry, &index); + err = tc_setup_offload_act(act, entry, &index, extack); if (!err) j += index; else @@ -3570,13 +3573,14 @@ int tc_setup_action(struct flow_action *flow_action, } int tc_setup_offload_action(struct flow_action *flow_action, - const struct tcf_exts *exts) + const struct tcf_exts *exts, + struct netlink_ext_ack *extack) { #ifdef CONFIG_NET_CLS_ACT if (!exts) return 0; - return tc_setup_action(flow_action, exts->actions); + return tc_setup_action(flow_action, exts->actions, extack); #else return 0; #endif diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index 70e95ce28ffdd1..acf827b0e30a6e 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -464,7 +464,8 @@ static int fl_hw_replace_filter(struct tcf_proto *tp, cls_flower.rule->match.key = &f->mkey; cls_flower.classid = f->res.classid; - err = tc_setup_offload_action(&cls_flower.rule->action, &f->exts); + err = tc_setup_offload_action(&cls_flower.rule->action, &f->exts, + cls_flower.common.extack); if (err) { kfree(cls_flower.rule); NL_SET_ERR_MSG_MOD(cls_flower.common.extack, @@ -2353,7 +2354,8 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb, cls_flower.rule->match.mask = &f->mask->key; cls_flower.rule->match.key = &f->mkey; - err = tc_setup_offload_action(&cls_flower.rule->action, &f->exts); + err = tc_setup_offload_action(&cls_flower.rule->action, &f->exts, + cls_flower.common.extack); if (err) { kfree(cls_flower.rule); NL_SET_ERR_MSG_MOD(cls_flower.common.extack, diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c index 37283b30692451..7553443e1ae7e0 100644 --- a/net/sched/cls_matchall.c +++ b/net/sched/cls_matchall.c @@ -97,7 +97,8 @@ static int mall_replace_hw_filter(struct tcf_proto *tp, cls_mall.command = TC_CLSMATCHALL_REPLACE; cls_mall.cookie = cookie; - err = tc_setup_offload_action(&cls_mall.rule->action, &head->exts); + err = tc_setup_offload_action(&cls_mall.rule->action, &head->exts, + cls_mall.common.extack); if (err) { kfree(cls_mall.rule); mall_destroy_hw_filter(tp, head, cookie, NULL); @@ -300,7 +301,8 @@ static int mall_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb, TC_CLSMATCHALL_REPLACE : TC_CLSMATCHALL_DESTROY; cls_mall.cookie = (unsigned long)head; - err = tc_setup_offload_action(&cls_mall.rule->action, &head->exts); + err = tc_setup_offload_action(&cls_mall.rule->action, &head->exts, + cls_mall.common.extack); if (err) { kfree(cls_mall.rule); NL_SET_ERR_MSG_MOD(cls_mall.common.extack, From 69642c2ab2f553fd8c4c121fcdf3b3054c727e86 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:23 +0300 Subject: [PATCH 04/14] net/sched: act_gact: Add extack messages for offload failure For better error reporting to user space, add extack messages when gact action offload fails. Example: # echo 1 > /sys/kernel/tracing/events/netlink/netlink_extack/enable # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action continue Error: cls_matchall: Failed to setup flow action. We have an error talking to the kernel # cat /sys/kernel/tracing/trace_pipe tc-181 [002] b..1. 105.493450: netlink_extack: msg=act_gact: Offload of "continue" action is not supported tc-181 [002] ..... 105.493466: netlink_extack: msg=cls_matchall: Failed to setup flow action # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action reclassify Error: cls_matchall: Failed to setup flow action. We have an error talking to the kernel # cat /sys/kernel/tracing/trace_pipe tc-183 [002] b..1. 124.126477: netlink_extack: msg=act_gact: Offload of "reclassify" action is not supported tc-183 [002] ..... 124.126489: netlink_extack: msg=cls_matchall: Failed to setup flow action # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action pipe action drop Error: cls_matchall: Failed to setup flow action. We have an error talking to the kernel # cat /sys/kernel/tracing/trace_pipe tc-185 [002] b..1. 137.097791: netlink_extack: msg=act_gact: Offload of "pipe" action is not supported tc-185 [002] ..... 137.097804: netlink_extack: msg=cls_matchall: Failed to setup flow action Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- include/net/tc_act/tc_gact.h | 15 +++++++++++++++ net/sched/act_gact.c | 10 ++++++++++ 2 files changed, 25 insertions(+) diff --git a/include/net/tc_act/tc_gact.h b/include/net/tc_act/tc_gact.h index eb8f01c819e636..832efd40e0233c 100644 --- a/include/net/tc_act/tc_gact.h +++ b/include/net/tc_act/tc_gact.h @@ -59,4 +59,19 @@ static inline u32 tcf_gact_goto_chain_index(const struct tc_action *a) return READ_ONCE(a->tcfa_action) & TC_ACT_EXT_VAL_MASK; } +static inline bool is_tcf_gact_continue(const struct tc_action *a) +{ + return __is_tcf_gact_act(a, TC_ACT_UNSPEC, false); +} + +static inline bool is_tcf_gact_reclassify(const struct tc_action *a) +{ + return __is_tcf_gact_act(a, TC_ACT_RECLASSIFY, false); +} + +static inline bool is_tcf_gact_pipe(const struct tc_action *a) +{ + return __is_tcf_gact_act(a, TC_ACT_PIPE, false); +} + #endif /* __NET_TC_GACT_H */ diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c index db84a0473cc131..ac29d10652321d 100644 --- a/net/sched/act_gact.c +++ b/net/sched/act_gact.c @@ -268,7 +268,17 @@ static int tcf_gact_offload_act_setup(struct tc_action *act, void *entry_data, } else if (is_tcf_gact_goto_chain(act)) { entry->id = FLOW_ACTION_GOTO; entry->chain_index = tcf_gact_goto_chain_index(act); + } else if (is_tcf_gact_continue(act)) { + NL_SET_ERR_MSG_MOD(extack, "Offload of \"continue\" action is not supported"); + return -EOPNOTSUPP; + } else if (is_tcf_gact_reclassify(act)) { + NL_SET_ERR_MSG_MOD(extack, "Offload of \"reclassify\" action is not supported"); + return -EOPNOTSUPP; + } else if (is_tcf_gact_pipe(act)) { + NL_SET_ERR_MSG_MOD(extack, "Offload of \"pipe\" action is not supported"); + return -EOPNOTSUPP; } else { + NL_SET_ERR_MSG_MOD(extack, "Unsupported generic action offload"); return -EOPNOTSUPP; } *index_inc = 1; From 4dcaa50d02927f045c8653ba992aadc7c94ee71a Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:24 +0300 Subject: [PATCH 05/14] net/sched: act_mirred: Add extack message for offload failure For better error reporting to user space, add an extack message when mirred action offload fails. Currently, the failure cannot be triggered, but add a message in case the action is extended in the future to support more than ingress/egress mirror/redirect. Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/act_mirred.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 70a6a4447e6bd6..ebb92fb072ab55 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -479,6 +479,7 @@ static int tcf_mirred_offload_act_setup(struct tc_action *act, void *entry_data, entry->id = FLOW_ACTION_MIRRED_INGRESS; tcf_offload_mirred_get_dev(entry, act); } else { + NL_SET_ERR_MSG_MOD(extack, "Unsupported mirred offload"); return -EOPNOTSUPP; } *index_inc = 1; From bca3821d19d9aa05cd8164e4f716150d6ef7e5a5 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:25 +0300 Subject: [PATCH 06/14] net/sched: act_mpls: Add extack messages for offload failure For better error reporting to user space, add extack messages when mpls action offload fails. Example: # echo 1 > /sys/kernel/tracing/events/netlink/netlink_extack/enable # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action mpls dec_ttl Error: cls_matchall: Failed to setup flow action. We have an error talking to the kernel # cat /sys/kernel/tracing/trace_pipe tc-182 [000] b..1. 18.693915: netlink_extack: msg=act_mpls: Offload not supported when "dec_ttl" option is used tc-182 [000] ..... 18.693921: netlink_extack: msg=cls_matchall: Failed to setup flow action Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/act_mpls.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c index 23fcfa5605dfa4..adabeccb63e1d2 100644 --- a/net/sched/act_mpls.c +++ b/net/sched/act_mpls.c @@ -411,7 +411,14 @@ static int tcf_mpls_offload_act_setup(struct tc_action *act, void *entry_data, entry->mpls_mangle.bos = tcf_mpls_bos(act); entry->mpls_mangle.ttl = tcf_mpls_ttl(act); break; + case TCA_MPLS_ACT_DEC_TTL: + NL_SET_ERR_MSG_MOD(extack, "Offload not supported when \"dec_ttl\" option is used"); + return -EOPNOTSUPP; + case TCA_MPLS_ACT_MAC_PUSH: + NL_SET_ERR_MSG_MOD(extack, "Offload not supported when \"mac_push\" option is used"); + return -EOPNOTSUPP; default: + NL_SET_ERR_MSG_MOD(extack, "Unsupported MPLS mode offload"); return -EOPNOTSUPP; } *index_inc = 1; From bf3b99e4f9ce52e8da6158894be5679d1217082b Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:26 +0300 Subject: [PATCH 07/14] net/sched: act_pedit: Add extack message for offload failure For better error reporting to user space, add an extack message when pedit action offload fails. Currently, the failure cannot be triggered, but add a message in case the action is extended in the future to support more than set/add commands. Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/act_pedit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index dc12d502c4fec2..e01ef7f109f480 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -504,6 +504,7 @@ static int tcf_pedit_offload_act_setup(struct tc_action *act, void *entry_data, entry->id = FLOW_ACTION_ADD; break; default: + NL_SET_ERR_MSG_MOD(extack, "Unsupported pedit command offload"); return -EOPNOTSUPP; } entry->mangle.htype = tcf_pedit_htype(act, k); From b50e462bc22df4488ec04d85606646e3db5952b8 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:27 +0300 Subject: [PATCH 08/14] net/sched: act_police: Add extack messages for offload failure For better error reporting to user space, add extack messages when police action offload fails. Example: # echo 1 > /sys/kernel/tracing/events/netlink/netlink_extack/enable # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action police rate 100Mbit burst 10000 Error: cls_matchall: Failed to setup flow action. We have an error talking to the kernel # cat /sys/kernel/tracing/trace_pipe tc-182 [000] b..1. 21.592969: netlink_extack: msg=act_police: Offload not supported when conform/exceed action is "reclassify" tc-182 [000] ..... 21.592982: netlink_extack: msg=cls_matchall: Failed to setup flow action # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action police rate 100Mbit burst 10000 conform-exceed drop/continue Error: cls_matchall: Failed to setup flow action. We have an error talking to the kernel # cat /sys/kernel/tracing/trace_pipe tc-184 [000] b..1. 38.882579: netlink_extack: msg=act_police: Offload not supported when conform/exceed action is "continue" tc-184 [000] ..... 38.882593: netlink_extack: msg=cls_matchall: Failed to setup flow action Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/act_police.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/net/sched/act_police.c b/net/sched/act_police.c index 77c17e9b46d112..79c8901f66ab1c 100644 --- a/net/sched/act_police.c +++ b/net/sched/act_police.c @@ -419,7 +419,8 @@ static int tcf_police_search(struct net *net, struct tc_action **a, u32 index) return tcf_idr_search(tn, a, index); } -static int tcf_police_act_to_flow_act(int tc_act, u32 *extval) +static int tcf_police_act_to_flow_act(int tc_act, u32 *extval, + struct netlink_ext_ack *extack) { int act_id = -EOPNOTSUPP; @@ -430,12 +431,20 @@ static int tcf_police_act_to_flow_act(int tc_act, u32 *extval) act_id = FLOW_ACTION_DROP; else if (tc_act == TC_ACT_PIPE) act_id = FLOW_ACTION_PIPE; + else if (tc_act == TC_ACT_RECLASSIFY) + NL_SET_ERR_MSG_MOD(extack, "Offload not supported when conform/exceed action is \"reclassify\""); + else + NL_SET_ERR_MSG_MOD(extack, "Unsupported conform/exceed action offload"); } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_GOTO_CHAIN)) { act_id = FLOW_ACTION_GOTO; *extval = tc_act & TC_ACT_EXT_VAL_MASK; } else if (TC_ACT_EXT_CMP(tc_act, TC_ACT_JUMP)) { act_id = FLOW_ACTION_JUMP; *extval = tc_act & TC_ACT_EXT_VAL_MASK; + } else if (tc_act == TC_ACT_UNSPEC) { + NL_SET_ERR_MSG_MOD(extack, "Offload not supported when conform/exceed action is \"continue\""); + } else { + NL_SET_ERR_MSG_MOD(extack, "Unsupported conform/exceed action offload"); } return act_id; @@ -467,14 +476,16 @@ static int tcf_police_offload_act_setup(struct tc_action *act, void *entry_data, entry->police.mtu = tcf_police_tcfp_mtu(act); act_id = tcf_police_act_to_flow_act(police->tcf_action, - &entry->police.exceed.extval); + &entry->police.exceed.extval, + extack); if (act_id < 0) return act_id; entry->police.exceed.act_id = act_id; act_id = tcf_police_act_to_flow_act(p->tcfp_result, - &entry->police.notexceed.extval); + &entry->police.notexceed.extval, + extack); if (act_id < 0) return act_id; From a9c64939b669b3c83cc54738d1986430e6beaff2 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:28 +0300 Subject: [PATCH 09/14] net/sched: act_skbedit: Add extack messages for offload failure For better error reporting to user space, add extack messages when skbedit action offload fails. Example: # echo 1 > /sys/kernel/tracing/events/netlink/netlink_extack/enable # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action skbedit queue_mapping 1234 Error: cls_matchall: Failed to setup flow action. We have an error talking to the kernel # cat /sys/kernel/tracing/trace_pipe tc-185 [002] b..1. 31.802414: netlink_extack: msg=act_skbedit: Offload not supported when "queue_mapping" option is used tc-185 [002] ..... 31.802418: netlink_extack: msg=cls_matchall: Failed to setup flow action # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action skbedit inheritdsfield Error: cls_matchall: Failed to setup flow action. We have an error talking to the kernel # cat /sys/kernel/tracing/trace_pipe tc-187 [002] b..1. 45.985145: netlink_extack: msg=act_skbedit: Offload not supported when "inheritdsfield" option is used tc-187 [002] ..... 45.985160: netlink_extack: msg=cls_matchall: Failed to setup flow action Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- include/net/tc_act/tc_skbedit.h | 12 ++++++++++++ net/sched/act_skbedit.c | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/include/net/tc_act/tc_skbedit.h b/include/net/tc_act/tc_skbedit.h index 00bfee70609eae..cab8229b9bed38 100644 --- a/include/net/tc_act/tc_skbedit.h +++ b/include/net/tc_act/tc_skbedit.h @@ -94,4 +94,16 @@ static inline u32 tcf_skbedit_priority(const struct tc_action *a) return priority; } +/* Return true iff action is queue_mapping */ +static inline bool is_tcf_skbedit_queue_mapping(const struct tc_action *a) +{ + return is_tcf_skbedit_with_flag(a, SKBEDIT_F_QUEUE_MAPPING); +} + +/* Return true iff action is inheritdsfield */ +static inline bool is_tcf_skbedit_inheritdsfield(const struct tc_action *a) +{ + return is_tcf_skbedit_with_flag(a, SKBEDIT_F_INHERITDSFIELD); +} + #endif /* __NET_TC_SKBEDIT_H */ diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c index 8cd8e506c9c9b8..92d0dc754207fd 100644 --- a/net/sched/act_skbedit.c +++ b/net/sched/act_skbedit.c @@ -343,7 +343,14 @@ static int tcf_skbedit_offload_act_setup(struct tc_action *act, void *entry_data } else if (is_tcf_skbedit_priority(act)) { entry->id = FLOW_ACTION_PRIORITY; entry->priority = tcf_skbedit_priority(act); + } else if (is_tcf_skbedit_queue_mapping(act)) { + NL_SET_ERR_MSG_MOD(extack, "Offload not supported when \"queue_mapping\" option is used"); + return -EOPNOTSUPP; + } else if (is_tcf_skbedit_inheritdsfield(act)) { + NL_SET_ERR_MSG_MOD(extack, "Offload not supported when \"inheritdsfield\" option is used"); + return -EOPNOTSUPP; } else { + NL_SET_ERR_MSG_MOD(extack, "Unsupported skbedit option offload"); return -EOPNOTSUPP; } *index_inc = 1; From ee367d44b93664feaeea739c1324e914817be6f5 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:29 +0300 Subject: [PATCH 10/14] net/sched: act_tunnel_key: Add extack message for offload failure For better error reporting to user space, add an extack message when tunnel_key action offload fails. Currently, the failure cannot be triggered, but add a message in case the action is extended in the future to support more than set/release modes. Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/act_tunnel_key.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c index 3c6f40478c8135..856dc23cef8c85 100644 --- a/net/sched/act_tunnel_key.c +++ b/net/sched/act_tunnel_key.c @@ -824,6 +824,7 @@ static int tcf_tunnel_key_offload_act_setup(struct tc_action *act, } else if (is_tcf_tunnel_release(act)) { entry->id = FLOW_ACTION_TUNNEL_DECAP; } else { + NL_SET_ERR_MSG_MOD(extack, "Unsupported tunnel key mode offload"); return -EOPNOTSUPP; } *index_inc = 1; From f8fab3169464623b88d0f423f8de2b28809c2fcf Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:30 +0300 Subject: [PATCH 11/14] net/sched: act_vlan: Add extack message for offload failure For better error reporting to user space, add an extack message when vlan action offload fails. Currently, the failure cannot be triggered, but add a message in case the action is extended in the future to support more than the current set of modes. Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/act_vlan.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c index 8c89bce99cbd17..68b5e772386a21 100644 --- a/net/sched/act_vlan.c +++ b/net/sched/act_vlan.c @@ -399,6 +399,7 @@ static int tcf_vlan_offload_act_setup(struct tc_action *act, void *entry_data, tcf_vlan_push_eth(entry->vlan_push_eth.src, entry->vlan_push_eth.dst, act); break; default: + NL_SET_ERR_MSG_MOD(extack, "Unsupported vlan action mode offload"); return -EOPNOTSUPP; } *index_inc = 1; From c440615ffbcb9c06975103e5abbcb094589329d1 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:31 +0300 Subject: [PATCH 12/14] net/sched: cls_api: Add extack message for unsupported action offload For better error reporting to user space, add an extack message when the requested action does not support offload. Example: # echo 1 > /sys/kernel/tracing/events/netlink/netlink_extack/enable # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action nat ingress 192.0.2.1 198.51.100.1 Error: cls_matchall: Failed to setup flow action. We have an error talking to the kernel # cat /sys/kernel/tracing/trace_pipe tc-181 [000] b..1. 88.406093: netlink_extack: msg=Action does not support offload tc-181 [000] ..... 88.406108: netlink_extack: msg=cls_matchall: Failed to setup flow action Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/cls_api.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index dd711ae048ff45..d8a22bedd02627 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -3517,11 +3517,13 @@ static int tc_setup_offload_act(struct tc_action *act, struct netlink_ext_ack *extack) { #ifdef CONFIG_NET_CLS_ACT - if (act->ops->offload_act_setup) + if (act->ops->offload_act_setup) { return act->ops->offload_act_setup(act, entry, index_inc, true, extack); - else + } else { + NL_SET_ERR_MSG(extack, "Action does not support offload"); return -EOPNOTSUPP; + } #else return 0; #endif From 0cba5c34b8f4b4b81e5102992c8a9e189ec27768 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:32 +0300 Subject: [PATCH 13/14] net/sched: matchall: Avoid overwriting error messages The various error paths of tc_setup_offload_action() now report specific error messages. Remove the generic messages to avoid overwriting the more specific ones. Before: # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action police rate 100Mbit burst 10000 Error: cls_matchall: Failed to setup flow action. We have an error talking to the kernel After: # tc filter add dev dummy0 ingress pref 1 proto all matchall skip_sw action police rate 100Mbit burst 10000 Error: act_police: Offload not supported when conform/exceed action is "reclassify". We have an error talking to the kernel Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/cls_matchall.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c index 7553443e1ae7e0..06cf22adbab732 100644 --- a/net/sched/cls_matchall.c +++ b/net/sched/cls_matchall.c @@ -102,8 +102,6 @@ static int mall_replace_hw_filter(struct tcf_proto *tp, if (err) { kfree(cls_mall.rule); mall_destroy_hw_filter(tp, head, cookie, NULL); - NL_SET_ERR_MSG_MOD(cls_mall.common.extack, - "Failed to setup flow action"); return skip_sw ? err : 0; } @@ -305,8 +303,6 @@ static int mall_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb, cls_mall.common.extack); if (err) { kfree(cls_mall.rule); - NL_SET_ERR_MSG_MOD(cls_mall.common.extack, - "Failed to setup flow action"); return add && tc_skip_sw(head->flags) ? err : 0; } From fd23e0e250c6a7a7fd8a2ec9ab4253299471c163 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 7 Apr 2022 10:35:33 +0300 Subject: [PATCH 14/14] net/sched: flower: Avoid overwriting error messages The various error paths of tc_setup_offload_action() now report specific error messages. Remove the generic messages to avoid overwriting the more specific ones. Before: # tc filter add dev dummy0 ingress pref 1 proto ip flower skip_sw dst_ip 198.51.100.1 action police rate 100Mbit burst 10000 Error: cls_flower: Failed to setup flow action. We have an error talking to the kernel After: # tc filter add dev dummy0 ingress pref 1 proto ip flower skip_sw dst_ip 198.51.100.1 action police rate 100Mbit burst 10000 Error: act_police: Offload not supported when conform/exceed action is "reclassify". We have an error talking to the kernel Signed-off-by: Ido Schimmel Reviewed-by: Petr Machata Signed-off-by: David S. Miller --- net/sched/cls_flower.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index acf827b0e30a6e..87e030dfc3283a 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -468,8 +468,6 @@ static int fl_hw_replace_filter(struct tcf_proto *tp, cls_flower.common.extack); if (err) { kfree(cls_flower.rule); - NL_SET_ERR_MSG_MOD(cls_flower.common.extack, - "Failed to setup flow action"); return skip_sw ? err : 0; } @@ -2358,8 +2356,6 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, flow_setup_cb_t *cb, cls_flower.common.extack); if (err) { kfree(cls_flower.rule); - NL_SET_ERR_MSG_MOD(cls_flower.common.extack, - "Failed to setup flow action"); if (tc_skip_sw(f->flags)) { __fl_put(f); return err;