[PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[Jakub Kicinski]

Hi!

This series some of Jiri's comments and the fact that today drivers
may produce extack even if there is no skip_sw flag (meaning the
driver failure is not really a problem), and warning messages will
only confuse the users.

First patch propagates extack to destroy as requested by Jiri, extack
is then propagated to the driver callback for each classifier.  I chose
not to provide the extack on error paths.  As a rule of thumb it seems
best to keep the extack of the condition which caused the error.  E.g.

     err = this_will_fail(arg, extack);
     if (err) {
        undo_things(arg, NULL /* don't pass extack */);
	return err;
     }

Note that NL_SET_ERR_MSG() will ignore the message if extack is NULL.
I was pondering whether we should make NL_SET_ERR_MSG() refuse to
overwrite the msg, but there seem to be cases in the tree where extack
is set like this:

     err = this_will_fail(arg, extack);
     if (err) {
        undo_things(arg, NULL /* don't pass extack */);
	NL_SET_ERR_MSG(extack, "extack is set after undo call :/");
	return err;
     }

I think not passing extack to undo calls is reasonable.

v2:
 - rename the temporary tc_cls_common_offload_init().

Jakub Kicinski (12):
  net: sched: propagate extack to cls->destroy callbacks
  net: sched: prepare for reimplementation of
    tc_cls_common_offload_init()
  cls_bpf: remove gen_flags from bpf_offload
  cls_bpf: pass offload flags to tc_cls_common_offload_init()
  cls_bpf: propagate extack to offload delete callback
  cls_matchall: pass offload flags to tc_cls_common_offload_init()
  cls_matchall: propagate extack to delete callback
  cls_flower: pass offload flags to tc_cls_common_offload_init()
  cls_flower: propagate extack to delete callback
  cls_u32: pass offload flags to tc_cls_common_offload_init()
  cls_u32: propagate extack to delete callback
  net: sched: remove tc_cls_common_offload_init_deprecated()

 include/net/pkt_cls.h     | 24 ++++++++++++------------
 include/net/sch_generic.h |  3 ++-
 net/sched/cls_api.c       | 15 ++++++++-------
 net/sched/cls_basic.c     |  2 +-
 net/sched/cls_bpf.c       | 24 +++++++++++++-----------
 net/sched/cls_cgroup.c    |  3 ++-
 net/sched/cls_flow.c      |  2 +-
 net/sched/cls_flower.c    | 24 +++++++++++++-----------
 net/sched/cls_fw.c        |  2 +-
 net/sched/cls_matchall.c  | 13 +++++++------
 net/sched/cls_route.c     |  2 +-
 net/sched/cls_rsvp.h      |  2 +-
 net/sched/cls_tcindex.c   |  3 ++-
 net/sched/cls_u32.c       | 42 ++++++++++++++++++++++++------------------
 14 files changed, 88 insertions(+), 73 deletions(-)

-- 
2.15.1

[PATCH net-next v2 01/12] net: sched: propagate extack to cls->destroy callbacks

[Jakub Kicinski]

Propagate extack to cls->destroy callbacks when called from
non-error paths.  On error paths pass NULL to avoid overwriting
the failure message.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 include/net/sch_generic.h |  3 ++-
 net/sched/cls_api.c       | 15 ++++++++-------
 net/sched/cls_basic.c     |  2 +-
 net/sched/cls_bpf.c       |  3 ++-
 net/sched/cls_cgroup.c    |  3 ++-
 net/sched/cls_flow.c      |  2 +-
 net/sched/cls_flower.c    |  2 +-
 net/sched/cls_fw.c        |  2 +-
 net/sched/cls_matchall.c  |  2 +-
 net/sched/cls_route.c     |  2 +-
 net/sched/cls_rsvp.h      |  2 +-
 net/sched/cls_tcindex.c   |  3 ++-
 net/sched/cls_u32.c       |  2 +-
 13 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index cd1be1f25c36..eac43e8ca96d 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -233,7 +233,8 @@ struct tcf_proto_ops {
 					    const struct tcf_proto *,
 					    struct tcf_result *);
 	int			(*init)(struct tcf_proto*);
-	void			(*destroy)(struct tcf_proto*);
+	void			(*destroy)(struct tcf_proto *tp,
+					   struct netlink_ext_ack *extack);
 
 	void*			(*get)(struct tcf_proto*, u32 handle);
 	int			(*change)(struct net *net, struct sk_buff *,
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index f5d293416f46..bcb4ccb5f894 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -172,9 +172,10 @@ static struct tcf_proto *tcf_proto_create(const char *kind, u32 protocol,
 	return ERR_PTR(err);
 }
 
-static void tcf_proto_destroy(struct tcf_proto *tp)
+static void tcf_proto_destroy(struct tcf_proto *tp,
+			      struct netlink_ext_ack *extack)
 {
-	tp->ops->destroy(tp);
+	tp->ops->destroy(tp, extack);
 	module_put(tp->ops->owner);
 	kfree_rcu(tp, rcu);
 }
@@ -223,7 +224,7 @@ static void tcf_chain_flush(struct tcf_chain *chain)
 	tcf_chain_head_change(chain, NULL);
 	while (tp) {
 		RCU_INIT_POINTER(chain->filter_chain, tp->next);
-		tcf_proto_destroy(tp);
+		tcf_proto_destroy(tp, NULL);
 		tp = rtnl_dereference(chain->filter_chain);
 		tcf_chain_put(chain);
 	}
@@ -1182,7 +1183,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 			tcf_chain_tp_remove(chain, &chain_info, tp);
 			tfilter_notify(net, skb, n, tp, block, q, parent, fh,
 				       RTM_DELTFILTER, false);
-			tcf_proto_destroy(tp);
+			tcf_proto_destroy(tp, extack);
 			err = 0;
 			goto errout;
 		}
@@ -1200,7 +1201,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 		case RTM_NEWTFILTER:
 			if (n->nlmsg_flags & NLM_F_EXCL) {
 				if (tp_created)
-					tcf_proto_destroy(tp);
+					tcf_proto_destroy(tp, NULL);
 				NL_SET_ERR_MSG(extack, "Filter already exists");
 				err = -EEXIST;
 				goto errout;
@@ -1214,7 +1215,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 				goto errout;
 			if (last) {
 				tcf_chain_tp_remove(chain, &chain_info, tp);
-				tcf_proto_destroy(tp);
+				tcf_proto_destroy(tp, extack);
 			}
 			goto errout;
 		case RTM_GETTFILTER:
@@ -1240,7 +1241,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 			       RTM_NEWTFILTER, false);
 	} else {
 		if (tp_created)
-			tcf_proto_destroy(tp);
+			tcf_proto_destroy(tp, NULL);
 	}
 
 errout:
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 6088be65d167..d333f5c5101d 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -112,7 +112,7 @@ static void basic_delete_filter(struct rcu_head *head)
 	tcf_queue_work(&f->work);
 }
 
