From: Jamal Hadi Salim <jhs@mojatatu•com>
To: Cong Wang <xiyou.wangcong@gmail•com>, netdev@vger•kernel.org
Cc: "David S. Miller" <davem@davemloft•net>
Subject: Re: [PATCH net-next v4 3/8] net_sched: mirred: remove action when the target device is gone
Date: Wed, 18 Dec 2013 09:31:13 -0500 [thread overview]
Message-ID: <52B1B1B1.2060501@mojatatu.com> (raw)
In-Reply-To: <1387167311-14763-4-git-send-email-xiyou.wangcong@gmail.com>
On 12/15/13 23:15, Cong Wang wrote:
> When the target device is removed, the mirred action is
> still there but with the dev pointer setting to NULL.
> This makes the output from 'tc filter' ugly. There is no
> reason to keep it.
>
Sorry - this one i have problems with.
actions may be referenced from multiple filters,
you cant just delete it (that would leave other users
pointing to it dangling). Destroying would eventually
delete it when the refcount goes to 0.
[And when we delete actions we send netlink events to announce
that]. The proper solution would require i.e tag it as to be
deleted and implement some form of garbage collection.
Do you mind leaving this one out for this series?
cheers,
jamal
> Cc: Jamal Hadi Salim <jhs@mojatatu•com>
> Cc: David S. Miller <davem@davemloft•net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail•com>
> ---
> include/net/tc_act/tc_mirred.h | 4 +++-
> net/sched/act_mirred.c | 15 +++++++--------
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
> index cfe2943..2026cf6 100644
> --- a/include/net/tc_act/tc_mirred.h
> +++ b/include/net/tc_act/tc_mirred.h
> @@ -7,9 +7,11 @@ struct tcf_mirred {
> struct tcf_common common;
> int tcfm_eaction;
> int tcfm_ifindex;
> - int tcfm_ok_push;
> + unsigned int tcfm_ok_push:1;
> + unsigned int tcfm_bind:1;
> struct net_device *tcfm_dev;
> struct list_head tcfm_list;
> + struct tc_action *tcfm_act;
> };
> #define to_mirred(pc) \
> container_of(pc, struct tcf_mirred, common)
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index 2523781..c26bfe5 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -136,6 +136,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
> dev_hold(dev);
> m->tcfm_dev = dev;
> m->tcfm_ok_push = ok_push;
> + m->tcfm_bind = bind;
> + m->tcfm_act = a;
> }
> spin_unlock_bh(&m->tcf_lock);
> if (ret == ACT_P_CREATED) {
> @@ -169,10 +171,6 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
> bstats_update(&m->tcf_bstats, skb);
>
> dev = m->tcfm_dev;
> - if (!dev) {
> - printk_once(KERN_NOTICE "tc mirred: target device is gone\n");
> - goto out;
> - }
>
> if (!(dev->flags & IFF_UP)) {
> net_notice_ratelimited("tc mirred to Houston: device %s is down\n",
> @@ -244,13 +242,14 @@ static int mirred_device_event(struct notifier_block *unused,
> unsigned long event, void *ptr)
> {
> struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> - struct tcf_mirred *m;
> + struct tcf_mirred *m, *tmp;
>
> if (event == NETDEV_UNREGISTER)
> - list_for_each_entry(m, &mirred_list, tcfm_list) {
> + list_for_each_entry_safe(m, tmp, &mirred_list, tcfm_list) {
> if (m->tcfm_dev == dev) {
> - dev_put(dev);
> - m->tcfm_dev = NULL;
> + list_del(&m->tcfm_act->list);
> + tcf_mirred_cleanup(m->tcfm_act, m->tcfm_bind);
> + kfree(m->tcfm_act);
> }
> }
>
>
next prev parent reply other threads:[~2013-12-18 14:31 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-16 4:15 [PATCH net-next v4 0/8] net_sched: some cleanup and improvments Cong Wang
2013-12-16 4:15 ` [PATCH net-next v4 1/8] net_sched: remove get_stats from tc_action_ops Cong Wang
2013-12-18 14:14 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 2/8] net_sched: act: use standard struct list_head Cong Wang
2013-12-18 14:22 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 3/8] net_sched: mirred: remove action when the target device is gone Cong Wang
2013-12-18 14:31 ` Jamal Hadi Salim [this message]
2013-12-18 18:36 ` Cong Wang
2013-12-18 18:50 ` David Miller
2013-12-18 19:50 ` Jamal Hadi Salim
2013-12-18 21:42 ` Cong Wang
2013-12-22 16:15 ` Jamal Hadi Salim
2013-12-22 19:42 ` Cong Wang
2013-12-22 20:31 ` Jamal Hadi Salim
2013-12-22 21:11 ` Cong Wang
2013-12-23 12:41 ` Jamal Hadi Salim
2013-12-23 18:50 ` Cong Wang
2013-12-23 20:41 ` Jamal Hadi Salim
2013-12-23 22:14 ` Cong Wang
2013-12-23 22:30 ` Jamal Hadi Salim
2013-12-23 22:51 ` Cong Wang
2013-12-23 23:05 ` Jamal Hadi Salim
2013-12-23 23:14 ` Cong Wang
2013-12-16 4:15 ` [PATCH net-next v4 4/8] net_sched: cls: refactor out struct tcf_ext_map Cong Wang
2013-12-18 14:36 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 5/8] net_sched: init struct tcf_hashinfo at register time Cong Wang
2013-12-18 14:37 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 6/8] net_sched: convert tcf_hashinfo to hlist and use spinlock Cong Wang
2013-12-18 14:38 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 7/8] net_sched: convert tc_action_ops to use struct list_head Cong Wang
2013-12-18 14:40 ` Jamal Hadi Salim
2013-12-16 4:15 ` [PATCH net-next v4 8/8] net_sched: convert tcf_proto_ops " Cong Wang
2013-12-18 14:41 ` Jamal Hadi Salim
2013-12-16 12:45 ` [PATCH net-next v4 0/8] net_sched: some cleanup and improvments Jamal Hadi Salim
2013-12-17 21:30 ` David Miller
2013-12-18 13:21 ` Jamal Hadi Salim
2013-12-18 14:13 ` Jamal Hadi Salim
2013-12-18 18:51 ` David Miller
2013-12-18 21:40 ` Cong Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52B1B1B1.2060501@mojatatu.com \
--to=jhs@mojatatu$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=xiyou.wangcong@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox