diff mbox series

[PATCHv2,net-next,7/8] net: sched: cls: add extack support for tc_setup_cb_call

Message ID 20180117224027.24049-8-aring@mojatatu.com
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series net: sched: cls: add extack support | expand

Commit Message

Alexander Aring Jan. 17, 2018, 10:40 p.m. UTC 
This patch adds extack handling for the tc_setup_cb_call function which
is common used by TC classifier implementations.

Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 include/net/pkt_cls.h    |  3 ++-
 net/sched/cls_api.c      | 11 ++++++++---
 net/sched/cls_bpf.c      | 19 +++++++++++--------
 net/sched/cls_flower.c   | 11 ++++++-----
 net/sched/cls_matchall.c | 11 +++++++----
 net/sched/cls_u32.c      | 20 +++++++++++---------
 6 files changed, 45 insertions(+), 30 deletions(-)

Comments

Jakub Kicinski Jan. 18, 2018, 1:25 a.m. UTC | #1 
On Wed, 17 Jan 2018 17:40:26 -0500, Alexander Aring wrote:
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 9f88107c29c5..e864ad523800 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -1566,21 +1566,26 @@ static int tc_exts_setup_cb_egdev_call(struct tcf_exts *exts,
>  }
>  
>  int tc_setup_cb_call(struct tcf_block *block, struct tcf_exts *exts,
> -		     enum tc_setup_type type, void *type_data, bool err_stop)
> +		     enum tc_setup_type type, void *type_data, bool err_stop,
> +		     struct netlink_ext_ack *extack)
>  {
>  	int ok_count;
>  	int ret;
>  
>  	ret = tcf_block_cb_call(block, type, type_data, err_stop);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		NL_SET_ERR_MSG(extack, "Failed to initialize tcf block");

This has nothing to do with block init.

>  		return ret;
> +	}
>  	ok_count = ret;
>  
>  	if (!exts)
>  		return ok_count;
>  	ret = tc_exts_setup_cb_egdev_call(exts, type, type_data, err_stop);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		NL_SET_ERR_MSG(extack, "Failed to initialize tcf block extensions");

Ditto, plus this is about redirections to other devices (hence
eg[ress ]dev).  exts part is an internal detail.

>  		return ret;
> +	}
>  	ok_count += ret;
>  
>  	return ok_count;

These messages are completely off-target, and this is not a right place
to handle any of this.

Quentin's patches do the correct thing for offload, please drop this
patch from the series and let us handle the offload cases.

Thank you.
Alexander Aring Jan. 18, 2018, 3:38 p.m. UTC | #2 
Hi,