-static void basic_destroy(struct tcf_proto *tp)
+static void basic_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 {
 	struct basic_head *head = rtnl_dereference(tp->root);
 	struct basic_filter *f, *n;
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index c11e0fe23a17..a562b9a39e71 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -314,7 +314,8 @@ static int cls_bpf_delete(struct tcf_proto *tp, void *arg, bool *last,
 	return 0;
 }
 
-static void cls_bpf_destroy(struct tcf_proto *tp)
+static void cls_bpf_destroy(struct tcf_proto *tp,
+			    struct netlink_ext_ack *extack)
 {
 	struct cls_bpf_head *head = rtnl_dereference(tp->root);
 	struct cls_bpf_prog *prog, *tmp;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 1b54fbfca414..762da5c0cf5e 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -143,7 +143,8 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 	return err;
 }
 
-static void cls_cgroup_destroy(struct tcf_proto *tp)
+static void cls_cgroup_destroy(struct tcf_proto *tp,
+			       struct netlink_ext_ack *extack)
 {
 	struct cls_cgroup_head *head = rtnl_dereference(tp->root);
 
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 64c24b488058..cd5fe383afdd 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -600,7 +600,7 @@ static int flow_init(struct tcf_proto *tp)
 	return 0;
 }
 
-static void flow_destroy(struct tcf_proto *tp)
+static void flow_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 {
 	struct flow_head *head = rtnl_dereference(tp->root);
 	struct flow_filter *f, *next;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 727c10378f37..213be0e6f1d1 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -315,7 +315,7 @@ static void fl_destroy_rcu(struct rcu_head *rcu)
 	schedule_work(&head->work);
 }
 
-static void fl_destroy(struct tcf_proto *tp)
+static void fl_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 {
 	struct cls_fl_head *head = rtnl_dereference(tp->root);
 	struct cls_fl_filter *f, *next;
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 94d159a8869a..8b207723fbc2 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -149,7 +149,7 @@ static void fw_delete_filter(struct rcu_head *head)
 	tcf_queue_work(&f->work);
 }
 
-static void fw_destroy(struct tcf_proto *tp)
+static void fw_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 {
 	struct fw_head *head = rtnl_dereference(tp->root);
 	struct fw_filter *f;
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index d990d2a52c6d..2de2338f4030 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -114,7 +114,7 @@ static int mall_replace_hw_filter(struct tcf_proto *tp,
 	return 0;
 }
 
-static void mall_destroy(struct tcf_proto *tp)
+static void mall_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 {
 	struct cls_mall_head *head = rtnl_dereference(tp->root);
 
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 55467c30d524..21a03a8ee029 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -281,7 +281,7 @@ static void route4_delete_filter(struct rcu_head *head)
 	tcf_queue_work(&f->work);
 }
 
-static void route4_destroy(struct tcf_proto *tp)
+static void route4_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 {
 	struct route4_head *head = rtnl_dereference(tp->root);
 	int h1, h2;
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index 5cc0df690cff..4f1297657c27 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -322,7 +322,7 @@ static void rsvp_delete_filter(struct tcf_proto *tp, struct rsvp_filter *f)
 		__rsvp_delete_filter(f);
 }
 
-static void rsvp_destroy(struct tcf_proto *tp)
+static void rsvp_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 {
 	struct rsvp_head *data = rtnl_dereference(tp->root);
 	int h1, h2;
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 01a163e0b6aa..b49cc990a000 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -581,7 +581,8 @@ static void tcindex_walk(struct tcf_proto *tp, struct tcf_walker *walker)
 	}
 }
 
-static void tcindex_destroy(struct tcf_proto *tp)
+static void tcindex_destroy(struct tcf_proto *tp,
+			    struct netlink_ext_ack *extack)
 {
 	struct tcindex_data *p = rtnl_dereference(tp->root);
 	struct tcf_walker walker;
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 7030240f8826..98cabe835fd8 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -638,7 +638,7 @@ static bool ht_empty(struct tc_u_hnode *ht)
 	return true;
 }
 
-static void u32_destroy(struct tcf_proto *tp)
+static void u32_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 {
 	struct tc_u_common *tp_c = tp->data;
 	struct tc_u_hnode *root_ht = rtnl_dereference(tp->root);
-- 
2.15.1

[PATCH net-next v2 02/12] net: sched: prepare for reimplementation of tc_cls_common_offload_init()

[Jakub Kicinski]

Rename the tc_cls_common_offload_init() helper function to
tc_cls_common_offload_init_deprecated() and add a new implementation
which also takes flags argument.  We will only set extack if flags
indicate that offload is forced (skip_sw) otherwise driver errors
should be ignored, as they don't influence the overall filter
installation.

Note that we need the tc_skip_hw() helper for new version, therefore
it is added later in the file.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 include/net/pkt_cls.h    | 18 +++++++++++++++---
 net/sched/cls_bpf.c      |  4 ++--
 net/sched/cls_flower.c   |  6 +++---
 net/sched/cls_matchall.c |  4 ++--
 net/sched/cls_u32.c      |  8 ++++----
 5 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 2f8f16a4d88e..08815fe9314d 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -606,9 +606,9 @@ struct tc_cls_common_offload {
 };
 
 static inline void
-tc_cls_common_offload_init(struct tc_cls_common_offload *cls_common,
-			   const struct tcf_proto *tp,
-			   struct netlink_ext_ack *extack)
+tc_cls_common_offload_init_deprecated(struct tc_cls_common_offload *cls_common,
+				      const struct tcf_proto *tp,
+				      struct netlink_ext_ack *extack)
 {
 	cls_common->chain_index = tp->chain->index;
 	cls_common->protocol = tp->protocol;
@@ -694,6 +694,18 @@ static inline bool tc_in_hw(u32 flags)
 	return (flags & TCA_CLS_FLAGS_IN_HW) ? true : false;
 }
 
+static inline void
+tc_cls_common_offload_init(struct tc_cls_common_offload *cls_common,
+			   const struct tcf_proto *tp, u32 flags,
+			   struct netlink_ext_ack *extack)
+{
+	cls_common->chain_index = tp->chain->index;
+	cls_common->protocol = tp->protocol;
+	cls_common->prio = tp->prio;
+	if (tc_skip_sw(flags))
+		cls_common->extack = extack;
+}
+
 enum tc_fl_command {
 	TC_CLSFLOWER_REPLACE,
 	TC_CLSFLOWER_DESTROY,
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index a562b9a39e71..0bffb189d646 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -159,7 +159,7 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	skip_sw = prog && tc_skip_sw(prog->gen_flags);
 	obj = prog ?: oldprog;
 
-	tc_cls_common_offload_init(&cls_bpf.common, tp, extack);
+	tc_cls_common_offload_init_deprecated(&cls_bpf.common, tp, extack);
 	cls_bpf.command = TC_CLSBPF_OFFLOAD;
 	cls_bpf.exts = &obj->exts;
 	cls_bpf.prog = prog ? prog->filter : NULL;
@@ -227,7 +227,7 @@ static void cls_bpf_offload_update_stats(struct tcf_proto *tp,
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_bpf_offload cls_bpf = {};
 
-	tc_cls_common_offload_init(&cls_bpf.common, tp, NULL);
+	tc_cls_common_offload_init_deprecated(&cls_bpf.common, tp, NULL);
 	cls_bpf.command = TC_CLSBPF_STATS;
 	cls_bpf.exts = &prog->exts;
 	cls_bpf.prog = prog->filter;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 213be0e6f1d1..3f2654ca8ff7 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -223,7 +223,7 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f)
 	struct tc_cls_flower_offload cls_flower = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init(&cls_flower.common, tp, NULL);
+	tc_cls_common_offload_init_deprecated(&cls_flower.common, tp, NULL);
 	cls_flower.command = TC_CLSFLOWER_DESTROY;
 	cls_flower.cookie = (unsigned long) f;
 
@@ -243,7 +243,7 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
 	bool skip_sw = tc_skip_sw(f->flags);
 	int err;
 
-	tc_cls_common_offload_init(&cls_flower.common, tp, extack);
+	tc_cls_common_offload_init_deprecated(&cls_flower.common, tp, extack);
 	cls_flower.command = TC_CLSFLOWER_REPLACE;
 	cls_flower.cookie = (unsigned long) f;
 	cls_flower.dissector = dissector;
@@ -272,7 +272,7 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f)
 	struct tc_cls_flower_offload cls_flower = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init(&cls_flower.common, tp, NULL);
+	tc_cls_common_offload_init_deprecated(&cls_flower.common, tp, NULL);
 	cls_flower.command = TC_CLSFLOWER_STATS;
 	cls_flower.cookie = (unsigned long) f;
 	cls_flower.exts = &f->exts;
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 2de2338f4030..a9a535a7a431 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -76,7 +76,7 @@ static void mall_destroy_hw_filter(struct tcf_proto *tp,
 	struct tc_cls_matchall_offload cls_mall = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init(&cls_mall.common, tp, NULL);
+	tc_cls_common_offload_init_deprecated(&cls_mall.common, tp, NULL);
 	cls_mall.command = TC_CLSMATCHALL_DESTROY;
 	cls_mall.cookie = cookie;
 
@@ -94,7 +94,7 @@ static int mall_replace_hw_filter(struct tcf_proto *tp,
 	bool skip_sw = tc_skip_sw(head->flags);
 	int err;
 
-	tc_cls_common_offload_init(&cls_mall.common, tp, extack);
+	tc_cls_common_offload_init_deprecated(&cls_mall.common, tp, extack);
 	cls_mall.command = TC_CLSMATCHALL_REPLACE;
 	cls_mall.exts = &head->exts;
 	cls_mall.cookie = cookie;
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 98cabe835fd8..e2e8d08c4a0d 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -491,7 +491,7 @@ static void u32_clear_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h)
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
 
-	tc_cls_common_offload_init(&cls_u32.common, tp, NULL);
+	tc_cls_common_offload_init_deprecated(&cls_u32.common, tp, NULL);
 	cls_u32.command = TC_CLSU32_DELETE_HNODE;
 	cls_u32.hnode.divisor = h->divisor;
 	cls_u32.hnode.handle = h->handle;
@@ -509,7 +509,7 @@ static int u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h,
 	bool offloaded = false;
 	int err;
 
-	tc_cls_common_offload_init(&cls_u32.common, tp, extack);
+	tc_cls_common_offload_init_deprecated(&cls_u32.common, tp, extack);
 	cls_u32.command = TC_CLSU32_NEW_HNODE;
 	cls_u32.hnode.divisor = h->divisor;
 	cls_u32.hnode.handle = h->handle;
@@ -534,7 +534,7 @@ static void u32_remove_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n)
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
 
-	tc_cls_common_offload_init(&cls_u32.common, tp, NULL);
+	tc_cls_common_offload_init_deprecated(&cls_u32.common, tp, NULL);
 	cls_u32.command = TC_CLSU32_DELETE_KNODE;
 	cls_u32.knode.handle = n->handle;
 
@@ -550,7 +550,7 @@ static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 	bool skip_sw = tc_skip_sw(flags);
 	int err;
 
-	tc_cls_common_offload_init(&cls_u32.common, tp, extack);
+	tc_cls_common_offload_init_deprecated(&cls_u32.common, tp, extack);
 	cls_u32.command = TC_CLSU32_REPLACE_KNODE;
 	cls_u32.knode.handle = n->handle;
 	cls_u32.knode.fshift = n->fshift;
-- 
2.15.1

[PATCH net-next v2 03/12] cls_bpf: remove gen_flags from bpf_offload

[Jakub Kicinski]

cls_bpf now guarantees that only device-bound programs are
allowed with skip_sw.  The drivers no longer pay attention to
flags on filter load, therefore the bpf_offload member can be
removed.  If flags are needed again they should probably be
added to struct tc_cls_common_offload instead.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 include/net/pkt_cls.h | 1 -
 net/sched/cls_bpf.c   | 2 --
 2 files changed, 3 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 08815fe9314d..85cee929b9ce 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -748,7 +748,6 @@ struct tc_cls_bpf_offload {
 	struct bpf_prog *oldprog;
 	const char *name;
 	bool exts_integrated;
-	u32 gen_flags;
 };
 
 struct tc_mqprio_qopt_offload {
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 0bffb189d646..b8f953d00c46 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -166,7 +166,6 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	cls_bpf.oldprog = oldprog ? oldprog->filter : NULL;
 	cls_bpf.name = obj->bpf_name;
 	cls_bpf.exts_integrated = obj->exts_integrated;
-	cls_bpf.gen_flags = obj->gen_flags;
 
 	if (oldprog)
 		tcf_block_offload_dec(block, &oldprog->gen_flags);
@@ -233,7 +232,6 @@ static void cls_bpf_offload_update_stats(struct tcf_proto *tp,
 	cls_bpf.prog = prog->filter;
 	cls_bpf.name = prog->bpf_name;
 	cls_bpf.exts_integrated = prog->exts_integrated;
-	cls_bpf.gen_flags = prog->gen_flags;
 
 	tc_setup_cb_call(block, NULL, TC_SETUP_CLSBPF, &cls_bpf, false);
 }
-- 
2.15.1

[PATCH net-next v2 04/12] cls_bpf: pass offload flags to tc_cls_common_offload_init()

[Jakub Kicinski]

Pass offload flags to the new implementation of
tc_cls_common_offload_init().  Extack will now only
be set if user requested skip_sw.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 net/sched/cls_bpf.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index b8f953d00c46..323b01f76a4c 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -159,7 +159,8 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	skip_sw = prog && tc_skip_sw(prog->gen_flags);
 	obj = prog ?: oldprog;
 
-	tc_cls_common_offload_init_deprecated(&cls_bpf.common, tp, extack);
+	tc_cls_common_offload_init(&cls_bpf.common, tp, obj->gen_flags,
+				   extack);
 	cls_bpf.command = TC_CLSBPF_OFFLOAD;
 	cls_bpf.exts = &obj->exts;
 	cls_bpf.prog = prog ? prog->filter : NULL;
@@ -226,7 +227,7 @@ static void cls_bpf_offload_update_stats(struct tcf_proto *tp,
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_bpf_offload cls_bpf = {};
 
-	tc_cls_common_offload_init_deprecated(&cls_bpf.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_bpf.common, tp, prog->gen_flags, NULL);
 	cls_bpf.command = TC_CLSBPF_STATS;
 	cls_bpf.exts = &prog->exts;
 	cls_bpf.prog = prog->filter;
-- 
2.15.1

[PATCH net-next v2 05/12] cls_bpf: propagate extack to offload delete callback

[Jakub Kicinski]

Propagate extack on removal of offloaded filter.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 net/sched/cls_bpf.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 323b01f76a4c..8e5326bc6440 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -212,11 +212,12 @@ static int cls_bpf_offload(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 }
 
 static void cls_bpf_stop_offload(struct tcf_proto *tp,
-				 struct cls_bpf_prog *prog)
+				 struct cls_bpf_prog *prog,
+				 struct netlink_ext_ack *extack)
 {
 	int err;
 
-	err = cls_bpf_offload_cmd(tp, NULL, prog, NULL);
+	err = cls_bpf_offload_cmd(tp, NULL, prog, extack);
 	if (err)
 		pr_err("Stopping hardware offload failed: %d\n", err);
 }
@@ -289,12 +290,13 @@ static void cls_bpf_delete_prog_rcu(struct rcu_head *rcu)
 	tcf_queue_work(&prog->work);
 }
 
-static void __cls_bpf_delete(struct tcf_proto *tp, struct cls_bpf_prog *prog)
+static void __cls_bpf_delete(struct tcf_proto *tp, struct cls_bpf_prog *prog,
+			     struct netlink_ext_ack *extack)
 {
 	struct cls_bpf_head *head = rtnl_dereference(tp->root);
 
 	idr_remove_ext(&head->handle_idr, prog->handle);
-	cls_bpf_stop_offload(tp, prog);
+	cls_bpf_stop_offload(tp, prog, extack);
 	list_del_rcu(&prog->link);
 	tcf_unbind_filter(tp, &prog->res);
 	if (tcf_exts_get_net(&prog->exts))
@@ -308,7 +310,7 @@ static int cls_bpf_delete(struct tcf_proto *tp, void *arg, bool *last,
 {
 	struct cls_bpf_head *head = rtnl_dereference(tp->root);
 
-	__cls_bpf_delete(tp, arg);
+	__cls_bpf_delete(tp, arg, extack);
 	*last = list_empty(&head->plist);
 	return 0;
 }
@@ -320,7 +322,7 @@ static void cls_bpf_destroy(struct tcf_proto *tp,
 	struct cls_bpf_prog *prog, *tmp;
 
 	list_for_each_entry_safe(prog, tmp, &head->plist, link)
-		__cls_bpf_delete(tp, prog);
+		__cls_bpf_delete(tp, prog, extack);
 
 	idr_destroy(&head->handle_idr);
 	kfree_rcu(head, rcu);
-- 
2.15.1

[PATCH net-next v2 06/12] cls_matchall: pass offload flags to tc_cls_common_offload_init()

[Jakub Kicinski]

Pass offload flags to the new implementation of
tc_cls_common_offload_init().  Extack will now only
be set if user requested skip_sw.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 net/sched/cls_matchall.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index a9a535a7a431..b0b8627b53d2 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -76,7 +76,7 @@ static void mall_destroy_hw_filter(struct tcf_proto *tp,
 	struct tc_cls_matchall_offload cls_mall = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init_deprecated(&cls_mall.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_mall.common, tp, head->flags, NULL);
 	cls_mall.command = TC_CLSMATCHALL_DESTROY;
 	cls_mall.cookie = cookie;
 
@@ -94,7 +94,7 @@ static int mall_replace_hw_filter(struct tcf_proto *tp,
 	bool skip_sw = tc_skip_sw(head->flags);
 	int err;
 
-	tc_cls_common_offload_init_deprecated(&cls_mall.common, tp, extack);
+	tc_cls_common_offload_init(&cls_mall.common, tp, head->flags, extack);
 	cls_mall.command = TC_CLSMATCHALL_REPLACE;
 	cls_mall.exts = &head->exts;
 	cls_mall.cookie = cookie;
-- 
2.15.1

[PATCH net-next v2 07/12] cls_matchall: propagate extack to delete callback

[Jakub Kicinski]

Propagate extack on removal of offloaded filter.  Don't pass
extack from error paths.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 net/sched/cls_matchall.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index b0b8627b53d2..2ba721a590a7 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -71,12 +71,13 @@ static void mall_destroy_rcu(struct rcu_head *rcu)
 
 static void mall_destroy_hw_filter(struct tcf_proto *tp,
 				   struct cls_mall_head *head,
-				   unsigned long cookie)
+				   unsigned long cookie,
+				   struct netlink_ext_ack *extack)
 {
 	struct tc_cls_matchall_offload cls_mall = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init(&cls_mall.common, tp, head->flags, NULL);
+	tc_cls_common_offload_init(&cls_mall.common, tp, head->flags, extack);
 	cls_mall.command = TC_CLSMATCHALL_DESTROY;
 	cls_mall.cookie = cookie;
 
@@ -102,7 +103,7 @@ static int mall_replace_hw_filter(struct tcf_proto *tp,
 	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSMATCHALL,
 			       &cls_mall, skip_sw);
 	if (err < 0) {
-		mall_destroy_hw_filter(tp, head, cookie);
+		mall_destroy_hw_filter(tp, head, cookie, NULL);
 		return err;
 	} else if (err > 0) {
 		tcf_block_offload_inc(block, &head->flags);
@@ -122,7 +123,7 @@ static void mall_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 		return;
 
 	if (!tc_skip_hw(head->flags))
-		mall_destroy_hw_filter(tp, head, (unsigned long) head);
+		mall_destroy_hw_filter(tp, head, (unsigned long) head, extack);
 
 	if (tcf_exts_get_net(&head->exts))
 		call_rcu(&head->rcu, mall_destroy_rcu);
-- 
2.15.1

[PATCH net-next v2 08/12] cls_flower: pass offload flags to tc_cls_common_offload_init()

[Jakub Kicinski]

Pass offload flags to the new implementation of
tc_cls_common_offload_init().  Extack will now only
be set if user requested skip_sw.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 net/sched/cls_flower.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 3f2654ca8ff7..79aa5049f028 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -223,7 +223,7 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f)
 	struct tc_cls_flower_offload cls_flower = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init_deprecated(&cls_flower.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_flower.common, tp, f->flags, NULL);
 	cls_flower.command = TC_CLSFLOWER_DESTROY;
 	cls_flower.cookie = (unsigned long) f;
 
@@ -243,7 +243,7 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
 	bool skip_sw = tc_skip_sw(f->flags);
 	int err;
 
-	tc_cls_common_offload_init_deprecated(&cls_flower.common, tp, extack);
+	tc_cls_common_offload_init(&cls_flower.common, tp, f->flags, extack);
 	cls_flower.command = TC_CLSFLOWER_REPLACE;
 	cls_flower.cookie = (unsigned long) f;
 	cls_flower.dissector = dissector;
@@ -272,7 +272,7 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f)
 	struct tc_cls_flower_offload cls_flower = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init_deprecated(&cls_flower.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_flower.common, tp, f->flags, NULL);
 	cls_flower.command = TC_CLSFLOWER_STATS;
 	cls_flower.cookie = (unsigned long) f;
 	cls_flower.exts = &f->exts;
-- 
2.15.1

[PATCH net-next v2 09/12] cls_flower: propagate extack to delete callback

[Jakub Kicinski]

Propagate extack on removal of offloaded filter.  Don't pass
extack from error paths.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 net/sched/cls_flower.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 79aa5049f028..dc9acaafc0a8 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -218,12 +218,13 @@ static void fl_destroy_filter(struct rcu_head *head)
 	tcf_queue_work(&f->work);
 }
 
-static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f)
+static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f,
+				 struct netlink_ext_ack *extack)
 {
 	struct tc_cls_flower_offload cls_flower = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init(&cls_flower.common, tp, f->flags, NULL);
+	tc_cls_common_offload_init(&cls_flower.common, tp, f->flags, extack);
 	cls_flower.command = TC_CLSFLOWER_DESTROY;
 	cls_flower.cookie = (unsigned long) f;
 
@@ -255,7 +256,7 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
 	err = tc_setup_cb_call(block, &f->exts, TC_SETUP_CLSFLOWER,
 			       &cls_flower, skip_sw);
 	if (err < 0) {
-		fl_hw_destroy_filter(tp, f);
+		fl_hw_destroy_filter(tp, f, NULL);
 		return err;
 	} else if (err > 0) {
 		tcf_block_offload_inc(block, &f->flags);
@@ -282,14 +283,15 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f)
 			 &cls_flower, false);
 }
 
-static void __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f)
+static void __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f,
+			struct netlink_ext_ack *extack)
 {
 	struct cls_fl_head *head = rtnl_dereference(tp->root);
 
 	idr_remove_ext(&head->handle_idr, f->handle);
 	list_del_rcu(&f->list);
 	if (!tc_skip_hw(f->flags))
-		fl_hw_destroy_filter(tp, f);
+		fl_hw_destroy_filter(tp, f, extack);
 	tcf_unbind_filter(tp, &f->res);
 	if (tcf_exts_get_net(&f->exts))
 		call_rcu(&f->rcu, fl_destroy_filter);
@@ -321,7 +323,7 @@ static void fl_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 	struct cls_fl_filter *f, *next;
 
 	list_for_each_entry_safe(f, next, &head->filters, list)
-		__fl_delete(tp, f);
+		__fl_delete(tp, f, extack);
 	idr_destroy(&head->handle_idr);
 
 	__module_get(THIS_MODULE);
@@ -958,7 +960,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 			rhashtable_remove_fast(&head->ht, &fold->ht_node,
 					       head->ht_params);
 		if (!tc_skip_hw(fold->flags))
-			fl_hw_destroy_filter(tp, fold);
+			fl_hw_destroy_filter(tp, fold, NULL);
 	}
 
 	*arg = fnew;
@@ -997,7 +999,7 @@ static int fl_delete(struct tcf_proto *tp, void *arg, bool *last,
 	if (!tc_skip_sw(f->flags))
 		rhashtable_remove_fast(&head->ht, &f->ht_node,
 				       head->ht_params);
-	__fl_delete(tp, f);
+	__fl_delete(tp, f, extack);
 	*last = list_empty(&head->filters);
 	return 0;
 }
-- 
2.15.1

[PATCH net-next v2 10/12] cls_u32: pass offload flags to tc_cls_common_offload_init()

[Jakub Kicinski]

Pass offload flags to the new implementation of
tc_cls_common_offload_init().  Extack will now only
be set if user requested skip_sw.  hnodes need to
hold onto the flags now to be able to reuse them
on filter removal.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 net/sched/cls_u32.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index e2e8d08c4a0d..21e84abe4226 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -87,6 +87,7 @@ struct tc_u_hnode {
 	unsigned int		divisor;
 	struct idr		handle_idr;
 	struct rcu_head		rcu;
+	u32			flags;
 	/* The 'ht' field MUST be the last field in structure to allow for
 	 * more entries allocated at end of structure.
 	 */
@@ -491,7 +492,7 @@ static void u32_clear_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h)
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
 
-	tc_cls_common_offload_init_deprecated(&cls_u32.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_u32.common, tp, h->flags, NULL);
 	cls_u32.command = TC_CLSU32_DELETE_HNODE;
 	cls_u32.hnode.divisor = h->divisor;
 	cls_u32.hnode.handle = h->handle;
@@ -509,7 +510,7 @@ static int u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h,
 	bool offloaded = false;
 	int err;
 
-	tc_cls_common_offload_init_deprecated(&cls_u32.common, tp, extack);
+	tc_cls_common_offload_init(&cls_u32.common, tp, flags, extack);
 	cls_u32.command = TC_CLSU32_NEW_HNODE;
 	cls_u32.hnode.divisor = h->divisor;
 	cls_u32.hnode.handle = h->handle;
@@ -534,7 +535,7 @@ static void u32_remove_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n)
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
 
-	tc_cls_common_offload_init_deprecated(&cls_u32.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_u32.common, tp, n->flags, NULL);
 	cls_u32.command = TC_CLSU32_DELETE_KNODE;
 	cls_u32.knode.handle = n->handle;
 
@@ -550,7 +551,7 @@ static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 	bool skip_sw = tc_skip_sw(flags);
 	int err;
 
-	tc_cls_common_offload_init_deprecated(&cls_u32.common, tp, extack);
+	tc_cls_common_offload_init(&cls_u32.common, tp, flags, extack);
 	cls_u32.command = TC_CLSU32_REPLACE_KNODE;
 	cls_u32.knode.handle = n->handle;
 	cls_u32.knode.fshift = n->fshift;
@@ -1015,6 +1016,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		ht->handle = handle;
 		ht->prio = tp->prio;
 		idr_init(&ht->handle_idr);
