Skip to content

Commit

Permalink
net sched: fix race in mirred device removal
Browse files Browse the repository at this point in the history
This fixes hang when target device of mirred packet classifier
action is removed.

If a mirror or redirection action is configured to cause packets
to go to another device, the classifier holds a ref count, but was assuming
the adminstrator cleaned up all redirections before removing. The fix
is to add a notifier and cleanup during unregister.

The new list is implicitly protected by RTNL mutex because
it is held during filter add/delete as well as notifier.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Acked-by: Jamal Hadi Salim <hadi@cyberus.ca>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
stephen hemminger authored and David S. Miller committed Jul 25, 2010
1 parent 76ac21f commit 3b87956
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 3 deletions.
1 change: 1 addition & 0 deletions include/net/tc_act/tc_mirred.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ struct tcf_mirred {
int tcfm_ifindex;
int tcfm_ok_push;
struct net_device *tcfm_dev;
struct list_head tcfm_list;
};
#define to_mirred(pc) \
container_of(pc, struct tcf_mirred, common)
Expand Down
43 changes: 40 additions & 3 deletions net/sched/act_mirred.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
static struct tcf_common *tcf_mirred_ht[MIRRED_TAB_MASK + 1];
static u32 mirred_idx_gen;
static DEFINE_RWLOCK(mirred_lock);
static LIST_HEAD(mirred_list);

static struct tcf_hashinfo mirred_hash_info = {
.htab = tcf_mirred_ht,
Expand All @@ -47,7 +48,9 @@ static inline int tcf_mirred_release(struct tcf_mirred *m, int bind)
m->tcf_bindcnt--;
m->tcf_refcnt--;
if(!m->tcf_bindcnt && m->tcf_refcnt <= 0) {
dev_put(m->tcfm_dev);
list_del(&m->tcfm_list);
if (m->tcfm_dev)
dev_put(m->tcfm_dev);
tcf_hash_destroy(&m->common, &mirred_hash_info);
return 1;
}
Expand Down Expand Up @@ -134,8 +137,10 @@ static int tcf_mirred_init(struct nlattr *nla, struct nlattr *est,
m->tcfm_ok_push = ok_push;
}
spin_unlock_bh(&m->tcf_lock);
if (ret == ACT_P_CREATED)
if (ret == ACT_P_CREATED) {
list_add(&m->tcfm_list, &mirred_list);
tcf_hash_insert(pc, &mirred_hash_info);
}

return ret;
}
Expand All @@ -162,9 +167,14 @@ static int tcf_mirred(struct sk_buff *skb, struct tc_action *a,
m->tcf_tm.lastuse = jiffies;

dev = m->tcfm_dev;
if (!dev) {
printk_once(KERN_NOTICE "tc mirred: target device is gone\n");
goto out;
}

if (!(dev->flags & IFF_UP)) {
if (net_ratelimit())
pr_notice("tc mirred to Houston: device %s is gone!\n",
pr_notice("tc mirred to Houston: device %s is down\n",
dev->name);
goto out;
}
Expand Down Expand Up @@ -232,6 +242,28 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind, i
return -1;
}

static int mirred_device_event(struct notifier_block *unused,
unsigned long event, void *ptr)
{
struct net_device *dev = ptr;
struct tcf_mirred *m;

if (event == NETDEV_UNREGISTER)
list_for_each_entry(m, &mirred_list, tcfm_list) {
if (m->tcfm_dev == dev) {
dev_put(dev);
m->tcfm_dev = NULL;
}
}

return NOTIFY_DONE;
}

static struct notifier_block mirred_device_notifier = {
.notifier_call = mirred_device_event,
};


static struct tc_action_ops act_mirred_ops = {
.kind = "mirred",
.hinfo = &mirred_hash_info,
Expand All @@ -252,12 +284,17 @@ MODULE_LICENSE("GPL");

static int __init mirred_init_module(void)
{
int err = register_netdevice_notifier(&mirred_device_notifier);
if (err)
return err;

pr_info("Mirror/redirect action on\n");
return tcf_register_action(&act_mirred_ops);
}

static void __exit mirred_cleanup_module(void)
{
unregister_netdevice_notifier(&mirred_device_notifier);
tcf_unregister_action(&act_mirred_ops);
}

Expand Down

0 comments on commit 3b87956

Please sign in to comment.