On Wed, Jan 17, 2018 at 8:25 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Wed, 17 Jan 2018 17:40:26 -0500, Alexander Aring wrote:
>> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>> index 9f88107c29c5..e864ad523800 100644
>> --- a/net/sched/cls_api.c
>> +++ b/net/sched/cls_api.c
>> @@ -1566,21 +1566,26 @@ static int tc_exts_setup_cb_egdev_call(struct tcf_exts *exts,
>>  }
>>
>>  int tc_setup_cb_call(struct tcf_block *block, struct tcf_exts *exts,
>> -                  enum tc_setup_type type, void *type_data, bool err_stop)
>> +                  enum tc_setup_type type, void *type_data, bool err_stop,
>> +                  struct netlink_ext_ack *extack)
>>  {
>>       int ok_count;
>>       int ret;
>>
>>       ret = tcf_block_cb_call(block, type, type_data, err_stop);
>> -     if (ret < 0)
>> +     if (ret < 0) {
>> +             NL_SET_ERR_MSG(extack, "Failed to initialize tcf block");
>
> This has nothing to do with block init.
>
>>               return ret;
>> +     }
>>       ok_count = ret;
>>
>>       if (!exts)
>>               return ok_count;
>>       ret = tc_exts_setup_cb_egdev_call(exts, type, type_data, err_stop);
>> -     if (ret < 0)
>> +     if (ret < 0) {
>> +             NL_SET_ERR_MSG(extack, "Failed to initialize tcf block extensions");
>
> Ditto, plus this is about redirections to other devices (hence
> eg[ress ]dev).  exts part is an internal detail.
>

Ok, I am going to remove this patch.

- Alex
diff mbox series

Patch

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 2e4b8e436d25..7457232ae59f 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -585,7 +585,8 @@  tcf_match_indev(struct sk_buff *skb, int ifindex)
 #endif /* CONFIG_NET_CLS_IND */
 
 int tc_setup_cb_call(struct tcf_block *block, struct tcf_exts *exts,
-		     enum tc_setup_type type, void *type_data, bool err_stop);
+		     enum tc_setup_type type, void *type_data, bool err_stop,
+		     struct netlink_ext_ack *extack);
 
 enum tc_block_command {
 	TC_BLOCK_BIND,
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 9f88107c29c5..e864ad523800 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1566,21 +1566,26 @@  static int tc_exts_setup_cb_egdev_call(struct tcf_exts *exts,
 }
 
 int tc_setup_cb_call(struct tcf_block *block, struct tcf_exts *exts,
-		     enum tc_setup_type type, void *type_data, bool err_stop)
+		     enum tc_setup_type type, void *type_data, bool err_stop,
+		     struct netlink_ext_ack *extack)
 {
 	int ok_count;
 	int ret;
 
 	ret = tcf_block_cb_call(block, type, type_data, err_stop);
-	if (ret < 0)
+	if (ret < 0) {
+		NL_SET_ERR_MSG(extack, "Failed to initialize tcf block");
 		return ret;
+	}
 	ok_count = ret;
 
 	if (!exts)
 		return ok_count;
 	ret = tc_exts_setup_cb_egdev_call(exts, type, type_data, err_stop);
-	if (ret < 0)
+	if (ret < 0) {
+		NL_SET_ERR_MSG(extack, "Failed to initialize tcf block extensions");
 		return ret;
+	}
 	ok_count += ret;
 
 	return ok_count;
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index fc024fc3ec2f..566befb6ac71 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -147,7 +147,8 @@  static bool cls_bpf_is_ebpf(const struct cls_bpf_prog *prog)
 }
 
 static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
-			       struct cls_bpf_prog *oldprog)
+			       struct cls_bpf_prog *oldprog,
+			       struct netlink_ext_ack *extack)
 {
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_bpf_offload cls_bpf = {};
@@ -170,10 +171,11 @@  static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	if (oldprog)
 		tcf_block_offload_dec(block, &oldprog->gen_flags);
 
-	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSBPF, &cls_bpf, skip_sw);
+	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSBPF, &cls_bpf, skip_sw,
+			       extack);
 	if (prog) {
 		if (err < 0) {
-			cls_bpf_offload_cmd(tp, oldprog, prog);
+			cls_bpf_offload_cmd(tp, oldprog, prog, NULL);
 			return err;
 		} else if (err > 0) {
 			tcf_block_offload_inc(block, &prog->gen_flags);
@@ -187,7 +189,8 @@  static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 }
 
 static int cls_bpf_offload(struct tcf_proto *tp, struct cls_bpf_prog *prog,
-			   struct cls_bpf_prog *oldprog)
+			   struct cls_bpf_prog *oldprog,
+			   struct netlink_ext_ack *extack)
 {
 	if (prog && oldprog && prog->gen_flags != oldprog->gen_flags)
 		return -EINVAL;
@@ -199,7 +202,7 @@  static int cls_bpf_offload(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	if (!prog && !oldprog)
 		return 0;
 
-	return cls_bpf_offload_cmd(tp, prog, oldprog);
+	return cls_bpf_offload_cmd(tp, prog, oldprog, extack);
 }
 
 static void cls_bpf_stop_offload(struct tcf_proto *tp,
@@ -207,7 +210,7 @@  static void cls_bpf_stop_offload(struct tcf_proto *tp,
 {
 	int err;
 
-	err = cls_bpf_offload_cmd(tp, NULL, prog);
+	err = cls_bpf_offload_cmd(tp, NULL, prog, NULL);
 	if (err)
 		pr_err("Stopping hardware offload failed: %d\n", err);
 }
@@ -226,7 +229,7 @@  static void cls_bpf_offload_update_stats(struct tcf_proto *tp,
 	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);
+	tc_setup_cb_call(block, NULL, TC_SETUP_CLSBPF, &cls_bpf, false, NULL);
 }
 
 static int cls_bpf_init(struct tcf_proto *tp)
@@ -507,7 +510,7 @@  static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
 	if (ret < 0)
 		goto errout_idr;
 
-	ret = cls_bpf_offload(tp, prog, oldprog);
+	ret = cls_bpf_offload(tp, prog, oldprog, extack);
 	if (ret)
 		goto errout_parms;
 
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index c6ac4a612c4a..f160dbccf8a7 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -228,14 +228,15 @@  static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f)
 	cls_flower.cookie = (unsigned long) f;
 
 	tc_setup_cb_call(block, &f->exts, TC_SETUP_CLSFLOWER,
-			 &cls_flower, false);
+			 &cls_flower, false, NULL);
 	tcf_block_offload_dec(block, &f->flags);
 }
 
 static int fl_hw_replace_filter(struct tcf_proto *tp,
 				struct flow_dissector *dissector,
 				struct fl_flow_key *mask,
-				struct cls_fl_filter *f)
+				struct cls_fl_filter *f,
+				struct netlink_ext_ack *extack)
 {
 	struct tc_cls_flower_offload cls_flower = {};
 	struct tcf_block *block = tp->chain->block;
@@ -252,7 +253,7 @@  static int fl_hw_replace_filter(struct tcf_proto *tp,
 	cls_flower.classid = f->res.classid;
 
 	err = tc_setup_cb_call(block, &f->exts, TC_SETUP_CLSFLOWER,
-			       &cls_flower, skip_sw);
+			       &cls_flower, skip_sw, extack);
 	if (err < 0) {
 		fl_hw_destroy_filter(tp, f);
 		return err;
@@ -278,7 +279,7 @@  static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f)
 	cls_flower.classid = f->res.classid;
 
 	tc_setup_cb_call(block, &f->exts, TC_SETUP_CLSFLOWER,
-			 &cls_flower, false);
+			 &cls_flower, false, NULL);
 }
 
 static void __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f)