+		ht->flags = flags;
 
 		err = u32_replace_hw_hnode(tp, ht, flags, extack);
 		if (err) {
-- 
2.15.1

[PATCH net-next v2 11/12] cls_u32: propagate extack to delete callback

[Jakub Kicinski]

Propagate extack on removal of offloaded filter.  Don't pass
extack from error paths.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 net/sched/cls_u32.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 21e84abe4226..60c892c36a60 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -487,12 +487,13 @@ static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
 	return 0;
 }
 
-static void u32_clear_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h)
+static void u32_clear_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h,
+			       struct netlink_ext_ack *extack)
 {
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
 
-	tc_cls_common_offload_init(&cls_u32.common, tp, h->flags, NULL);
+	tc_cls_common_offload_init(&cls_u32.common, tp, h->flags, extack);
 	cls_u32.command = TC_CLSU32_DELETE_HNODE;
 	cls_u32.hnode.divisor = h->divisor;
 	cls_u32.hnode.handle = h->handle;
@@ -518,7 +519,7 @@ static int u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h,
 
 	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSU32, &cls_u32, skip_sw);
 	if (err < 0) {
-		u32_clear_hw_hnode(tp, h);
+		u32_clear_hw_hnode(tp, h, NULL);
 		return err;
 	} else if (err > 0) {
 		offloaded = true;
@@ -530,12 +531,13 @@ static int u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h,
 	return 0;
 }
 
