Skip to content

Commit

Permalink
net: sched: atomically check-allocate action
Browse files Browse the repository at this point in the history 
Implement function that atomically checks if action exists and either takes
reference to it, or allocates idr slot for action index to prevent
concurrent allocations of actions with same index. Use EBUSY error pointer
to indicate that idr slot is reserved.

Implement cleanup helper function that removes temporary error pointer from
idr. (in case of error between idr allocation and insertion of newly
created action to specified index)

Refactor all action init functions to insert new action to idr using this
API.

Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
@w1ldptr @davem330
w1ldptr authored and davem330 committed Jul 8, 2018
1 parent cae422f commit 0190c1d
Show file tree
Hide file tree
Showing 18 changed files with 216 additions and 59 deletions.
3 changes: 3 additions & 0 deletions include/net/act_api.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
int bind, bool cpustats);
void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a);

void tcf_idr_cleanup(struct tc_action_net *tn, u32 index);
int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
struct tc_action **a, int bind);
int tcf_idr_delete_index(struct tc_action_net *tn, u32 index);
int __tcf_idr_release(struct tc_action *a, bool bind, bool strict);

Expand Down
92 changes: 72 additions & 20 deletions net/sched/act_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,9 @@ static bool __tcf_idr_check(struct tc_action_net *tn, u32 index,

spin_lock(&idrinfo->lock);
p = idr_find(&idrinfo->action_idr, index);
if (p) {
if (IS_ERR(p)) {
p = NULL;
} else if (p) {
refcount_inc(&p->tcfa_refcnt);
if (bind)
atomic_inc(&p->tcfa_bindcnt);
Expand Down Expand Up @@ -371,7 +373,6 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
{
struct tc_action *p = kzalloc(ops->size, GFP_KERNEL);
struct tcf_idrinfo *idrinfo = tn->idrinfo;
struct idr *idr = &idrinfo->action_idr;
int err = -ENOMEM;

if (unlikely(!p))
Expand All @@ -389,20 +390,6 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
goto err2;
}
spin_lock_init(&p->tcfa_lock);
idr_preload(GFP_KERNEL);
spin_lock(&idrinfo->lock);
/* user doesn't specify an index */
if (!index) {
index = 1;
err = idr_alloc_u32(idr, NULL, &index, UINT_MAX, GFP_ATOMIC);
} else {
err = idr_alloc_u32(idr, NULL, &index, index, GFP_ATOMIC);
}
spin_unlock(&idrinfo->lock);
idr_preload_end();
if (err)
goto err3;

p->tcfa_index = index;
p->tcfa_tm.install = jiffies;
p->tcfa_tm.lastuse = jiffies;
Expand All @@ -412,16 +399,14 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
&p->tcfa_rate_est,
&p->tcfa_lock, NULL, est);
if (err)
goto err4;
goto err3;
}

p->idrinfo = idrinfo;
p->ops = ops;
INIT_LIST_HEAD(&p->list);
*a = p;
return 0;
err4:
idr_remove(idr, index);
err3:
free_percpu(p->cpu_qstats);
err2:
Expand All @@ -437,11 +422,78 @@ void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a)
struct tcf_idrinfo *idrinfo = tn->idrinfo;

spin_lock(&idrinfo->lock);
idr_replace(&idrinfo->action_idr, a, a->tcfa_index);
/* Replace ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc */
WARN_ON(!IS_ERR(idr_replace(&idrinfo->action_idr, a, a->tcfa_index)));
spin_unlock(&idrinfo->lock);
}
EXPORT_SYMBOL(tcf_idr_insert);

/* Cleanup idr index that was allocated but not initialized. */

void tcf_idr_cleanup(struct tc_action_net *tn, u32 index)
{
struct tcf_idrinfo *idrinfo = tn->idrinfo;

spin_lock(&idrinfo->lock);
/* Remove ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc */
WARN_ON(!IS_ERR(idr_remove(&idrinfo->action_idr, index)));
spin_unlock(&idrinfo->lock);
}
EXPORT_SYMBOL(tcf_idr_cleanup);

/* Check if action with specified index exists. If actions is found, increments
* its reference and bind counters, and return 1. Otherwise insert temporary
* error pointer (to prevent concurrent users from inserting actions with same
* index) and return 0.
*/