@@ -943,7 +944,7 @@  static int fl_change(struct net *net, struct sk_buff *in_skb,
 		err = fl_hw_replace_filter(tp,
 					   &head->dissector,
 					   &mask.key,
-					   fnew);
+					   fnew, extack);
 		if (err)
 			goto errout_idr;
 	}
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index f67d3d7fcf40..03f5cd6b7cbd 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -80,13 +80,15 @@  static void mall_destroy_hw_filter(struct tcf_proto *tp,
 	cls_mall.command = TC_CLSMATCHALL_DESTROY;
 	cls_mall.cookie = cookie;
 
-	tc_setup_cb_call(block, NULL, TC_SETUP_CLSMATCHALL, &cls_mall, false);
+	tc_setup_cb_call(block, NULL, TC_SETUP_CLSMATCHALL, &cls_mall, false,
+			 NULL);
 	tcf_block_offload_dec(block, &head->flags);
 }
 
 static int mall_replace_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;
@@ -99,7 +101,7 @@  static int mall_replace_hw_filter(struct tcf_proto *tp,
 	cls_mall.cookie = cookie;
 
 	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSMATCHALL,
-			       &cls_mall, skip_sw);
+			       &cls_mall, skip_sw, extack);
 	if (err < 0) {
 		mall_destroy_hw_filter(tp, head, cookie);
 		return err;
@@ -205,7 +207,8 @@  static int mall_change(struct net *net, struct sk_buff *in_skb,
 		goto err_set_parms;
 
 	if (!tc_skip_hw(new->flags)) {
-		err = mall_replace_hw_filter(tp, new, (unsigned long)new);
+		err = mall_replace_hw_filter(tp, new, (unsigned long)new,
+					     extack);
 		if (err)
 			goto err_replace_hw_filter;
 	}
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index e8963ed35899..8840baa1b9b4 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -497,11 +497,11 @@  static void u32_clear_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h)
 	cls_u32.hnode.handle = h->handle;
 	cls_u32.hnode.prio = h->prio;
 
-	tc_setup_cb_call(block, NULL, TC_SETUP_CLSU32, &cls_u32, false);
+	tc_setup_cb_call(block, NULL, TC_SETUP_CLSU32, &cls_u32, false, NULL);
 }
 
 static int u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h,
-				u32 flags)
+				u32 flags, struct netlink_ext_ack *extack)
 {
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
@@ -515,7 +515,8 @@  static int u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h,
 	cls_u32.hnode.handle = h->handle;
 	cls_u32.hnode.prio = h->prio;
 
-	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSU32, &cls_u32, skip_sw);
+	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSU32, &cls_u32, skip_sw,
+			       extack);
 	if (err < 0) {
 		u32_clear_hw_hnode(tp, h);
 		return err;
@@ -538,12 +539,12 @@  static void u32_remove_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n)
 	cls_u32.command = TC_CLSU32_DELETE_KNODE;
 	cls_u32.knode.handle = n->handle;
 
-	tc_setup_cb_call(block, NULL, TC_SETUP_CLSU32, &cls_u32, false);
+	tc_setup_cb_call(block, NULL, TC_SETUP_CLSU32, &cls_u32, false, NULL);
 	tcf_block_offload_dec(block, &n->flags);
 }
 
 static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
-				u32 flags)
+				u32 flags, struct netlink_ext_ack *extack)
 {
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
@@ -566,7 +567,8 @@  static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 	if (n->ht_down)
 		cls_u32.knode.link_handle = n->ht_down->handle;
 
-	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSU32, &cls_u32, skip_sw);
+	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSU32, &cls_u32, skip_sw,
+			       extack);
 	if (err < 0) {
 		u32_remove_hw_knode(tp, n);
 		return err;
@@ -946,7 +948,7 @@  static int u32_change(struct net *net, struct sk_buff *in_skb,
 			return err;
 		}
 
-		err = u32_replace_hw_knode(tp, new, flags);
+		err = u32_replace_hw_knode(tp, new, flags, extack);
 		if (err) {
 			u32_destroy_key(tp, new, false);
 			return err;
@@ -993,7 +995,7 @@  static int u32_change(struct net *net, struct sk_buff *in_skb,
 		ht->prio = tp->prio;
 		idr_init(&ht->handle_idr);
 
-		err = u32_replace_hw_hnode(tp, ht, flags);
+		err = u32_replace_hw_hnode(tp, ht, flags, extack);
 		if (err) {
 			idr_remove_ext(&tp_c->handle_idr, handle);
 			kfree(ht);
@@ -1092,7 +1094,7 @@  static int u32_change(struct net *net, struct sk_buff *in_skb,
 		struct tc_u_knode __rcu **ins;
 		struct tc_u_knode *pins;
 
-		err = u32_replace_hw_knode(tp, n, flags);
+		err = u32_replace_hw_knode(tp, n, flags, extack);
 		if (err)
 			goto errhw;