Commit 131f1ed3 authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'net-sched-propagate-extack-to-cls-offloads-on-destroy-and-only-with-skip_sw' 



Jakub Kicinski says:

====================
net: sched: propagate extack to cls offloads on destroy and only with skip_sw

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().
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 46410c2e c846adb6

include/net/pkt_cls.h

+12 −12
Original line number Diff line number Diff line
@@ -605,17 +605,6 @@ struct tc_cls_common_offload { 
	struct netlink_ext_ack *extack;

};



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)

{

	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;
@@ -694,6 +683,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,
@@ -736,7 +737,6 @@ struct tc_cls_bpf_offload { 
	struct bpf_prog *oldprog;

	const char *name;

	bool exts_integrated;

	u32 gen_flags;

};



struct tc_mqprio_qopt_offload {

include/net/sch_generic.h

+2 −1
Original line number Diff line number Diff line
@@ -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 *,

net/sched/cls_api.c

+8 −7
Original line number Diff line number Diff line
@@ -172,9 +172,10 @@ errout: 
	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 @@ replay: 
			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 @@ replay: 
		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 @@ replay: 
				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 @@ replay: 
			       RTM_NEWTFILTER, false);

	} else {

		if (tp_created)

			tcf_proto_destroy(tp);

			tcf_proto_destroy(tp, NULL);

	}



errout:

net/sched/cls_basic.c

+1 −1
Original line number Diff line number Diff line
@@ -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;

net/sched/cls_bpf.c

+13 −11
Original line number Diff line number Diff line
@@ -159,14 +159,14 @@ 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(&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;

	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);
@@ -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);

}
@@ -227,13 +228,12 @@ 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(&cls_bpf.common, tp, prog->gen_flags, NULL);

	cls_bpf.command = TC_CLSBPF_STATS;

	cls_bpf.exts = &prog->exts;

	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);

}
@@ -290,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))
@@ -309,18 +310,19 @@ 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;

}



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;



	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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE