[PATCH net-next 00/14] net/sched: Better error reporting for offload failures

[PATCH net-next 00/14] net/sched: Better error reporting for offload failures

[Ido Schimmel]

This patchset improves error reporting to user space when offload fails
during the flow action setup phase. That is, when failures occur in the
actions themselves, even before calling device drivers. Requested /
reported in [1].

This is done by passing extack to the offload_act_setup() callback and
making use of it in the various actions.

Patches #1-#2 change matchall and flower to log error messages to user
space in accordance with the verbose flag.

Patch #3 passes extack to the offload_act_setup() callback from the
various call sites, including matchall and flower.

Patches #4-#11 make use of extack in the various actions to report
offload failures.

Patch #12 adds an error message when the action does not support offload
at all.

Patches #13-#14 change matchall and flower to stop overwriting more
specific error messages.

[1] https://lore.kernel.org/netdev/20220317185249.5mff5u2x624pjewv@skbuf/

Ido Schimmel (14):
  net/sched: matchall: Take verbose flag into account when logging error
    messages
  net/sched: flower: Take verbose flag into account when logging error
    messages
  net/sched: act_api: Add extack to offload_act_setup() callback
  net/sched: act_gact: Add extack messages for offload failure
  net/sched: act_mirred: Add extack message for offload failure
  net/sched: act_mpls: Add extack messages for offload failure
  net/sched: act_pedit: Add extack message for offload failure
  net/sched: act_police: Add extack messages for offload failure
  net/sched: act_skbedit: Add extack messages for offload failure
  net/sched: act_tunnel_key: Add extack message for offload failure
  net/sched: act_vlan: Add extack message for offload failure
  net/sched: cls_api: Add extack message for unsupported action offload
  net/sched: matchall: Avoid overwriting error messages
  net/sched: flower: Avoid overwriting error messages

 include/net/act_api.h           |  3 ++-
 include/net/pkt_cls.h           |  6 ++++--
 include/net/tc_act/tc_gact.h    | 15 +++++++++++++++
 include/net/tc_act/tc_skbedit.h | 12 ++++++++++++
 net/sched/act_api.c             |  4 ++--
 net/sched/act_csum.c            |  3 ++-
 net/sched/act_ct.c              |  3 ++-
 net/sched/act_gact.c            | 13 ++++++++++++-
 net/sched/act_gate.c            |  3 ++-
 net/sched/act_mirred.c          |  4 +++-
 net/sched/act_mpls.c            | 10 +++++++++-
 net/sched/act_pedit.c           |  4 +++-
 net/sched/act_police.c          | 20 ++++++++++++++++----
 net/sched/act_sample.c          |  3 ++-
 net/sched/act_skbedit.c         | 10 +++++++++-
 net/sched/act_tunnel_key.c      |  4 +++-
 net/sched/act_vlan.c            |  4 +++-
 net/sched/cls_api.c             | 22 ++++++++++++++--------
 net/sched/cls_flower.c          | 14 ++++++--------
 net/sched/cls_matchall.c        | 19 +++++++------------
 20 files changed, 128 insertions(+), 48 deletions(-)

-- 
2.33.1

[PATCH net-next 01/14] net/sched: matchall: Take verbose flag into account when logging error messages

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 ca5670fd5228..37283b306924 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,
-- 
2.33.1

[PATCH net-next 02/14] net/sched: flower: Take verbose flag into account when logging error messages

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 c80fc49c0da1..70e95ce28ffd 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;
 			}
-- 
2.33.1

[PATCH net-next 03/14] net/sched: act_api: Add extack to offload_act_setup() callback

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 3049cb69c025..9cf6870b526e 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 a3b57a93228a..8cf001aed858 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 4f51094da9da..da9733da9868 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 e0f515b774ca..22847ee009ef 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 b1f502fce595..8af9d6e5ba61 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 bde6a6c01e64..db84a0473cc1 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 d56e73843a4b..fd5155274733 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 39acd1d18609..70a6a4447e6b 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 b9ff3459fdab..23fcfa5605df 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 31fcd279c177..dc12d502c4fe 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 f4d917705263..77c17e9b46d1 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 9a22cdda6bbd..2f7f5e44d28c 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 ceba11b198bb..8cd8e506c9c9 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 23aba03d26a8..3c6f40478c81 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 883454c4f921..8c89bce99cbd 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 2957f8f5cea7..dd711ae048ff 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 70e95ce28ffd..acf827b0e30a 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 37283b306924..7553443e1ae7 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,
-- 
2.33.1

[PATCH net-next 04/14] net/sched: act_gact: Add extack messages for offload failure

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 eb8f01c819e6..832efd40e023 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 db84a0473cc1..ac29d1065232 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;
-- 
2.33.1

[PATCH net-next 05/14] net/sched: act_mirred: Add extack message for offload failure

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 70a6a4447e6b..ebb92fb072ab 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;
-- 
2.33.1

[PATCH net-next 06/14] net/sched: act_mpls: Add extack messages for offload failure

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 23fcfa5605df..adabeccb63e1 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;
-- 
2.33.1

[PATCH net-next 07/14] net/sched: act_pedit: Add extack message for offload failure

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 dc12d502c4fe..e01ef7f109f4 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);
-- 
2.33.1

[PATCH net-next 08/14] net/sched: act_police: Add extack messages for offload failure

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 77c17e9b46d1..79c8901f66ab 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;
 
-- 
2.33.1

[PATCH net-next 09/14] net/sched: act_skbedit: Add extack messages for offload failure

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 00bfee70609e..cab8229b9bed 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 8cd8e506c9c9..92d0dc754207 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;
-- 
2.33.1

[PATCH net-next 10/14] net/sched: act_tunnel_key: Add extack message for offload failure

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 3c6f40478c81..856dc23cef8c 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;
-- 
2.33.1

[PATCH net-next 11/14] net/sched: act_vlan: Add extack message for offload failure

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 8c89bce99cbd..68b5e772386a 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;
-- 
2.33.1

[PATCH net-next 12/14] net/sched: cls_api: Add extack message for unsupported action offload

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 dd711ae048ff..d8a22bedd026 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
-- 
2.33.1

[PATCH net-next 13/14] net/sched: matchall: Avoid overwriting error messages

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 7553443e1ae7..06cf22adbab7 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;
 	}
-- 
2.33.1

[PATCH net-next 14/14] net/sched: flower: Avoid overwriting error messages

[Ido Schimmel]

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 <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
---
 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 acf827b0e30a..87e030dfc328 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;
-- 
2.33.1

Re: [PATCH net-next 00/14] net/sched: Better error reporting for offload failures

[Jamal Hadi Salim]

Excellent work.

Small comment, maybe a better message is:
"Failed to setup offload flow action"?

cheers,
jamal

On 2022-04-07 03:35, Ido Schimmel wrote:
> This patchset improves error reporting to user space when offload fails
> during the flow action setup phase. That is, when failures occur in the
> actions themselves, even before calling device drivers. Requested /
> reported in [1].
> 
> This is done by passing extack to the offload_act_setup() callback and
> making use of it in the various actions.
> 
> Patches #1-#2 change matchall and flower to log error messages to user
> space in accordance with the verbose flag.
> 
> Patch #3 passes extack to the offload_act_setup() callback from the
> various call sites, including matchall and flower.
> 
> Patches #4-#11 make use of extack in the various actions to report
> offload failures.
> 
> Patch #12 adds an error message when the action does not support offload
> at all.
> 
> Patches #13-#14 change matchall and flower to stop overwriting more
> specific error messages.
> 
> [1] https://lore.kernel.org/netdev/20220317185249.5mff5u2x624pjewv@skbuf/
> 
> Ido Schimmel (14):
>    net/sched: matchall: Take verbose flag into account when logging error
>      messages
>    net/sched: flower: Take verbose flag into account when logging error
>      messages
>    net/sched: act_api: Add extack to offload_act_setup() callback
>    net/sched: act_gact: Add extack messages for offload failure
>    net/sched: act_mirred: Add extack message for offload failure
>    net/sched: act_mpls: Add extack messages for offload failure
>    net/sched: act_pedit: Add extack message for offload failure
>    net/sched: act_police: Add extack messages for offload failure
>    net/sched: act_skbedit: Add extack messages for offload failure
>    net/sched: act_tunnel_key: Add extack message for offload failure
>    net/sched: act_vlan: Add extack message for offload failure
>    net/sched: cls_api: Add extack message for unsupported action offload
>    net/sched: matchall: Avoid overwriting error messages
>    net/sched: flower: Avoid overwriting error messages
> 
>   include/net/act_api.h           |  3 ++-
>   include/net/pkt_cls.h           |  6 ++++--
>   include/net/tc_act/tc_gact.h    | 15 +++++++++++++++
>   include/net/tc_act/tc_skbedit.h | 12 ++++++++++++
>   net/sched/act_api.c             |  4 ++--
>   net/sched/act_csum.c            |  3 ++-
>   net/sched/act_ct.c              |  3 ++-
>   net/sched/act_gact.c            | 13 ++++++++++++-
>   net/sched/act_gate.c            |  3 ++-
>   net/sched/act_mirred.c          |  4 +++-
>   net/sched/act_mpls.c            | 10 +++++++++-
>   net/sched/act_pedit.c           |  4 +++-
>   net/sched/act_police.c          | 20 ++++++++++++++++----
>   net/sched/act_sample.c          |  3 ++-
>   net/sched/act_skbedit.c         | 10 +++++++++-
>   net/sched/act_tunnel_key.c      |  4 +++-
>   net/sched/act_vlan.c            |  4 +++-
>   net/sched/cls_api.c             | 22 ++++++++++++++--------
>   net/sched/cls_flower.c          | 14 ++++++--------
>   net/sched/cls_matchall.c        | 19 +++++++------------
>   20 files changed, 128 insertions(+), 48 deletions(-)
> 