int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
struct tc_action **a, int bind)
{
struct tcf_idrinfo *idrinfo = tn->idrinfo;
struct tc_action *p;
int ret;

again:
spin_lock(&idrinfo->lock);
if (*index) {
p = idr_find(&idrinfo->action_idr, *index);
if (IS_ERR(p)) {
/* This means that another process allocated
* index but did not assign the pointer yet.
*/
spin_unlock(&idrinfo->lock);
goto again;
}

if (p) {
refcount_inc(&p->tcfa_refcnt);
if (bind)
atomic_inc(&p->tcfa_bindcnt);
*a = p;
ret = 1;
} else {
*a = NULL;
ret = idr_alloc_u32(&idrinfo->action_idr, NULL, index,
*index, GFP_ATOMIC);
if (!ret)
idr_replace(&idrinfo->action_idr,
ERR_PTR(-EBUSY), *index);
}
} else {
*index = 1;
*a = NULL;
ret = idr_alloc_u32(&idrinfo->action_idr, NULL, index,
UINT_MAX, GFP_ATOMIC);
if (!ret)
idr_replace(&idrinfo->action_idr, ERR_PTR(-EBUSY),
*index);
}
spin_unlock(&idrinfo->lock);
return ret;
}
EXPORT_SYMBOL(tcf_idr_check_alloc);

void tcf_idrinfo_destroy(const struct tc_action_ops *ops,
struct tcf_idrinfo *idrinfo)
{
Expand Down
11 changes: 8 additions & 3 deletions net/sched/act_bpf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,14 +299,17 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,

parm = nla_data(tb[TCA_ACT_BPF_PARMS]);

if (!tcf_idr_check(tn, parm->index, act, bind)) {
ret = tcf_idr_check_alloc(tn, &parm->index, act, bind);
if (!ret) {
ret = tcf_idr_create(tn, parm->index, est, act,
&act_bpf_ops, bind, true);
if (ret < 0)
if (ret < 0) {
tcf_idr_cleanup(tn, parm->index);
return ret;
}

res = ACT_P_CREATED;
} else {
} else if (ret > 0) {
/* Don't override defaults. */
if (bind)
return 0;
Expand All @@ -315,6 +318,8 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
tcf_idr_release(*act, bind);
return -EEXIST;
}
} else {
return ret;
}

is_bpf = tb[TCA_ACT_BPF_OPS_LEN] && tb[TCA_ACT_BPF_OPS];
Expand Down
10 changes: 7 additions & 3 deletions net/sched/act_connmark.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,14 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,

parm = nla_data(tb[TCA_CONNMARK_PARMS]);

if (!tcf_idr_check(tn, parm->index, a, bind)) {
ret = tcf_idr_check_alloc(tn, &parm->index, a, bind);
if (!ret) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_connmark_ops, bind, false);
if (ret)
if (ret) {
tcf_idr_cleanup(tn, parm->index);
return ret;
}

ci = to_connmark(*a);
ci->tcf_action = parm->action;
Expand All @@ -131,7 +134,7 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,

tcf_idr_insert(tn, *a);
ret = ACT_P_CREATED;
} else {
} else if (ret > 0) {
ci = to_connmark(*a);
if (bind)
return 0;
Expand All @@ -142,6 +145,7 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
/* replacing action and zone */
ci->tcf_action = parm->action;
ci->zone = parm->zone;
ret = 0;
}

return ret;
Expand Down
11 changes: 8 additions & 3 deletions net/sched/act_csum.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,24 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
return -EINVAL;
parm = nla_data(tb[TCA_CSUM_PARMS]);

if (!tcf_idr_check(tn, parm->index, a, bind)) {
err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
if (!err) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_csum_ops, bind, true);
if (ret)
if (ret) {
tcf_idr_cleanup(tn, parm->index);
return ret;
}
ret = ACT_P_CREATED;
} else {
} else if (err > 0) {
if (bind)/* dont override defaults */
return 0;
if (!ovr) {
tcf_idr_release(*a, bind);
return -EEXIST;
}
} else {
return err;
}

p = to_tcf_csum(*a);
Expand Down
11 changes: 8 additions & 3 deletions net/sched/act_gact.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,24 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
}
#endif