-static void u32_remove_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n)
+static void u32_remove_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
+				struct netlink_ext_ack *extack)
 {
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
 
-	tc_cls_common_offload_init(&cls_u32.common, tp, n->flags, NULL);
+	tc_cls_common_offload_init(&cls_u32.common, tp, n->flags, extack);
 	cls_u32.command = TC_CLSU32_DELETE_KNODE;
 	cls_u32.knode.handle = n->handle;
 
@@ -569,7 +571,7 @@ static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 
 	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSU32, &cls_u32, skip_sw);
 	if (err < 0) {
-		u32_remove_hw_knode(tp, n);
+		u32_remove_hw_knode(tp, n, NULL);
 		return err;
 	} else if (err > 0) {
 		tcf_block_offload_inc(block, &n->flags);
@@ -581,7 +583,8 @@ static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 	return 0;
 }
 
-static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
+static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
+			    struct netlink_ext_ack *extack)
 {
 	struct tc_u_knode *n;
 	unsigned int h;
@@ -591,7 +594,7 @@ static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
 			RCU_INIT_POINTER(ht->ht[h],
 					 rtnl_dereference(n->next));
 			tcf_unbind_filter(tp, &n->res);
-			u32_remove_hw_knode(tp, n);
+			u32_remove_hw_knode(tp, n, extack);
 			idr_remove_ext(&ht->handle_idr, n->handle);
 			if (tcf_exts_get_net(&n->exts))
 				call_rcu(&n->rcu, u32_delete_key_freepf_rcu);
@@ -601,7 +604,8 @@ static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
 	}
 }
 
