Skip to content

Commit b96c22c

Browse files
davem330davem330
committed
Merge branch 'tc_action-fixes'
Cong Wang says: ==================== net_sched: tc action fixes and updates This patchset fixes a few regressions caused by the previous code refactor and more. Thanks to Jamal for catching them! Note, patch 3/7 and 4/7 are not strictly necessary for this patchset, I just want to carry them together. --- v4: adjust an indention for Jamal add two more patches v3: avoid list for fast path, suggested by Jamal v2: replace flex_array with regular dynamic array keep tcf_action_stats_update() in act_api.h fix macro typos found by Amir ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents f4abf05 + b5ac851 commit b96c22c

File tree

8 files changed

+112
-119
lines changed

8 files changed

+112
-119
lines changed

‎drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8396,12 +8396,14 @@ static int parse_tc_actions(struct ixgbe_adapter *adapter,
83968396
struct tcf_exts *exts, u64 *action, u8 *queue)
83978397
{
83988398
const struct tc_action *a;
8399+
LIST_HEAD(actions);
83998400
int err;
84008401

84018402
if (tc_no_actions(exts))
84028403
return -EINVAL;
84038404

8404-
tc_for_each_action(a, exts) {
8405+
tcf_exts_to_list(exts, &actions);
8406+
list_for_each_entry(a, &actions, list) {
84058407

84068408
/* Drop action */
84078409
if (is_tcf_gact_shot(a)) {

‎drivers/net/ethernet/mellanox/mlx5/core/en_tc.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,14 +318,16 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
318318
u32 *action, u32 *flow_tag)
319319
{
320320
const struct tc_action *a;
321+
LIST_HEAD(actions);
321322

322323
if (tc_no_actions(exts))
323324
return -EINVAL;
324325

325326
*flow_tag = MLX5_FS_DEFAULT_FLOW_TAG;
326327
*action = 0;
327328

328-
tc_for_each_action(a, exts) {
329+
tcf_exts_to_list(exts, &actions);
330+
list_for_each_entry(a, &actions, list) {
329331
/* Only support a single action per rule */
330332
if (*action)
331333
return -EINVAL;
@@ -362,13 +364,15 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
362364
u32 *action, u32 *dest_vport)
363365
{
364366
const struct tc_action *a;
367+
LIST_HEAD(actions);
365368

366369
if (tc_no_actions(exts))
367370
return -EINVAL;
368371

369372
*action = 0;
370373

371-
tc_for_each_action(a, exts) {
374+
tcf_exts_to_list(exts, &actions);
375+
list_for_each_entry(a, &actions, list) {
372376
/* Only support a single action per rule */
373377
if (*action)
374378
return -EINVAL;
@@ -503,6 +507,7 @@ int mlx5e_stats_flower(struct mlx5e_priv *priv,
503507
struct mlx5e_tc_flow *flow;
504508
struct tc_action *a;
505509
struct mlx5_fc *counter;
510+
LIST_HEAD(actions);
506511
u64 bytes;
507512
u64 packets;
508513
u64 lastuse;
@@ -518,7 +523,8 @@ int mlx5e_stats_flower(struct mlx5e_priv *priv,
518523

519524
mlx5_fc_query_cached(counter, &bytes, &packets, &lastuse);
520525

521-
tc_for_each_action(a, f->exts)
526+
tcf_exts_to_list(f->exts, &actions);
527+
list_for_each_entry(a, &actions, list)
522528
tcf_action_stats_update(a, bytes, packets, lastuse);
523529

524530
return 0;

‎drivers/net/ethernet/mellanox/mlxsw/spectrum.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1121,14 +1121,16 @@ static int mlxsw_sp_port_add_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
11211121
bool ingress)
11221122
{
11231123
const struct tc_action *a;
1124+
LIST_HEAD(actions);
11241125
int err;
11251126

11261127
if (!tc_single_action(cls->exts)) {
11271128
netdev_err(mlxsw_sp_port->dev, "only singular actions are supported\n");
11281129
return -ENOTSUPP;
11291130
}
11301131

1131-
tc_for_each_action(a, cls->exts) {
1132+
tcf_exts_to_list(cls->exts, &actions);
1133+
list_for_each_entry(a, &actions, list) {
11321134
if (!is_tcf_mirred_mirror(a) || protocol != htons(ETH_P_ALL))
11331135
return -ENOTSUPP;
11341136

‎include/net/act_api.h

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ int tcf_register_action(struct tc_action_ops *a, struct pernet_operations *ops);
176176
int tcf_unregister_action(struct tc_action_ops *a,
177177
struct pernet_operations *ops);
178178
int tcf_action_destroy(struct list_head *actions, int bind);
179-
int tcf_action_exec(struct sk_buff *skb, const struct list_head *actions,
180-
struct tcf_result *res);
179+
int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
180+
int nr_actions, struct tcf_result *res);
181181
int tcf_action_init(struct net *net, struct nlattr *nla,
182182
struct nlattr *est, char *n, int ovr,
183183
int bind, struct list_head *);
@@ -189,30 +189,17 @@ int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
189189
int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
190190
int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int);
191191

192-
#define tc_no_actions(_exts) \
193-
(list_empty(&(_exts)->actions))
194-
195-
#define tc_for_each_action(_a, _exts) \
196-
list_for_each_entry(a, &(_exts)->actions, list)
197-
198-
#define tc_single_action(_exts) \
199-
(list_is_singular(&(_exts)->actions))
192+
#endif /* CONFIG_NET_CLS_ACT */
200193

201194
static inline void tcf_action_stats_update(struct tc_action *a, u64 bytes,
202195
u64 packets, u64 lastuse)
203196
{
197+
#ifdef CONFIG_NET_CLS_ACT
204198
if (!a->ops->stats_update)
205199
return;
206200

207201
a->ops->stats_update(a, bytes, packets, lastuse);
202+
#endif
208203
}
209204

210-
#else /* CONFIG_NET_CLS_ACT */
211-
212-
#define tc_no_actions(_exts) true
213-
#define tc_for_each_action(_a, _exts) while ((void)(_a), 0)
214-
#define tc_single_action(_exts) false
215-
#define tcf_action_stats_update(a, bytes, packets, lastuse)
216-
217-
#endif /* CONFIG_NET_CLS_ACT */
218205
#endif

‎include/net/pkt_cls.h

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ tcf_unbind_filter(struct tcf_proto *tp, struct tcf_result *r)
5959
struct tcf_exts {
6060
#ifdef CONFIG_NET_CLS_ACT
6161
__u32 type; /* for backward compat(TCA_OLD_COMPAT) */
62-
struct list_head actions;
62+
int nr_actions;
63+
struct tc_action **actions;
6364
#endif
6465
/* Map to export classifier specific extension TLV types to the
6566
* generic extensions API. Unsupported extensions must be set to 0.
@@ -72,7 +73,10 @@ static inline void tcf_exts_init(struct tcf_exts *exts, int action, int police)
7273
{
7374
#ifdef CONFIG_NET_CLS_ACT
7475
exts->type = 0;
75-
INIT_LIST_HEAD(&exts->actions);
76+
exts->nr_actions = 0;
77+
exts->actions = kcalloc(TCA_ACT_MAX_PRIO, sizeof(struct tc_action *),
78+
GFP_KERNEL);
79+
WARN_ON(!exts->actions); /* TODO: propagate the error to callers */
7680
#endif
7781
exts->action = action;
7882
exts->police = police;
@@ -89,7 +93,7 @@ static inline int
8993
tcf_exts_is_predicative(struct tcf_exts *exts)
9094
{
9195
#ifdef CONFIG_NET_CLS_ACT
92-
return !list_empty(&exts->actions);
96+
return exts->nr_actions;
9397
#else
9498
return 0;
9599
#endif
@@ -108,6 +112,20 @@ tcf_exts_is_available(struct tcf_exts *exts)
108112
return tcf_exts_is_predicative(exts);
109113
}
110114

115+
static inline void tcf_exts_to_list(const struct tcf_exts *exts,
116+
struct list_head *actions)
117+
{
118+
#ifdef CONFIG_NET_CLS_ACT
119+
int i;
120+
121+
for (i = 0; i < exts->nr_actions; i++) {
122+
struct tc_action *a = exts->actions[i];
123+
124+
list_add(&a->list, actions);
125+
}
126+
#endif
127+
}
128+
111129
/**
112130
* tcf_exts_exec - execute tc filter extensions
113131
* @skb: socket buffer
@@ -124,12 +142,25 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts,
124142
struct tcf_result *res)
125143
{
126144
#ifdef CONFIG_NET_CLS_ACT
127-
if (!list_empty(&exts->actions))
128-
return tcf_action_exec(skb, &exts->actions, res);
145+
if (exts->nr_actions)
146+
return tcf_action_exec(skb, exts->actions, exts->nr_actions,
147+
res);
129148
#endif
130149
return 0;
131150
}
132151

152+
#ifdef CONFIG_NET_CLS_ACT
153+
154+
#define tc_no_actions(_exts) ((_exts)->nr_actions == 0)
155+
#define tc_single_action(_exts) ((_exts)->nr_actions == 1)
156+
157+
#else /* CONFIG_NET_CLS_ACT */
158+
159+
#define tc_no_actions(_exts) true
160+
#define tc_single_action(_exts) false
161+
162+
#endif /* CONFIG_NET_CLS_ACT */
163+
133164
int tcf_exts_validate(struct net *net, struct tcf_proto *tp,
134165
struct nlattr **tb, struct nlattr *rate_tlv,
135166
struct tcf_exts *exts, bool ovr);

‎net/sched/act_api.c

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ int __tcf_hash_release(struct tc_action *p, bool bind, bool strict)
6464
if (p->tcfa_bindcnt <= 0 && p->tcfa_refcnt <= 0) {
6565
if (p->ops->cleanup)
6666
p->ops->cleanup(p, bind);
67-
list_del(&p->list);
6867
tcf_hash_destroy(p->hinfo, p);
6968
ret = ACT_P_DELETED;
7069
}
@@ -421,18 +420,19 @@ static struct tc_action_ops *tc_lookup_action(struct nlattr *kind)
421420
return res;
422421
}
423422

424-
int tcf_action_exec(struct sk_buff *skb, const struct list_head *actions,
425-
struct tcf_result *res)
423+
int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
424+
int nr_actions, struct tcf_result *res)
426425
{
427-
const struct tc_action *a;
428-
int ret = -1;
426+
int ret = -1, i;
429427

430428
if (skb->tc_verd & TC_NCLS) {
431429
skb->tc_verd = CLR_TC_NCLS(skb->tc_verd);
432430
ret = TC_ACT_OK;
433431
goto exec_done;
434432
}
435-
list_for_each_entry(a, actions, list) {
433+
for (i = 0; i < nr_actions; i++) {
434+
const struct tc_action *a = actions[i];
435+
436436
repeat:
437437
ret = a->ops->act(skb, a, res);
438438
if (ret == TC_ACT_REPEAT)
@@ -754,16 +754,6 @@ static struct tc_action *tcf_action_get_1(struct net *net, struct nlattr *nla,
754754
return ERR_PTR(err);
755755
}
756756

757-
static void cleanup_a(struct list_head *actions)
758-
{
759-
struct tc_action *a, *tmp;
760-
761-
list_for_each_entry_safe(a, tmp, actions, list) {
762-
list_del(&a->list);
763-
kfree(a);
764-
}
765-
}
766-
767757
static int tca_action_flush(struct net *net, struct nlattr *nla,
768758
struct nlmsghdr *n, u32 portid)
769759
{
@@ -905,7 +895,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
905895
return ret;
906896
}
907897
err:
908-
cleanup_a(&actions);
898+
tcf_action_destroy(&actions, 0);
909899
return ret;
910900
}
911901

@@ -942,15 +932,9 @@ tcf_action_add(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
942932

943933
ret = tcf_action_init(net, nla, NULL, NULL, ovr, 0, &actions);
944934
if (ret)
945-
goto done;
935+
return ret;
946936

947-
/* dump then free all the actions after update; inserted policy
948-
* stays intact
949-
*/
950-
ret = tcf_add_notify(net, n, &actions, portid);
951-
cleanup_a(&actions);
952-
done:
953-
return ret;
937+
return tcf_add_notify(net, n, &actions, portid);
954938
}
955939

956940
static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n)

‎net/sched/act_police.c

Lines changed: 11 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -63,49 +63,8 @@ static int tcf_act_police_walker(struct net *net, struct sk_buff *skb,
6363
const struct tc_action_ops *ops)
6464
{
6565
struct tc_action_net *tn = net_generic(net, police_net_id);
66-
struct tcf_hashinfo *hinfo = tn->hinfo;
67-
int err = 0, index = -1, i = 0, s_i = 0, n_i = 0;
68-
struct nlattr *nest;
69-
70-
spin_lock_bh(&hinfo->lock);
71-
72-
s_i = cb->args[0];
73-
74-
for (i = 0; i < (POL_TAB_MASK + 1); i++) {
75-
struct hlist_head *head;
76-
struct tc_action *p;
77-
78-
head = &hinfo->htab[tcf_hash(i, POL_TAB_MASK)];
79-
80-
hlist_for_each_entry_rcu(p, head, tcfa_head) {
81-
index++;
82-
if (index < s_i)
83-
continue;
84-
nest = nla_nest_start(skb, index);
85-
if (nest == NULL)
86-
goto nla_put_failure;
87-
if (type == RTM_DELACTION)
88-
err = tcf_action_dump_1(skb, p, 0, 1);
89-
else
90-
err = tcf_action_dump_1(skb, p, 0, 0);
91-
if (err < 0) {
92-
index--;
93-
nla_nest_cancel(skb, nest);
94-
goto done;
95-
}
96-
nla_nest_end(skb, nest);
97-
n_i++;
98-
}
99-
}
100-
done:
101-
spin_unlock_bh(&hinfo->lock);
102-
if (n_i)
103-
cb->args[0] += n_i;
104-
return n_i;
10566

106-
nla_put_failure:
107-
nla_nest_cancel(skb, nest);
108-
goto done;
67+
return tcf_generic_walker(tn, skb, cb, type, ops);
10968
}
11069

11170
static const struct nla_policy police_policy[TCA_POLICE_MAX + 1] = {
@@ -125,6 +84,7 @@ static int tcf_act_police_init(struct net *net, struct nlattr *nla,
12584
struct tcf_police *police;
12685
struct qdisc_rate_table *R_tab = NULL, *P_tab = NULL;
12786
struct tc_action_net *tn = net_generic(net, police_net_id);
87+
bool exists = false;
12888
int size;
12989

13090
if (nla == NULL)
@@ -139,24 +99,24 @@ static int tcf_act_police_init(struct net *net, struct nlattr *nla,
13999
size = nla_len(tb[TCA_POLICE_TBF]);
140100
if (size != sizeof(*parm) && size != sizeof(struct tc_police_compat))
141101
return -EINVAL;
102+
142103
parm = nla_data(tb[TCA_POLICE_TBF]);
104+
exists = tcf_hash_check(tn, parm->index, a, bind);
105+
if (exists && bind)
106+
return 0;
143107

144-
if (parm->index) {
145-
if (tcf_hash_check(tn, parm->index, a, bind)) {
146-
if (ovr)
147-
goto override;
148-
/* not replacing */
149-
return -EEXIST;
150-
}
151-
} else {
108+
if (!exists) {
152109
ret = tcf_hash_create(tn, parm->index, NULL, a,
153110
&act_police_ops, bind, false);
154111
if (ret)
155112
return ret;
156113
ret = ACT_P_CREATED;
114+
} else {
115+
tcf_hash_release(*a, bind);
116+
if (!ovr)
117+
return -EEXIST;
157118
}
158119

159-
override:
160120
police = to_police(*a);
161121
if (parm->rate.rate) {
162122
err = -ENOMEM;

0 commit comments

Comments
 (0)