if (!tcf_idr_check(tn, parm->index, a, bind)) {
err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
if (!err) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_gact_ops, bind, true);
if (ret)
if (ret) {
tcf_idr_cleanup(tn, parm->index);
return ret;
}
ret = ACT_P_CREATED;
} else {
} else if (err > 0) {
if (bind)/* dont override defaults */
return 0;
if (!ovr) {
tcf_idr_release(*a, bind);
return -EEXIST;
}
} else {
return err;
}

gact = to_gact(*a);
Expand Down
6 changes: 5 additions & 1 deletion net/sched/act_ife.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,10 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
if (!p)
return -ENOMEM;

exists = tcf_idr_check(tn, parm->index, a, bind);
err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
if (err < 0)
return err;
exists = err;
if (exists && bind) {
kfree(p);
return 0;
Expand All @@ -494,6 +497,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
ret = tcf_idr_create(tn, parm->index, est, a, &act_ife_ops,
bind, true);
if (ret) {
tcf_idr_cleanup(tn, parm->index);
kfree(p);
return ret;
}
Expand Down
13 changes: 11 additions & 2 deletions net/sched/act_ipt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,28 +119,37 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
if (tb[TCA_IPT_INDEX] != NULL)
index = nla_get_u32(tb[TCA_IPT_INDEX]);

exists = tcf_idr_check(tn, index, a, bind);
err = tcf_idr_check_alloc(tn, &index, a, bind);
if (err < 0)
return err;
exists = err;
if (exists && bind)
return 0;

if (tb[TCA_IPT_HOOK] == NULL || tb[TCA_IPT_TARG] == NULL) {
if (exists)
tcf_idr_release(*a, bind);
else
tcf_idr_cleanup(tn, index);
return -EINVAL;
}

td = (struct xt_entry_target *)nla_data(tb[TCA_IPT_TARG]);
if (nla_len(tb[TCA_IPT_TARG]) < td->u.target_size) {
if (exists)
tcf_idr_release(*a, bind);
else
tcf_idr_cleanup(tn, index);
return -EINVAL;
}

if (!exists) {
ret = tcf_idr_create(tn, index, est, a, ops, bind,
false);
if (ret)
if (ret) {
tcf_idr_cleanup(tn, index);
return ret;
}
ret = ACT_P_CREATED;
} else {
if (bind)/* dont override defaults */
Expand Down
16 changes: 13 additions & 3 deletions net/sched/act_mirred.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
struct tcf_mirred *m;
struct net_device *dev;
bool exists = false;
int ret;
int ret, err;

if (!nla) {
NL_SET_ERR_MSG_MOD(extack, "Mirred requires attributes to be passed");
Expand All @@ -94,7 +94,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
}
parm = nla_data(tb[TCA_MIRRED_PARMS]);

exists = tcf_idr_check(tn, parm->index, a, bind);
err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
if (err < 0)
return err;
exists = err;
if (exists && bind)
return 0;

Expand All @@ -107,6 +110,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
default:
if (exists)
tcf_idr_release(*a, bind);
else
tcf_idr_cleanup(tn, parm->index);
NL_SET_ERR_MSG_MOD(extack, "Unknown mirred option");
return -EINVAL;
}
Expand All @@ -115,6 +120,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
if (dev == NULL) {
if (exists)
tcf_idr_release(*a, bind);
else
tcf_idr_cleanup(tn, parm->index);
return -ENODEV;
}
mac_header_xmit = dev_is_mac_header_xmit(dev);
Expand All @@ -124,13 +131,16 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,

if (!exists) {
if (!dev) {
tcf_idr_cleanup(tn, parm->index);
NL_SET_ERR_MSG_MOD(extack, "Specified device does not exist");
return -EINVAL;
}
ret = tcf_idr_create(tn, parm->index, est, a,
&act_mirred_ops, bind, true);
if (ret)
if (ret) {
tcf_idr_cleanup(tn, parm->index);
return ret;
}
ret = ACT_P_CREATED;
} else if (!ovr) {
tcf_idr_release(*a, bind);
Expand Down
Loading

0 comments on commit 0190c1d

Please sign in to comment.