-static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
+static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
+			     struct netlink_ext_ack *extack)
 {
 	struct tc_u_common *tp_c = tp->data;
 	struct tc_u_hnode __rcu **hn;
@@ -609,14 +613,14 @@ static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
 
 	WARN_ON(ht->refcnt);
 
-	u32_clear_hnode(tp, ht);
+	u32_clear_hnode(tp, ht, extack);
 
 	hn = &tp_c->hlist;
 	for (phn = rtnl_dereference(*hn);
 	     phn;
 	     hn = &phn->next, phn = rtnl_dereference(*hn)) {
 		if (phn == ht) {
-			u32_clear_hw_hnode(tp, ht);
+			u32_clear_hw_hnode(tp, ht, extack);
 			idr_destroy(&ht->handle_idr);
 			idr_remove_ext(&tp_c->handle_idr, ht->handle);
 			RCU_INIT_POINTER(*hn, ht->next);
@@ -647,7 +651,7 @@ static void u32_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 	WARN_ON(root_ht == NULL);
 
 	if (root_ht && --root_ht->refcnt == 0)
-		u32_destroy_hnode(tp, root_ht);
+		u32_destroy_hnode(tp, root_ht, extack);
 
 	if (--tp_c->refcnt == 0) {
 		struct tc_u_hnode *ht;
@@ -658,7 +662,7 @@ static void u32_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
 		     ht;
 		     ht = rtnl_dereference(ht->next)) {
 			ht->refcnt--;
-			u32_clear_hnode(tp, ht);
+			u32_clear_hnode(tp, ht, extack);
 		}
 
 		while ((ht = rtnl_dereference(tp_c->hlist)) != NULL) {
@@ -685,7 +689,7 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
 		goto out;
 
 	if (TC_U32_KEY(ht->handle)) {
-		u32_remove_hw_knode(tp, (struct tc_u_knode *)ht);
+		u32_remove_hw_knode(tp, (struct tc_u_knode *)ht, extack);
 		ret = u32_delete_key(tp, (struct tc_u_knode *)ht);
 		goto out;
 	}
@@ -697,7 +701,7 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
 
 	if (ht->refcnt == 1) {
 		ht->refcnt--;
-		u32_destroy_hnode(tp, ht);
+		u32_destroy_hnode(tp, ht, extack);
 	} else {
 		NL_SET_ERR_MSG_MOD(extack, "Can not delete in-use filter");
 		return -EBUSY;
-- 
2.15.1

[PATCH net-next v2 12/12] net: sched: remove tc_cls_common_offload_init_deprecated()

[Jakub Kicinski]

All users are now converted to tc_cls_common_offload_init().

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 include/net/pkt_cls.h | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 85cee929b9ce..1a41513cec7f 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -605,17 +605,6 @@ struct tc_cls_common_offload {
 	struct netlink_ext_ack *extack;
 };
 
-static inline void
-tc_cls_common_offload_init_deprecated(struct tc_cls_common_offload *cls_common,
-				      const struct tcf_proto *tp,
-				      struct netlink_ext_ack *extack)
-{
-	cls_common->chain_index = tp->chain->index;
-	cls_common->protocol = tp->protocol;
-	cls_common->prio = tp->prio;
-	cls_common->extack = extack;
-}
-
 struct tc_cls_u32_knode {
 	struct tcf_exts *exts;
 	struct tc_u32_sel *sel;
-- 
2.15.1

Re: [PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[David Miller]

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed, 24 Jan 2018 12:54:12 -0800

> This series some of Jiri's comments and the fact that today drivers
> may produce extack even if there is no skip_sw flag (meaning the
> driver failure is not really a problem), and warning messages will
> only confuse the users.
...
> v2:
>  - rename the temporary tc_cls_common_offload_init().

This looks better, series applied, thanks Jakub.

Re: [PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[Jiri Pirko]

Wed, Jan 24, 2018 at 09:54:12PM CET, jakub.kicinski@netronome.com wrote:
>Hi!
>
>This series some of Jiri's comments and the fact that today drivers
>may produce extack even if there is no skip_sw flag (meaning the
>driver failure is not really a problem), and warning messages will
>only confuse the users.
>
>First patch propagates extack to destroy as requested by Jiri, extack
>is then propagated to the driver callback for each classifier.  I chose
>not to provide the extack on error paths.  As a rule of thumb it seems
>best to keep the extack of the condition which caused the error.  E.g.
>
>     err = this_will_fail(arg, extack);
>     if (err) {
>        undo_things(arg, NULL /* don't pass extack */);
>	return err;
>     }
>
>Note that NL_SET_ERR_MSG() will ignore the message if extack is NULL.
>I was pondering whether we should make NL_SET_ERR_MSG() refuse to
>overwrite the msg, but there seem to be cases in the tree where extack
>is set like this:
>
>     err = this_will_fail(arg, extack);
>     if (err) {
>        undo_things(arg, NULL /* don't pass extack */);
>	NL_SET_ERR_MSG(extack, "extack is set after undo call :/");
>	return err;
>     }
>
>I think not passing extack to undo calls is reasonable.
>
>v2:
> - rename the temporary tc_cls_common_offload_init().
>
>Jakub Kicinski (12):
>  net: sched: propagate extack to cls->destroy callbacks
>  net: sched: prepare for reimplementation of
>    tc_cls_common_offload_init()
>  cls_bpf: remove gen_flags from bpf_offload
>  cls_bpf: pass offload flags to tc_cls_common_offload_init()
>  cls_bpf: propagate extack to offload delete callback
>  cls_matchall: pass offload flags to tc_cls_common_offload_init()
>  cls_matchall: propagate extack to delete callback
>  cls_flower: pass offload flags to tc_cls_common_offload_init()
>  cls_flower: propagate extack to delete callback
>  cls_u32: pass offload flags to tc_cls_common_offload_init()
>  cls_u32: propagate extack to delete callback
>  net: sched: remove tc_cls_common_offload_init_deprecated()

For the record, I still think it is odd to have 6 patches just to add
one arg to a function. I wonder where this unnecessary patch splits
would lead to in the future.

Anyway, since apparently no one really cares, and the code result looks
good to me, for whole patchset:
Acked-by: Jiri Pirko <jiri@mellanox.com>

Re: [PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[David Ahern]

On 1/24/18 2:04 PM, Jiri Pirko wrote:
> For the record, I still think it is odd to have 6 patches just to add
> one arg to a function. I wonder where this unnecessary patch splits
> would lead to in the future.

I think it made the review much easier than 1 really long patch.

Re: [PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[Jiri Pirko]

Wed, Jan 24, 2018 at 10:07:25PM CET, dsahern@gmail.com wrote:
>On 1/24/18 2:04 PM, Jiri Pirko wrote:
>> For the record, I still think it is odd to have 6 patches just to add
>> one arg to a function. I wonder where this unnecessary patch splits
>> would lead to in the future.
>
>I think it made the review much easier than 1 really long patch.

Even squashed, the patch is quite small. Doing the same thing in every
hunk.

On contrary, the split made it more complicated for me, because when
I looked at patch 1 and the function duplication with another arg,
I did not understand what is going on. Only the last patch actually
explained it. But perhaps I'm slow.

Re: [PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[Jakub Kicinski]

On Wed, 24 Jan 2018 22:15:00 +0100, Jiri Pirko wrote:
> Wed, Jan 24, 2018 at 10:07:25PM CET, dsahern@gmail.com wrote:
> >On 1/24/18 2:04 PM, Jiri Pirko wrote:  
> >> For the record, I still think it is odd to have 6 patches just to add
> >> one arg to a function. I wonder where this unnecessary patch splits
> >> would lead to in the future.  
> >
> >I think it made the review much easier than 1 really long patch.  
> 
> Even squashed, the patch is quite small. Doing the same thing in every
> hunk.
> 
> On contrary, the split made it more complicated for me, because when
> I looked at patch 1 and the function duplication with another arg,
> I did not understand what is going on. Only the last patch actually
> explained it. But perhaps I'm slow.

Next time I'll do a better job explaining things in commit logs, sorry!

Re: [PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[Marcelo Ricardo Leitner]

On Wed, Jan 24, 2018 at 12:54:12PM -0800, Jakub Kicinski wrote:
> Hi!
> 
> This series some of Jiri's comments and the fact that today drivers
> may produce extack even if there is no skip_sw flag (meaning the
> driver failure is not really a problem), and warning messages will
> only confuse the users.

It's a fair point, but I think this is not the best solution. How will
the user then know why it failed to install in hw? Will have to
install a new rule, just with skip_sw, and hope that it fails with the
same reason?

Maybe it's better to let tc/ovs/etc only exhibit this information
under a certain log/debug level.

  Marcelo

Re: [PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[Jakub Kicinski]

On Thu, 25 Jan 2018 13:11:57 -0200, Marcelo Ricardo Leitner wrote:
> On Wed, Jan 24, 2018 at 12:54:12PM -0800, Jakub Kicinski wrote:
> > Hi!
> > 
> > This series some of Jiri's comments and the fact that today drivers
> > may produce extack even if there is no skip_sw flag (meaning the
> > driver failure is not really a problem), and warning messages will
> > only confuse the users.  
> 
> It's a fair point, but I think this is not the best solution. How will
> the user then know why it failed to install in hw? Will have to
> install a new rule, just with skip_sw, and hope that it fails with the
> same reason?
>
> Maybe it's better to let tc/ovs/etc only exhibit this information
> under a certain log/debug level.

What you say does make sense in case of classifiers which are basically
HW offload vehicles.  But for cls_bpf, which people are actually using
heavily as a software solution, I don't want any warning to be produced
just because someone happened to run the command on a Netronome
card :(  Say someone swaps an old NIC for a NFP, and runs their old
cls_bpf scripts and suddenly there are warnings they don't care about
and have no way of silencing.

I do think skip_sw will fail for the same reason, unless someone adds
extacks for IO or memory allocation problems or other transient things.

Do I understand correctly that OvS TC does not set skip_sw?  We could
add a "verbose offload" flag to the API or filter the bad extacks at
the user space level.  Although, again, my preference would be not to
filter at the user space level, because user space can't differentiate
between a probably-doesn't-matter-but-HW-offload-failed warning or legit
something-is-not-right-in-the-software-but-command-succeeded warning :S
So if there is a major use for non-forced offload failure warnings I
would lean towards a new flag.

Re: [PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[Marcelo Ricardo Leitner]

On Thu, Jan 25, 2018 at 02:57:17PM -0800, Jakub Kicinski wrote:
> On Thu, 25 Jan 2018 13:11:57 -0200, Marcelo Ricardo Leitner wrote:
> > On Wed, Jan 24, 2018 at 12:54:12PM -0800, Jakub Kicinski wrote:
> > > Hi!
> > > 
> > > This series some of Jiri's comments and the fact that today drivers
> > > may produce extack even if there is no skip_sw flag (meaning the
> > > driver failure is not really a problem), and warning messages will
> > > only confuse the users.  
> > 
> > It's a fair point, but I think this is not the best solution. How will
> > the user then know why it failed to install in hw? Will have to
> > install a new rule, just with skip_sw, and hope that it fails with the
> > same reason?
> >
> > Maybe it's better to let tc/ovs/etc only exhibit this information
> > under a certain log/debug level.
> 
> What you say does make sense in case of classifiers which are basically
> HW offload vehicles.  But for cls_bpf, which people are actually using
> heavily as a software solution, I don't want any warning to be produced
> just because someone happened to run the command on a Netronome
> card :(  Say someone swaps an old NIC for a NFP, and runs their old
> cls_bpf scripts and suddenly there are warnings they don't care about
> and have no way of silencing.

(Sorry for the delay on replying, btw. I'm still traveling.)

Makes sense. I agree that at least it shouldn't be displayed in a way
that may lead the user to think it was a big/fatal error.

> 
> I do think skip_sw will fail for the same reason, unless someone adds
> extacks for IO or memory allocation problems or other transient things.

I don't really follow this one. Fail you mean, fail to report the
actual reason? If so, ok, but that's something easily fixable I think,
especially because with skip_sw, if such an error happens, it's fatal
for the operation so the error reporting is consistent.

> 
> Do I understand correctly that OvS TC does not set skip_sw?  We could

Yes.

> add a "verbose offload" flag to the API or filter the bad extacks at
> the user space level.  Although, again, my preference would be not to
> filter at the user space level, because user space can't differentiate
> between a probably-doesn't-matter-but-HW-offload-failed warning or legit
> something-is-not-right-in-the-software-but-command-succeeded warning :S

But userspace is the original requester. It should know what the rule
is intended for and how to act upon it. For ovs, for example, it could
just log a message and move on, while tc could report "hey, ok, but
please note that the rule couldn't be offloaded".

> So if there is a major use for non-forced offload failure warnings I
> would lean towards a new flag.

I'm thinking about this, still undecided. In the end maybe a counter
somewhere could be enough and such reporting is not needed. Thinking..

  Marcelo

Re: [PATCH net-next v2 00/12] net: sched: propagate extack to cls offloads on destroy and only with skip_sw

[Jakub Kicinski]

On Sun, 28 Jan 2018 20:39:02 -0200, Marcelo Ricardo Leitner wrote:
> On Thu, Jan 25, 2018 at 02:57:17PM -0800, Jakub Kicinski wrote:
> > On Thu, 25 Jan 2018 13:11:57 -0200, Marcelo Ricardo Leitner wrote:  
> > > On Wed, Jan 24, 2018 at 12:54:12PM -0800, Jakub Kicinski wrote:  
> > > > Hi!
> > > > 
> > > > This series some of Jiri's comments and the fact that today drivers
> > > > may produce extack even if there is no skip_sw flag (meaning the
> > > > driver failure is not really a problem), and warning messages will
> > > > only confuse the users.    
> > > 
> > > It's a fair point, but I think this is not the best solution. How will
> > > the user then know why it failed to install in hw? Will have to
> > > install a new rule, just with skip_sw, and hope that it fails with the
> > > same reason?
> > >
> > > Maybe it's better to let tc/ovs/etc only exhibit this information
> > > under a certain log/debug level.  
> > 
> > What you say does make sense in case of classifiers which are basically
> > HW offload vehicles.  But for cls_bpf, which people are actually using
> > heavily as a software solution, I don't want any warning to be produced
> > just because someone happened to run the command on a Netronome
> > card :(  Say someone swaps an old NIC for a NFP, and runs their old
> > cls_bpf scripts and suddenly there are warnings they don't care about
> > and have no way of silencing.  
> 
> (Sorry for the delay on replying, btw. I'm still traveling.)
> 
> Makes sense. I agree that at least it shouldn't be displayed in a way
> that may lead the user to think it was a big/fatal error.
> 
> > I do think skip_sw will fail for the same reason, unless someone adds
> > extacks for IO or memory allocation problems or other transient things.  
> 
> I don't really follow this one. Fail you mean, fail to report the
> actual reason? If so, ok, but that's something easily fixable I think,
> especially because with skip_sw, if such an error happens, it's fatal
> for the operation so the error reporting is consistent.

I was referring to your question: "Will have to install a new rule,
just with skip_sw, and hope that it fails with the same reason?"
So yes, currently the only way to get the extack would be to retry the
command with skip_sw.

> > add a "verbose offload" flag to the API or filter the bad extacks at
> > the user space level.  Although, again, my preference would be not to
> > filter at the user space level, because user space can't differentiate
> > between a probably-doesn't-matter-but-HW-offload-failed warning or legit
> > something-is-not-right-in-the-software-but-command-succeeded warning :S  
> 
> But userspace is the original requester. It should know what the rule
> is intended for and how to act upon it. For ovs, for example, it could
> just log a message and move on, while tc could report "hey, ok, but
> please note that the rule couldn't be offloaded".
> 
> > So if there is a major use for non-forced offload failure warnings I
> > would lean towards a new flag.  
> 
> I'm thinking about this, still undecided. In the end maybe a counter
> somewhere could be enough and such reporting is not needed. Thinking..

Hm, special counters for failure reasons or just for all failures to
offload?  FWIW user space can dump the filters and if the filter is
offloaded there will be an in_hw flag Or added a few releases back.  
Let us know what your thoughts are :)