Re: [PATCH net-next 00/14] net/sched: Better error reporting for offload failures

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu,  7 Apr 2022 10:35:19 +0300 you wrote:
> This patchset improves error reporting to user space when offload fails
> during the flow action setup phase. That is, when failures occur in the
> actions themselves, even before calling device drivers. Requested /
> reported in [1].
> 
> This is done by passing extack to the offload_act_setup() callback and
> making use of it in the various actions.
> 
> [...]

Here is the summary with links:
  - [net-next,01/14] net/sched: matchall: Take verbose flag into account when logging error messages
    https://git.kernel.org/netdev/net-next/c/4c096ea2d67c
  - [net-next,02/14] net/sched: flower: Take verbose flag into account when logging error messages
    https://git.kernel.org/netdev/net-next/c/11c95317bc1a
  - [net-next,03/14] net/sched: act_api: Add extack to offload_act_setup() callback
    https://git.kernel.org/netdev/net-next/c/c2ccf84ecb71
  - [net-next,04/14] net/sched: act_gact: Add extack messages for offload failure
    https://git.kernel.org/netdev/net-next/c/69642c2ab2f5
  - [net-next,05/14] net/sched: act_mirred: Add extack message for offload failure
    https://git.kernel.org/netdev/net-next/c/4dcaa50d0292
  - [net-next,06/14] net/sched: act_mpls: Add extack messages for offload failure
    https://git.kernel.org/netdev/net-next/c/bca3821d19d9
  - [net-next,07/14] net/sched: act_pedit: Add extack message for offload failure
    https://git.kernel.org/netdev/net-next/c/bf3b99e4f9ce
  - [net-next,08/14] net/sched: act_police: Add extack messages for offload failure
    https://git.kernel.org/netdev/net-next/c/b50e462bc22d
  - [net-next,09/14] net/sched: act_skbedit: Add extack messages for offload failure
    https://git.kernel.org/netdev/net-next/c/a9c64939b669
  - [net-next,10/14] net/sched: act_tunnel_key: Add extack message for offload failure
    https://git.kernel.org/netdev/net-next/c/ee367d44b936
  - [net-next,11/14] net/sched: act_vlan: Add extack message for offload failure
    https://git.kernel.org/netdev/net-next/c/f8fab3169464
  - [net-next,12/14] net/sched: cls_api: Add extack message for unsupported action offload
    https://git.kernel.org/netdev/net-next/c/c440615ffbcb
  - [net-next,13/14] net/sched: matchall: Avoid overwriting error messages
    https://git.kernel.org/netdev/net-next/c/0cba5c34b8f4
  - [net-next,14/14] net/sched: flower: Avoid overwriting error messages
    https://git.kernel.org/netdev/net-next/c/fd23e0e250c6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next 00/14] net/sched: Better error reporting for offload failures

[Marcelo Ricardo Leitner]

On Thu, Apr 07, 2022 at 10:35:19AM +0300, Ido Schimmel wrote:
> This patchset improves error reporting to user space when offload fails
> during the flow action setup phase. That is, when failures occur in the
> actions themselves, even before calling device drivers. Requested /
> reported in [1].
> 
> This is done by passing extack to the offload_act_setup() callback and
> making use of it in the various actions.
> 
> Patches #1-#2 change matchall and flower to log error messages to user
> space in accordance with the verbose flag.
> 
> Patch #3 passes extack to the offload_act_setup() callback from the
> various call sites, including matchall and flower.
> 
> Patches #4-#11 make use of extack in the various actions to report
> offload failures.
> 
> Patch #12 adds an error message when the action does not support offload
> at all.
> 
> Patches #13-#14 change matchall and flower to stop overwriting more
> specific error messages.
> 
> [1] https://lore.kernel.org/netdev/20220317185249.5mff5u2x624pjewv@skbuf/

Seems like tc is almost becoming user friendly!? :)

post-merge review FWIW,
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>