From f7ebdff7575c3a5c099c8b1d661b0a65f4c6f10f Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:28:56 +0200 Subject: [PATCH 01/20] net: sched: sch_atm: use Qdisc_class_common structure Even if it is only for classid now, use this common struct a be aligned with the rest of the classful qdiscs. Signed-off-by: Jiri Pirko Acked-by: Jamal Hadi Salim Signed-off-by: David S. Miller --- net/sched/sch_atm.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c index 572fe2584e48..0af4b1c6f674 100644 --- a/net/sched/sch_atm.c +++ b/net/sched/sch_atm.c @@ -41,6 +41,7 @@ #define VCC2FLOW(vcc) ((struct atm_flow_data *) ((vcc)->user_back)) struct atm_flow_data { + struct Qdisc_class_common common; struct Qdisc *q; /* FIFO, TBF, etc. */ struct tcf_proto __rcu *filter_list; struct tcf_block *block; @@ -49,7 +50,6 @@ struct atm_flow_data { struct sk_buff *skb); /* chaining */ struct atm_qdisc_data *parent; /* parent qdisc */ struct socket *sock; /* for closing */ - u32 classid; /* x:y type ID */ int ref; /* reference count */ struct gnet_stats_basic_packed bstats; struct gnet_stats_queue qstats; @@ -75,7 +75,7 @@ static inline struct atm_flow_data *lookup_flow(struct Qdisc *sch, u32 classid) struct atm_flow_data *flow; list_for_each_entry(flow, &p->flows, list) { - if (flow->classid == classid) + if (flow->common.classid == classid) return flow; } return NULL; @@ -293,7 +293,7 @@ static int atm_tc_change(struct Qdisc *sch, u32 classid, u32 parent, flow->old_pop = flow->vcc->pop; flow->parent = p; flow->vcc->pop = sch_atm_pop; - flow->classid = classid; + flow->common.classid = classid; flow->ref = 1; flow->excess = excess; list_add(&flow->list, &p->link.list); @@ -549,7 +549,7 @@ static int atm_tc_init(struct Qdisc *sch, struct nlattr *opt) p->link.vcc = NULL; p->link.sock = NULL; - p->link.classid = sch->handle; + p->link.common.classid = sch->handle; p->link.ref = 1; tasklet_init(&p->task, sch_atm_dequeue, (unsigned long)sch); return 0; @@ -594,7 +594,7 @@ static int atm_tc_dump_class(struct Qdisc *sch, unsigned long cl, sch, p, flow, skb, tcm); if (list_empty(&flow->list)) return -EINVAL; - tcm->tcm_handle = flow->classid; + tcm->tcm_handle = flow->common.classid; tcm->tcm_info = flow->q->handle; nest = nla_nest_start(skb, TCA_OPTIONS); @@ -619,7 +619,7 @@ static int atm_tc_dump_class(struct Qdisc *sch, unsigned long cl, goto nla_put_failure; } if (flow->excess) { - if (nla_put_u32(skb, TCA_ATM_EXCESS, flow->classid)) + if (nla_put_u32(skb, TCA_ATM_EXCESS, flow->common.classid)) goto nla_put_failure; } else { if (nla_put_u32(skb, TCA_ATM_EXCESS, 0)) From 4ebc1e3cfcd8778e2150bdb799b19e85348b8efa Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:28:57 +0200 Subject: [PATCH 02/20] net: sched: remove unneeded tcf_em_tree_change Since tcf_em_tree_validate could be always called on a newly created filter, there is no need for this change function. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- include/net/pkt_cls.h | 21 --------------------- net/sched/cls_basic.c | 4 +--- net/sched/cls_cgroup.c | 4 +--- net/sched/cls_flow.c | 12 +++++------- 4 files changed, 7 insertions(+), 34 deletions(-) diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 537d0a0ad4c4..f4462ec8b2f4 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -332,26 +332,6 @@ int tcf_em_tree_dump(struct sk_buff *, struct tcf_ematch_tree *, int); int __tcf_em_tree_match(struct sk_buff *, struct tcf_ematch_tree *, struct tcf_pkt_info *); -/** - * tcf_em_tree_change - replace ematch tree of a running classifier - * - * @tp: classifier kind handle - * @dst: destination ematch tree variable - * @src: source ematch tree (temporary tree from tcf_em_tree_validate) - * - * This functions replaces the ematch tree in @dst with the ematch - * tree in @src. The classifier in charge of the ematch tree may be - * running. - */ -static inline void tcf_em_tree_change(struct tcf_proto *tp, - struct tcf_ematch_tree *dst, - struct tcf_ematch_tree *src) -{ - tcf_tree_lock(tp); - memcpy(dst, src, sizeof(*dst)); - tcf_tree_unlock(tp); -} - /** * tcf_em_tree_match - evaulate an ematch tree * @@ -386,7 +366,6 @@ struct tcf_ematch_tree { #define tcf_em_tree_validate(tp, tb, t) ((void)(t), 0) #define tcf_em_tree_destroy(t) do { (void)(t); } while(0) #define tcf_em_tree_dump(skb, t, tlv) (0) -#define tcf_em_tree_change(tp, dst, src) do { } while(0) #define tcf_em_tree_match(skb, t, info) ((void)(info), 1) #endif /* CONFIG_NET_EMATCH */ diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c index c4fd63a068f9..979cd2683b46 100644 --- a/net/sched/cls_basic.c +++ b/net/sched/cls_basic.c @@ -130,7 +130,6 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp, { int err; struct tcf_exts e; - struct tcf_ematch_tree t; err = tcf_exts_init(&e, TCA_BASIC_ACT, TCA_BASIC_POLICE); if (err < 0) @@ -139,7 +138,7 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp, if (err < 0) goto errout; - err = tcf_em_tree_validate(tp, tb[TCA_BASIC_EMATCHES], &t); + err = tcf_em_tree_validate(tp, tb[TCA_BASIC_EMATCHES], &f->ematches); if (err < 0) goto errout; @@ -149,7 +148,6 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp, } tcf_exts_change(tp, &f->exts, &e); - tcf_em_tree_change(tp, &f->ematches, &t); f->tp = tp; return 0; diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c index 12ce547eea04..ce7d38beab95 100644 --- a/net/sched/cls_cgroup.c +++ b/net/sched/cls_cgroup.c @@ -76,7 +76,6 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb, struct nlattr *tb[TCA_CGROUP_MAX + 1]; struct cls_cgroup_head *head = rtnl_dereference(tp->root); struct cls_cgroup_head *new; - struct tcf_ematch_tree t; struct tcf_exts e; int err; @@ -112,14 +111,13 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb, goto errout; } - err = tcf_em_tree_validate(tp, tb[TCA_CGROUP_EMATCHES], &t); + err = tcf_em_tree_validate(tp, tb[TCA_CGROUP_EMATCHES], &new->ematches); if (err < 0) { tcf_exts_destroy(&e); goto errout; } tcf_exts_change(tp, &new->exts, &e); - tcf_em_tree_change(tp, &new->ematches, &t); rcu_assign_pointer(tp->root, new); if (head) diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c index 3065752b9cda..71fd1af01726 100644 --- a/net/sched/cls_flow.c +++ b/net/sched/cls_flow.c @@ -389,7 +389,6 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, struct nlattr *opt = tca[TCA_OPTIONS]; struct nlattr *tb[TCA_FLOW_MAX + 1]; struct tcf_exts e; - struct tcf_ematch_tree t; unsigned int nkeys = 0; unsigned int perturb_period = 0; u32 baseclass = 0; @@ -432,13 +431,13 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, if (err < 0) goto err1; - err = tcf_em_tree_validate(tp, tb[TCA_FLOW_EMATCHES], &t); - if (err < 0) - goto err1; - err = -ENOBUFS; fnew = kzalloc(sizeof(*fnew), GFP_KERNEL); if (!fnew) + goto err1; + + err = tcf_em_tree_validate(tp, tb[TCA_FLOW_EMATCHES], &fnew->ematches); + if (err < 0) goto err2; err = tcf_exts_init(&fnew->exts, TCA_FLOW_ACT, TCA_FLOW_POLICE); @@ -512,7 +511,6 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, (unsigned long)fnew); tcf_exts_change(tp, &fnew->exts, &e); - tcf_em_tree_change(tp, &fnew->ematches, &t); netif_keep_dst(qdisc_dev(tp->q)); @@ -554,8 +552,8 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, err3: tcf_exts_destroy(&fnew->exts); + tcf_em_tree_destroy(&fnew->ematches); err2: - tcf_em_tree_destroy(&t); kfree(fnew); err1: tcf_exts_destroy(&e); From 3bcc0cec818fa969fe555b44443347211ed787a3 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:28:58 +0200 Subject: [PATCH 03/20] net: sched: change names of action number helpers to be aligned with the rest The rest of the helpers are named tcf_exts_*, so change the name of the action number helpers to be aligned. While at it, change to inline functions. Signed-off-by: Jiri Pirko Acked-by: Jamal Hadi Salim Signed-off-by: David S. Miller --- .../net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- .../net/ethernet/mellanox/mlx5/core/en_tc.c | 4 +-- .../net/ethernet/mellanox/mlxsw/spectrum.c | 2 +- .../ethernet/mellanox/mlxsw/spectrum_flower.c | 2 +- .../net/ethernet/netronome/nfp/bpf/offload.c | 4 +-- include/net/pkt_cls.h | 36 ++++++++++++++----- net/dsa/slave.c | 2 +- net/sched/cls_api.c | 2 +- 9 files changed, 37 insertions(+), 19 deletions(-) diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c index ef06ce8247ab..6f734c52ef25 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c @@ -96,7 +96,7 @@ static int fill_action_fields(struct adapter *adap, LIST_HEAD(actions); exts = cls->knode.exts; - if (tc_no_actions(exts)) + if (!tcf_exts_has_actions(exts)) return -EINVAL; tcf_exts_to_list(exts, &actions); diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 96606e3eb965..091fcc7e6e43 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -8953,7 +8953,7 @@ static int parse_tc_actions(struct ixgbe_adapter *adapter, LIST_HEAD(actions); int err; - if (tc_no_actions(exts)) + if (!tcf_exts_has_actions(exts)) return -EINVAL; tcf_exts_to_list(exts, &actions); diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c index 3c536f560dd2..78f50d9f621d 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c @@ -1326,7 +1326,7 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv, struct tcf_exts *exts, LIST_HEAD(actions); int err; - if (tc_no_actions(exts)) + if (!tcf_exts_has_actions(exts)) return -EINVAL; attr->flow_tag = MLX5_FS_DEFAULT_FLOW_TAG; @@ -1839,7 +1839,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts, bool encap = false; int err = 0; - if (tc_no_actions(exts)) + if (!tcf_exts_has_actions(exts)) return -EINVAL; memset(attr, 0, sizeof(*attr)); diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c index 88b668ba0d8a..66d511d45c25 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c @@ -1626,7 +1626,7 @@ static int mlxsw_sp_port_add_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port, LIST_HEAD(actions); int err; - if (!tc_single_action(cls->exts)) { + if (!tcf_exts_has_one_action(cls->exts)) { netdev_err(mlxsw_sp_port->dev, "only singular actions are supported\n"); return -EOPNOTSUPP; } diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c index 400ad4081660..9be48d2e43ca 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c @@ -53,7 +53,7 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp, LIST_HEAD(actions); int err; - if (tc_no_actions(exts)) + if (!tcf_exts_has_actions(exts)) return 0; /* Count action is inserted first */ diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c index 78d80a364edb..a88bb5bc0082 100644 --- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c +++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c @@ -115,14 +115,14 @@ nfp_net_bpf_get_act(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf) /* TC direct action */ if (cls_bpf->exts_integrated) { - if (tc_no_actions(cls_bpf->exts)) + if (!tcf_exts_has_actions(cls_bpf->exts)) return NN_ACT_DIRECT; return -EOPNOTSUPP; } /* TC legacy mode */ - if (!tc_single_action(cls_bpf->exts)) + if (!tcf_exts_has_one_action(cls_bpf->exts)) return -EOPNOTSUPP; tcf_exts_to_list(cls_bpf->exts, &actions); diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index f4462ec8b2f4..7f2563636df0 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -199,17 +199,35 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts, return 0; } +/** + * tcf_exts_has_actions - check if at least one action is present + * @exts: tc filter extensions handle + * + * Returns true if at least one action is present. + */ +static inline bool tcf_exts_has_actions(struct tcf_exts *exts) +{ #ifdef CONFIG_NET_CLS_ACT + return exts->nr_actions; +#else + return false; +#endif +} -#define tc_no_actions(_exts) ((_exts)->nr_actions == 0) -#define tc_single_action(_exts) ((_exts)->nr_actions == 1) - -#else /* CONFIG_NET_CLS_ACT */ - -#define tc_no_actions(_exts) true -#define tc_single_action(_exts) false - -#endif /* CONFIG_NET_CLS_ACT */ +/** + * tcf_exts_has_one_action - check if exactly one action is present + * @exts: tc filter extensions handle + * + * Returns true if exactly one action is present. + */ +static inline bool tcf_exts_has_one_action(struct tcf_exts *exts) +{ +#ifdef CONFIG_NET_CLS_ACT + return exts->nr_actions == 1; +#else + return false; +#endif +} int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, struct nlattr *rate_tlv, diff --git a/net/dsa/slave.c b/net/dsa/slave.c index e196562035b1..83252e8426d7 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -779,7 +779,7 @@ static int dsa_slave_add_cls_matchall(struct net_device *dev, if (!ds->ops->port_mirror_add) return err; - if (!tc_single_action(cls->exts)) + if (!tcf_exts_has_one_action(cls->exts)) return err; tcf_exts_to_list(cls->exts, &actions); diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 39da0c5801c9..287ae6cbf73b 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -972,7 +972,7 @@ int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts, const struct tc_action *a; LIST_HEAD(actions); - if (tc_no_actions(exts)) + if (!tcf_exts_has_actions(exts)) return -EINVAL; tcf_exts_to_list(exts, &actions); From af69afc551eb9f9b1f2cc3295e2dfcdaa1dc948d Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:28:59 +0200 Subject: [PATCH 04/20] net: sched: use tcf_exts_has_actions in tcf_exts_exec Use the tcf_exts_has_actions helper instead or directly testing exts->nr_actions in tcf_exts_exec. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- include/net/pkt_cls.h | 46 +++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 7f2563636df0..322a2823cc6a 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -176,29 +176,6 @@ tcf_exts_stats_update(const struct tcf_exts *exts, #endif } -/** - * tcf_exts_exec - execute tc filter extensions - * @skb: socket buffer - * @exts: tc filter extensions handle - * @res: desired result - * - * Executes all configured extensions. Returns 0 on a normal execution, - * a negative number if the filter must be considered unmatched or - * a positive action code (TC_ACT_*) which must be returned to the - * underlying layer. - */ -static inline int -tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts, - struct tcf_result *res) -{ -#ifdef CONFIG_NET_CLS_ACT - if (exts->nr_actions) - return tcf_action_exec(skb, exts->actions, exts->nr_actions, - res); -#endif - return 0; -} - /** * tcf_exts_has_actions - check if at least one action is present * @exts: tc filter extensions handle @@ -229,6 +206,29 @@ static inline bool tcf_exts_has_one_action(struct tcf_exts *exts) #endif } +/** + * tcf_exts_exec - execute tc filter extensions + * @skb: socket buffer + * @exts: tc filter extensions handle + * @res: desired result + * + * Executes all configured extensions. Returns 0 on a normal execution, + * a negative number if the filter must be considered unmatched or + * a positive action code (TC_ACT_*) which must be returned to the + * underlying layer. + */ +static inline int +tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts, + struct tcf_result *res) +{ +#ifdef CONFIG_NET_CLS_ACT + if (tcf_exts_has_actions(exts)) + return tcf_action_exec(skb, exts->actions, exts->nr_actions, + res); +#endif + return 0; +} + int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, struct nlattr *rate_tlv, struct tcf_exts *exts, bool ovr); From 6fc6d06e5371507e68c6904a3423622b0e465b64 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:00 +0200 Subject: [PATCH 05/20] net: sched: remove redundant helpers tcf_exts_is_predicative and tcf_exts_is_available These two helpers are doing the same as tcf_exts_has_actions, so remove them and use tcf_exts_has_actions instead. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- include/net/pkt_cls.h | 30 ------------------------------ net/sched/cls_fw.c | 2 +- net/sched/cls_route.c | 2 +- net/sched/cls_tcindex.c | 2 +- 4 files changed, 3 insertions(+), 33 deletions(-) diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 322a2823cc6a..817badf733b5 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -113,36 +113,6 @@ static inline int tcf_exts_init(struct tcf_exts *exts, int action, int police) return 0; } -/** - * tcf_exts_is_predicative - check if a predicative extension is present - * @exts: tc filter extensions handle - * - * Returns 1 if a predicative extension is present, i.e. an extension which - * might cause further actions and thus overrule the regular tcf_result. - */ -static inline int -tcf_exts_is_predicative(struct tcf_exts *exts) -{ -#ifdef CONFIG_NET_CLS_ACT - return exts->nr_actions; -#else - return 0; -#endif -} - -/** - * tcf_exts_is_available - check if at least one extension is present - * @exts: tc filter extensions handle - * - * Returns 1 if at least one extension is present. - */ -static inline int -tcf_exts_is_available(struct tcf_exts *exts) -{ - /* All non-predicative extensions must be added here. */ - return tcf_exts_is_predicative(exts); -} - static inline void tcf_exts_to_list(const struct tcf_exts *exts, struct list_head *actions) { diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c index d3885362e017..a53fa75dfc7b 100644 --- a/net/sched/cls_fw.c +++ b/net/sched/cls_fw.c @@ -387,7 +387,7 @@ static int fw_dump(struct net *net, struct tcf_proto *tp, unsigned long fh, t->tcm_handle = f->id; - if (!f->res.classid && !tcf_exts_is_available(&f->exts)) + if (!f->res.classid && !tcf_exts_has_actions(&f->exts)) return skb->len; nest = nla_nest_start(skb, TCA_OPTIONS); diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c index d63d5502ee02..26f863634862 100644 --- a/net/sched/cls_route.c +++ b/net/sched/cls_route.c @@ -113,7 +113,7 @@ static inline int route4_hash_wild(void) #define ROUTE4_APPLY_RESULT() \ { \ *res = f->res; \ - if (tcf_exts_is_available(&f->exts)) { \ + if (tcf_exts_has_actions(&f->exts)) { \ int r = tcf_exts_exec(skb, &f->exts, res); \ if (r < 0) { \ dont_cache = 1; \ diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c index 8a8a58357c39..66924d147e97 100644 --- a/net/sched/cls_tcindex.c +++ b/net/sched/cls_tcindex.c @@ -52,7 +52,7 @@ struct tcindex_data { static inline int tcindex_filter_is_set(struct tcindex_filter_result *r) { - return tcf_exts_is_predicative(&r->exts) || r->res.classid; + return tcf_exts_has_actions(&r->exts) || r->res.classid; } static struct tcindex_filter_result *tcindex_lookup(struct tcindex_data *p, From af089e701adfb5898fee00a56ea4bb421edc308d Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:01 +0200 Subject: [PATCH 06/20] net: sched: fix return value of tcf_exts_exec Return the defined TC_ACT_OK instead of 0. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- include/net/pkt_cls.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 817badf733b5..61ce521688b2 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -182,7 +182,7 @@ static inline bool tcf_exts_has_one_action(struct tcf_exts *exts) * @exts: tc filter extensions handle * @res: desired result * - * Executes all configured extensions. Returns 0 on a normal execution, + * Executes all configured extensions. Returns TC_ACT_OK on a normal execution, * a negative number if the filter must be considered unmatched or * a positive action code (TC_ACT_*) which must be returned to the * underlying layer. @@ -196,7 +196,7 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts, return tcf_action_exec(skb, exts->actions, exts->nr_actions, res); #endif - return 0; + return TC_ACT_OK; } int tcf_exts_validate(struct net *net, struct tcf_proto *tp, From ec1a9cca0e13391167567964fd04e61a39d6a4ae Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:02 +0200 Subject: [PATCH 07/20] net: sched: remove check for number of actions in tcf_exts_exec Leave it to tcf_action_exec to return TC_ACT_OK in case there is no action present. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- include/net/pkt_cls.h | 4 +--- net/sched/act_api.c | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 61ce521688b2..b8959c9a190d 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -192,9 +192,7 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts, struct tcf_result *res) { #ifdef CONFIG_NET_CLS_ACT - if (tcf_exts_has_actions(exts)) - return tcf_action_exec(skb, exts->actions, exts->nr_actions, - res); + return tcf_action_exec(skb, exts->actions, exts->nr_actions, res); #endif return TC_ACT_OK; } diff --git a/net/sched/act_api.c b/net/sched/act_api.c index f19b118df414..a2915d958279 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -473,9 +473,10 @@ static struct tc_action_ops *tc_lookup_action(struct nlattr *kind) int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions, int nr_actions, struct tcf_result *res) { - int ret = -1, i; u32 jmp_prgcnt = 0; u32 jmp_ttl = TCA_ACT_MAX_PRIO; /*matches actions per filter */ + int i; + int ret = TC_ACT_OK; if (skb_skip_tc_classify(skb)) return TC_ACT_OK; From 978dfd8d14a503d20be1f1e2ae328c3eac675a2d Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:03 +0200 Subject: [PATCH 08/20] net: sched: use tcf_exts_has_actions instead of exts->nr_actions For check in tcf_exts_dump use tcf_exts_has_actions helper instead of exts->nr_actions for checking if there are any actions present. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 287ae6cbf73b..735d556a5283 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -915,7 +915,7 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts) #ifdef CONFIG_NET_CLS_ACT struct nlattr *nest; - if (exts->action && exts->nr_actions) { + if (exts->action && tcf_exts_has_actions(exts)) { /* * again for backward compatible mode - we want * to work with both old and new modes of entering From 6a725c481df36b1ad471ea788a5bc64c25bf7af8 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:04 +0200 Subject: [PATCH 09/20] net: sched: cls_bpf: rename cls_bpf_modify_existing function The name cls_bpf_modify_existing is highly misleading, as it indeed does not modify anything existing. It does not modify at all. Signed-off-by: Jiri Pirko Acked-by: Daniel Borkmann Signed-off-by: David S. Miller --- net/sched/cls_bpf.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c index f57bd531ba98..e9ab8374a877 100644 --- a/net/sched/cls_bpf.c +++ b/net/sched/cls_bpf.c @@ -382,10 +382,9 @@ static int cls_bpf_prog_from_efd(struct nlattr **tb, struct cls_bpf_prog *prog, return 0; } -static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp, - struct cls_bpf_prog *prog, - unsigned long base, struct nlattr **tb, - struct nlattr *est, bool ovr) +static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp, + struct cls_bpf_prog *prog, unsigned long base, + struct nlattr **tb, struct nlattr *est, bool ovr) { bool is_bpf, is_ebpf, have_exts = false; struct tcf_exts exts; @@ -508,8 +507,7 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb, goto errout; } - ret = cls_bpf_modify_existing(net, tp, prog, base, tb, tca[TCA_RATE], - ovr); + ret = cls_bpf_set_parms(net, tp, prog, base, tb, tca[TCA_RATE], ovr); if (ret < 0) goto errout; From 1e5003af37352f37ba008f99014cc47c684ae808 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:05 +0200 Subject: [PATCH 10/20] net: sched: cls_fw: rename fw_change_attrs function Since the function name is misleading since it is not changing anything, name it similarly to other cls. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_fw.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c index a53fa75dfc7b..358bf34f0dd0 100644 --- a/net/sched/cls_fw.c +++ b/net/sched/cls_fw.c @@ -190,10 +190,9 @@ static const struct nla_policy fw_policy[TCA_FW_MAX + 1] = { [TCA_FW_MASK] = { .type = NLA_U32 }, }; -static int -fw_change_attrs(struct net *net, struct tcf_proto *tp, struct fw_filter *f, - struct nlattr **tb, struct nlattr **tca, unsigned long base, - bool ovr) +static int fw_set_parms(struct net *net, struct tcf_proto *tp, + struct fw_filter *f, struct nlattr **tb, + struct nlattr **tca, unsigned long base, bool ovr) { struct fw_head *head = rtnl_dereference(tp->root); struct tcf_exts e; @@ -282,7 +281,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb, return err; } - err = fw_change_attrs(net, tp, fnew, tb, tca, base, ovr); + err = fw_set_parms(net, tp, fnew, tb, tca, base, ovr); if (err < 0) { tcf_exts_destroy(&fnew->exts); kfree(fnew); @@ -330,7 +329,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb, f->id = handle; f->tp = tp; - err = fw_change_attrs(net, tp, f, tb, tca, base, ovr); + err = fw_set_parms(net, tp, f, tb, tca, base, ovr); if (err < 0) goto errout; From 455075292b0d58f708c1afe7011a8b9223b2c9c8 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:06 +0200 Subject: [PATCH 11/20] net: sched: cls_flower: no need to call tcf_exts_change for newly allocated struct As the f struct was allocated right before fl_set_parms call, no need to use tcf_exts_change to do atomic change, and we can just fill-up the unused exts struct directly by tcf_exts_validate. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_flower.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index 7832eb93379b..7ab524fc43f9 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -852,15 +852,11 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, struct nlattr **tb, struct nlattr *est, bool ovr) { - struct tcf_exts e; int err; - err = tcf_exts_init(&e, TCA_FLOWER_ACT, 0); + err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr); if (err < 0) return err; - err = tcf_exts_validate(net, tp, tb, est, &e, ovr); - if (err < 0) - goto errout; if (tb[TCA_FLOWER_CLASSID]) { f->res.classid = nla_get_u32(tb[TCA_FLOWER_CLASSID]); @@ -869,17 +865,12 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp, err = fl_set_key(net, tb, &f->key, &mask->key); if (err) - goto errout; + return err; fl_mask_update_range(mask); fl_set_masked_key(&f->mkey, &f->key, mask); - tcf_exts_change(tp, &f->exts, &e); - return 0; -errout: - tcf_exts_destroy(&e); - return err; } static u32 fl_grab_new_handle(struct tcf_proto *tp, From 94611bff6e1e57c4a74a9242f3fe749aa4a31678 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:07 +0200 Subject: [PATCH 12/20] net: sched: cls_fw: no need to call tcf_exts_change for newly allocated struct As the f struct was allocated right before fw_set_parms call, no need to use tcf_exts_change to do atomic change, and we can just fill-up the unused exts struct directly by tcf_exts_validate. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_fw.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c index 358bf34f0dd0..11f178f1b2be 100644 --- a/net/sched/cls_fw.c +++ b/net/sched/cls_fw.c @@ -195,16 +195,12 @@ static int fw_set_parms(struct net *net, struct tcf_proto *tp, struct nlattr **tca, unsigned long base, bool ovr) { struct fw_head *head = rtnl_dereference(tp->root); - struct tcf_exts e; u32 mask; int err; - err = tcf_exts_init(&e, TCA_FW_ACT, TCA_FW_POLICE); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &f->exts, ovr); if (err < 0) return err; - err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr); - if (err < 0) - goto errout; if (tb[TCA_FW_CLASSID]) { f->res.classid = nla_get_u32(tb[TCA_FW_CLASSID]); @@ -215,10 +211,8 @@ static int fw_set_parms(struct net *net, struct tcf_proto *tp, if (tb[TCA_FW_INDEV]) { int ret; ret = tcf_change_indev(net, tb[TCA_FW_INDEV]); - if (ret < 0) { - err = ret; - goto errout; - } + if (ret < 0) + return ret; f->ifindex = ret; } #endif /* CONFIG_NET_CLS_IND */ @@ -227,16 +221,11 @@ static int fw_set_parms(struct net *net, struct tcf_proto *tp, if (tb[TCA_FW_MASK]) { mask = nla_get_u32(tb[TCA_FW_MASK]); if (mask != head->mask) - goto errout; + return err; } else if (head->mask != 0xFFFFFFFF) - goto errout; - - tcf_exts_change(tp, &f->exts, &e); + return err; return 0; -errout: - tcf_exts_destroy(&e); - return err; } static int fw_change(struct net *net, struct sk_buff *in_skb, From a74cb36980c3676c888a87dd66bcd410f7e75c08 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:08 +0200 Subject: [PATCH 13/20] net: sched: cls_matchall: no need to call tcf_exts_change for newly allocated struct As the head struct was allocated right before mall_set_parms call, no need to use tcf_exts_change to do atomic change, and we can just fill-up the unused exts struct directly by tcf_exts_validate. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_matchall.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c index 9dc26c32cf32..f35177b48373 100644 --- a/net/sched/cls_matchall.c +++ b/net/sched/cls_matchall.c @@ -120,27 +120,17 @@ static int mall_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, struct nlattr **tb, struct nlattr *est, bool ovr) { - struct tcf_exts e; int err; - err = tcf_exts_init(&e, TCA_MATCHALL_ACT, 0); - if (err) - return err; - err = tcf_exts_validate(net, tp, tb, est, &e, ovr); + err = tcf_exts_validate(net, tp, tb, est, &head->exts, ovr); if (err < 0) - goto errout; + return err; if (tb[TCA_MATCHALL_CLASSID]) { head->res.classid = nla_get_u32(tb[TCA_MATCHALL_CLASSID]); tcf_bind_filter(tp, &head->res, base); } - - tcf_exts_change(tp, &head->exts, &e); - return 0; -errout: - tcf_exts_destroy(&e); - return err; } static int mall_change(struct net *net, struct sk_buff *in_skb, From ff1f8ca0805ab03c31e14a9356aa2ac81d65c338 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:09 +0200 Subject: [PATCH 14/20] net: sched: cls_basic: no need to call tcf_exts_change for newly allocated struct As the f struct was allocated right before basic_set_parms call, no need to use tcf_exts_change to do atomic change, and we can just fill-up the unused exts struct directly by tcf_exts_validate. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_basic.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c index 979cd2683b46..7c7a82138f76 100644 --- a/net/sched/cls_basic.c +++ b/net/sched/cls_basic.c @@ -129,31 +129,22 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp, struct nlattr *est, bool ovr) { int err; - struct tcf_exts e; - err = tcf_exts_init(&e, TCA_BASIC_ACT, TCA_BASIC_POLICE); + err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr); if (err < 0) return err; - err = tcf_exts_validate(net, tp, tb, est, &e, ovr); - if (err < 0) - goto errout; err = tcf_em_tree_validate(tp, tb[TCA_BASIC_EMATCHES], &f->ematches); if (err < 0) - goto errout; + return err; if (tb[TCA_BASIC_CLASSID]) { f->res.classid = nla_get_u32(tb[TCA_BASIC_CLASSID]); tcf_bind_filter(tp, &f->res, base); } - tcf_exts_change(tp, &f->exts, &e); f->tp = tp; - return 0; -errout: - tcf_exts_destroy(&e); - return err; } static int basic_change(struct net *net, struct sk_buff *in_skb, From 6839da326dfcb98bf8020d42b416636471f1e462 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:10 +0200 Subject: [PATCH 15/20] net: sched: cls_bpf: no need to call tcf_exts_change for newly allocated struct As the prog struct was allocated right before cls_bpf_set_parms call, no need to use tcf_exts_change to do atomic change, and we can just fill-up the unused exts struct directly by tcf_exts_validate. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_bpf.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c index e9ab8374a877..cf248c3137ad 100644 --- a/net/sched/cls_bpf.c +++ b/net/sched/cls_bpf.c @@ -387,7 +387,6 @@ static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp, struct nlattr **tb, struct nlattr *est, bool ovr) { bool is_bpf, is_ebpf, have_exts = false; - struct tcf_exts exts; u32 gen_flags = 0; int ret; @@ -396,30 +395,23 @@ static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp, if ((!is_bpf && !is_ebpf) || (is_bpf && is_ebpf)) return -EINVAL; - ret = tcf_exts_init(&exts, TCA_BPF_ACT, TCA_BPF_POLICE); + ret = tcf_exts_validate(net, tp, tb, est, &prog->exts, ovr); if (ret < 0) return ret; - ret = tcf_exts_validate(net, tp, tb, est, &exts, ovr); - if (ret < 0) - goto errout; if (tb[TCA_BPF_FLAGS]) { u32 bpf_flags = nla_get_u32(tb[TCA_BPF_FLAGS]); - if (bpf_flags & ~TCA_BPF_FLAG_ACT_DIRECT) { - ret = -EINVAL; - goto errout; - } + if (bpf_flags & ~TCA_BPF_FLAG_ACT_DIRECT) + return -EINVAL; have_exts = bpf_flags & TCA_BPF_FLAG_ACT_DIRECT; } if (tb[TCA_BPF_FLAGS_GEN]) { gen_flags = nla_get_u32(tb[TCA_BPF_FLAGS_GEN]); if (gen_flags & ~CLS_BPF_SUPPORTED_GEN_FLAGS || - !tc_flags_valid(gen_flags)) { - ret = -EINVAL; - goto errout; - } + !tc_flags_valid(gen_flags)) + return -EINVAL; } prog->exts_integrated = have_exts; @@ -428,19 +420,14 @@ static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp, ret = is_bpf ? cls_bpf_prog_from_ops(tb, prog) : cls_bpf_prog_from_efd(tb, prog, tp); if (ret < 0) - goto errout; + return ret; if (tb[TCA_BPF_CLASSID]) { prog->res.classid = nla_get_u32(tb[TCA_BPF_CLASSID]); tcf_bind_filter(tp, &prog->res, base); } - tcf_exts_change(tp, &prog->exts, &exts); return 0; - -errout: - tcf_exts_destroy(&exts); - return ret; } static u32 cls_bpf_grab_new_handle(struct tcf_proto *tp, From 8cc6251381183191f99ecd6d49931251ebbbe27d Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:11 +0200 Subject: [PATCH 16/20] net: sched: cls_cgroup: no need to call tcf_exts_change for newly allocated struct As the new struct just was allocated, so no need to use tcf_exts_change to do atomic change, and we can just fill-up the unused exts struct directly by tcf_exts_validate. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_cgroup.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c index ce7d38beab95..df7a582775df 100644 --- a/net/sched/cls_cgroup.c +++ b/net/sched/cls_cgroup.c @@ -76,7 +76,6 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb, struct nlattr *tb[TCA_CGROUP_MAX + 1]; struct cls_cgroup_head *head = rtnl_dereference(tp->root); struct cls_cgroup_head *new; - struct tcf_exts e; int err; if (!tca[TCA_OPTIONS]) @@ -102,22 +101,13 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb, if (err < 0) goto errout; - err = tcf_exts_init(&e, TCA_CGROUP_ACT, TCA_CGROUP_POLICE); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &new->exts, ovr); if (err < 0) goto errout; - err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr); - if (err < 0) { - tcf_exts_destroy(&e); - goto errout; - } err = tcf_em_tree_validate(tp, tb[TCA_CGROUP_EMATCHES], &new->ematches); - if (err < 0) { - tcf_exts_destroy(&e); + if (err < 0) goto errout; - } - - tcf_exts_change(tp, &new->exts, &e); rcu_assign_pointer(tp->root, new); if (head) From c09fc2e11ed1b7fc8cfa97fb1da544225fc32277 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:12 +0200 Subject: [PATCH 17/20] net: sched: cls_flow: no need to call tcf_exts_change for newly allocated struct As the fnew struct just was allocated, so no need to use tcf_exts_change to do atomic change, and we can just fill-up the unused exts struct directly by tcf_exts_validate. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_flow.c | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c index 71fd1af01726..55e281b20140 100644 --- a/net/sched/cls_flow.c +++ b/net/sched/cls_flow.c @@ -388,7 +388,6 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, struct flow_filter *fold, *fnew; struct nlattr *opt = tca[TCA_OPTIONS]; struct nlattr *tb[TCA_FLOW_MAX + 1]; - struct tcf_exts e; unsigned int nkeys = 0; unsigned int perturb_period = 0; u32 baseclass = 0; @@ -424,31 +423,27 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, return -EOPNOTSUPP; } - err = tcf_exts_init(&e, TCA_FLOW_ACT, TCA_FLOW_POLICE); - if (err < 0) - goto err1; - err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr); - if (err < 0) - goto err1; - - err = -ENOBUFS; fnew = kzalloc(sizeof(*fnew), GFP_KERNEL); if (!fnew) - goto err1; + return -ENOBUFS; err = tcf_em_tree_validate(tp, tb[TCA_FLOW_EMATCHES], &fnew->ematches); if (err < 0) - goto err2; + goto err1; err = tcf_exts_init(&fnew->exts, TCA_FLOW_ACT, TCA_FLOW_POLICE); if (err < 0) - goto err3; + goto err2; + + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &fnew->exts, ovr); + if (err < 0) + goto err2; fold = (struct flow_filter *)*arg; if (fold) { err = -EINVAL; if (fold->handle != handle && handle) - goto err3; + goto err2; /* Copy fold into fnew */ fnew->tp = fold->tp; @@ -468,31 +463,31 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, if (tb[TCA_FLOW_MODE]) mode = nla_get_u32(tb[TCA_FLOW_MODE]); if (mode != FLOW_MODE_HASH && nkeys > 1) - goto err3; + goto err2; if (mode == FLOW_MODE_HASH) perturb_period = fold->perturb_period; if (tb[TCA_FLOW_PERTURB]) { if (mode != FLOW_MODE_HASH) - goto err3; + goto err2; perturb_period = nla_get_u32(tb[TCA_FLOW_PERTURB]) * HZ; } } else { err = -EINVAL; if (!handle) - goto err3; + goto err2; if (!tb[TCA_FLOW_KEYS]) - goto err3; + goto err2; mode = FLOW_MODE_MAP; if (tb[TCA_FLOW_MODE]) mode = nla_get_u32(tb[TCA_FLOW_MODE]); if (mode != FLOW_MODE_HASH && nkeys > 1) - goto err3; + goto err2; if (tb[TCA_FLOW_PERTURB]) { if (mode != FLOW_MODE_HASH) - goto err3; + goto err2; perturb_period = nla_get_u32(tb[TCA_FLOW_PERTURB]) * HZ; } @@ -510,8 +505,6 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, setup_deferrable_timer(&fnew->perturb_timer, flow_perturbation, (unsigned long)fnew); - tcf_exts_change(tp, &fnew->exts, &e); - netif_keep_dst(qdisc_dev(tp->q)); if (tb[TCA_FLOW_KEYS]) { @@ -550,13 +543,11 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, call_rcu(&fold->rcu, flow_destroy_filter); return 0; -err3: +err2: tcf_exts_destroy(&fnew->exts); tcf_em_tree_destroy(&fnew->ematches); -err2: - kfree(fnew); err1: - tcf_exts_destroy(&e); + kfree(fnew); return err; } From 8c98d571bb0e9717fd7be7242945e8e0abebbaa3 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:13 +0200 Subject: [PATCH 18/20] net: sched: cls_route: no need to call tcf_exts_change for newly allocated struct As the f struct was allocated right before route4_set_parms call, no need to use tcf_exts_change to do atomic change, and we can just fill-up the unused exts struct directly by tcf_exts_validate. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_route.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c index 26f863634862..f1e7d7850b44 100644 --- a/net/sched/cls_route.c +++ b/net/sched/cls_route.c @@ -372,37 +372,32 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp, struct route4_filter *fp; unsigned int h1; struct route4_bucket *b; - struct tcf_exts e; int err; - err = tcf_exts_init(&e, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE); + err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr); if (err < 0) return err; - err = tcf_exts_validate(net, tp, tb, est, &e, ovr); - if (err < 0) - goto errout; - err = -EINVAL; if (tb[TCA_ROUTE4_TO]) { if (new && handle & 0x8000) - goto errout; + return -EINVAL; to = nla_get_u32(tb[TCA_ROUTE4_TO]); if (to > 0xFF) - goto errout; + return -EINVAL; nhandle = to; } if (tb[TCA_ROUTE4_FROM]) { if (tb[TCA_ROUTE4_IIF]) - goto errout; + return -EINVAL; id = nla_get_u32(tb[TCA_ROUTE4_FROM]); if (id > 0xFF) - goto errout; + return -EINVAL; nhandle |= id << 16; } else if (tb[TCA_ROUTE4_IIF]) { id = nla_get_u32(tb[TCA_ROUTE4_IIF]); if (id > 0x7FFF) - goto errout; + return -EINVAL; nhandle |= (id | 0x8000) << 16; } else nhandle |= 0xFFFF << 16; @@ -410,27 +405,25 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp, if (handle && new) { nhandle |= handle & 0x7F00; if (nhandle != handle) - goto errout; + return -EINVAL; } h1 = to_hash(nhandle); b = rtnl_dereference(head->table[h1]); if (!b) { - err = -ENOBUFS; b = kzalloc(sizeof(struct route4_bucket), GFP_KERNEL); if (b == NULL) - goto errout; + return -ENOBUFS; rcu_assign_pointer(head->table[h1], b); } else { unsigned int h2 = from_hash(nhandle >> 16); - err = -EEXIST; for (fp = rtnl_dereference(b->ht[h2]); fp; fp = rtnl_dereference(fp->next)) if (fp->handle == f->handle) - goto errout; + return -EEXIST; } if (tb[TCA_ROUTE4_TO]) @@ -450,12 +443,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp, tcf_bind_filter(tp, &f->res, base); } - tcf_exts_change(tp, &f->exts, &e); - return 0; -errout: - tcf_exts_destroy(&e); - return err; } static int route4_change(struct net *net, struct sk_buff *in_skb, From 705c7091262d02b09eb686c24491de61bf42fdb2 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:14 +0200 Subject: [PATCH 19/20] net: sched: cls_u32: no need to call tcf_exts_change for newly allocated struct As the n struct was allocated right before u32_set_parms call, no need to use tcf_exts_change to do atomic change, and we can just fill-up the unused exts struct directly by tcf_exts_validate. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_u32.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 2d01195153e6..9fd243799fe7 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -723,29 +723,24 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp, struct tc_u_knode *n, struct nlattr **tb, struct nlattr *est, bool ovr) { - struct tcf_exts e; int err; - err = tcf_exts_init(&e, TCA_U32_ACT, TCA_U32_POLICE); + err = tcf_exts_validate(net, tp, tb, est, &n->exts, ovr); if (err < 0) return err; - err = tcf_exts_validate(net, tp, tb, est, &e, ovr); - if (err < 0) - goto errout; - err = -EINVAL; if (tb[TCA_U32_LINK]) { u32 handle = nla_get_u32(tb[TCA_U32_LINK]); struct tc_u_hnode *ht_down = NULL, *ht_old; if (TC_U32_KEY(handle)) - goto errout; + return -EINVAL; if (handle) { ht_down = u32_lookup_ht(ht->tp_c, handle); if (ht_down == NULL) - goto errout; + return -EINVAL; ht_down->refcnt++; } @@ -765,16 +760,11 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp, int ret; ret = tcf_change_indev(net, tb[TCA_U32_INDEV]); if (ret < 0) - goto errout; + return -EINVAL; n->ifindex = ret; } #endif - tcf_exts_change(tp, &n->exts, &e); - return 0; -errout: - tcf_exts_destroy(&e); - return err; } static void u32_replace_knode(struct tcf_proto *tp, struct tc_u_common *tp_c, From 9b0d4446b56904b59ae3809913b0ac760fa941a6 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Fri, 4 Aug 2017 14:29:15 +0200 Subject: [PATCH 20/20] net: sched: avoid atomic swap in tcf_exts_change tcf_exts_change is always called on newly created exts, which are not used on fastpath. Therefore, simple struct copy is enough. Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- include/net/pkt_cls.h | 3 +-- net/sched/cls_api.c | 10 ++-------- net/sched/cls_rsvp.h | 4 ++-- net/sched/cls_tcindex.c | 6 +++--- 4 files changed, 8 insertions(+), 15 deletions(-) diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index b8959c9a190d..e0c54f111467 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -201,8 +201,7 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, struct nlattr *rate_tlv, struct tcf_exts *exts, bool ovr); void tcf_exts_destroy(struct tcf_exts *exts); -void tcf_exts_change(struct tcf_proto *tp, struct tcf_exts *dst, - struct tcf_exts *src); +void tcf_exts_change(struct tcf_exts *dst, struct tcf_exts *src); int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts); int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts); int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts, diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 735d556a5283..e655221c654e 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -883,18 +883,12 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, } EXPORT_SYMBOL(tcf_exts_validate); -void tcf_exts_change(struct tcf_proto *tp, struct tcf_exts *dst, - struct tcf_exts *src) +void tcf_exts_change(struct tcf_exts *dst, struct tcf_exts *src) { #ifdef CONFIG_NET_CLS_ACT struct tcf_exts old = *dst; - tcf_tree_lock(tp); - dst->nr_actions = src->nr_actions; - dst->actions = src->actions; - dst->type = src->type; - tcf_tree_unlock(tp); - + *dst = *src; tcf_exts_destroy(&old); #endif } diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h index 0d9d07798699..4adb67a73491 100644 --- a/net/sched/cls_rsvp.h +++ b/net/sched/cls_rsvp.h @@ -518,7 +518,7 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb, tcf_bind_filter(tp, &n->res, base); } - tcf_exts_change(tp, &n->exts, &e); + tcf_exts_change(&n->exts, &e); rsvp_replace(tp, n, handle); return 0; } @@ -591,7 +591,7 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb, if (f->tunnelhdr == 0) tcf_bind_filter(tp, &f->res, base); - tcf_exts_change(tp, &f->exts, &e); + tcf_exts_change(&f->exts, &e); fp = &s->ht[h2]; for (nfp = rtnl_dereference(*fp); nfp; diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c index 66924d147e97..d69f828f3fed 100644 --- a/net/sched/cls_tcindex.c +++ b/net/sched/cls_tcindex.c @@ -419,9 +419,9 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, } if (old_r) - tcf_exts_change(tp, &r->exts, &e); + tcf_exts_change(&r->exts, &e); else - tcf_exts_change(tp, &cr.exts, &e); + tcf_exts_change(&cr.exts, &e); if (old_r && old_r != r) { err = tcindex_filter_result_init(old_r); @@ -439,7 +439,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, struct tcindex_filter *nfp; struct tcindex_filter __rcu **fp; - tcf_exts_change(tp, &f->result.exts, &r->exts); + tcf_exts_change(&f->result.exts, &r->exts); fp = cp->h + (handle % cp->hash); for (nfp = rtnl_dereference(*fp);