[Patch net-next 0/4] net_sched: improve tc action code

[Patch net-next 0/4] net_sched: improve tc action code

[Cong Wang]

This patchset continues to cleanup and improve tc action
code, especially for mirred action we are now able to cleanup
the action upon device unregister event.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Cong Wang (4):
  net_sched: act: clean up notification functions
  net_sched: act: move idx_gen to struct tcf_hash_info
  net_sched: mirred: remove action when the target device is gone
  net_sched: mirred: notify user-space when action gets removed

 include/net/act_api.h          |   8 +--
 include/net/tc_act/tc_mirred.h |   1 +
 net/sched/act_api.c            | 110 +++++++++++++++++------------------------
 net/sched/act_csum.c           |   3 +-
 net/sched/act_gact.c           |   3 +-
 net/sched/act_ipt.c            |   3 +-
 net/sched/act_mirred.c         |  31 +++++++-----
 net/sched/act_nat.c            |   3 +-
 net/sched/act_pedit.c          |   3 +-
 net/sched/act_police.c         |   3 +-
 net/sched/act_simple.c         |   3 +-
 net/sched/act_skbedit.c        |   3 +-
 12 files changed, 80 insertions(+), 94 deletions(-)

-- 
1.8.3.1

[Patch net-next 1/4] net_sched: act: clean up notification functions

[Cong Wang]

Refactor tcf_add_notify() and factor out tcf_del_notify().

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/act_api.c | 99 ++++++++++++++++++++++-------------------------------
 1 file changed, 40 insertions(+), 59 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 8114fef..c83c799 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -833,6 +833,33 @@ noflush_out:
 }
 
 static int
+tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
+	       u32 portid)
+{
+	int ret;
+	struct sk_buff *skb;
+
+	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOBUFS;
+
+	if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
+			 0, 1) <= 0) {
+		kfree_skb(skb);
+		return -EINVAL;
+	}
+
+	/* now do the delete */
+	tcf_action_destroy(actions, 0);
+
+	ret = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
+			     n->nlmsg_flags & NLM_F_ECHO);
+	if (ret > 0)
+		return 0;
+	return ret;
+}
+
+static int
 tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
 	      u32 portid, int event)
 {
@@ -864,89 +891,43 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
 
 	if (event == RTM_GETACTION)
 		ret = act_get_notify(net, portid, n, &actions, event);
-	else { /* delete */
-		struct sk_buff *skb;
-
-		skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
-		if (!skb) {
-			ret = -ENOBUFS;
-			goto err;
-		}
-
-		if (tca_get_fill(skb, &actions, portid, n->nlmsg_seq, 0, event,
-				 0, 1) <= 0) {
-			kfree_skb(skb);
-			ret = -EINVAL;
-			goto err;
-		}
-
-		/* now do the delete */
-		tcf_action_destroy(&actions, 0);
-		ret = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
-				     n->nlmsg_flags & NLM_F_ECHO);
-		if (ret > 0)
-			return 0;
-		return ret;
-	}
+	else /* delete */
+		ret = tcf_del_notify(net, n, &actions, portid);
 err:
 	cleanup_a(&actions);
 	return ret;
 }
 
-static int tcf_add_notify(struct net *net, struct list_head *actions,
-			  u32 portid, u32 seq, int event, u16 flags)
+static int
+tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
+	       u32 portid)
 {
-	struct tcamsg *t;
-	struct nlmsghdr *nlh;
 	struct sk_buff *skb;
-	struct nlattr *nest;
-	unsigned char *b;
 	int err = 0;
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (!skb)
 		return -ENOBUFS;
 
-	b = skb_tail_pointer(skb);
-
-	nlh = nlmsg_put(skb, portid, seq, event, sizeof(*t), flags);
-	if (!nlh)
-		goto out_kfree_skb;
-	t = nlmsg_data(nlh);
-	t->tca_family = AF_UNSPEC;
-	t->tca__pad1 = 0;
-	t->tca__pad2 = 0;
-
-	nest = nla_nest_start(skb, TCA_ACT_TAB);
-	if (nest == NULL)
-		goto out_kfree_skb;
-
-	if (tcf_action_dump(skb, actions, 0, 0) < 0)
-		goto out_kfree_skb;
-
-	nla_nest_end(skb, nest);
-
-	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
-	NETLINK_CB(skb).dst_group = RTNLGRP_TC;
+	if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
+			 RTM_NEWACTION, 0, 0) <= 0) {
+		kfree_skb(skb);
+		return -EINVAL;
+	}
 
-	err = rtnetlink_send(skb, net, portid, RTNLGRP_TC, flags & NLM_F_ECHO);
+	err = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
+			     n->nlmsg_flags & NLM_F_ECHO);
 	if (err > 0)
 		err = 0;
 	return err;
-
-out_kfree_skb:
-	kfree_skb(skb);
-	return -1;
 }
 
-
 static int
 tcf_action_add(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
 	       u32 portid, int ovr)
 {
 	int ret = 0;
 	LIST_HEAD(actions);
-	u32 seq = n->nlmsg_seq;
 
 	ret = tcf_action_init(net, nla, NULL, NULL, ovr, 0, &actions);
 	if (ret)
@@ -955,7 +936,7 @@ tcf_action_add(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
 	/* dump then free all the actions after update; inserted policy
 	 * stays intact
 	 */
-	ret = tcf_add_notify(net, &actions, portid, seq, RTM_NEWACTION, n->nlmsg_flags);
+	ret = tcf_add_notify(net, n, &actions, portid);
 	cleanup_a(&actions);
 done:
 	return ret;
-- 
1.8.3.1

[Patch net-next 2/4] net_sched: act: move idx_gen to struct tcf_hashinfo

[Cong Wang]

There is no need to store the index separatedly
since tcf_hashinfo is allocated statically too.

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/act_api.h   |  7 ++++---
 net/sched/act_api.c     | 10 +++++-----
 net/sched/act_csum.c    |  3 +--
 net/sched/act_gact.c    |  3 +--
 net/sched/act_ipt.c     |  3 +--
 net/sched/act_mirred.c  |  3 +--
 net/sched/act_nat.c     |  3 +--
 net/sched/act_pedit.c   |  3 +--
 net/sched/act_police.c  |  3 +--
 net/sched/act_simple.c  |  3 +--
 net/sched/act_skbedit.c |  3 +--
 11 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 77d5d81..6b697c0 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -39,6 +39,7 @@ struct tcf_hashinfo {
 	struct hlist_head	*htab;
 	unsigned int		hmask;
 	spinlock_t		lock;
+	u32			index;
 };
 
 static inline unsigned int tcf_hash(u32 index, unsigned int hmask)
@@ -51,6 +52,7 @@ static inline int tcf_hashinfo_init(struct tcf_hashinfo *hf, unsigned int mask)
 	int i;
 
 	spin_lock_init(&hf->lock);
+	hf->index = 0;
 	hf->hmask = mask;
 	hf->htab = kzalloc((mask + 1) * sizeof(struct hlist_head),
 			   GFP_KERNEL);
@@ -107,14 +109,13 @@ int tcf_hash_release(struct tcf_common *p, int bind,
 		     struct tcf_hashinfo *hinfo);
 int tcf_generic_walker(struct sk_buff *skb, struct netlink_callback *cb,
 		       int type, struct tc_action *a);
-u32 tcf_hash_new_index(u32 *idx_gen, struct tcf_hashinfo *hinfo);
+u32 tcf_hash_new_index(struct tcf_hashinfo *hinfo);
 int tcf_hash_search(struct tc_action *a, u32 index);
 struct tcf_common *tcf_hash_check(u32 index, struct tc_action *a,
 				  int bind, struct tcf_hashinfo *hinfo);
 struct tcf_common *tcf_hash_create(u32 index, struct nlattr *est,
 				   struct tc_action *a, int size,
-				   int bind, u32 *idx_gen,
-				   struct tcf_hashinfo *hinfo);
+				   int bind, struct tcf_hashinfo *hinfo);
 void tcf_hash_insert(struct tcf_common *p, struct tcf_hashinfo *hinfo);
 
 int tcf_register_action(struct tc_action_ops *a);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index c83c799..e619982 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -173,16 +173,16 @@ struct tcf_common *tcf_hash_lookup(u32 index, struct tcf_hashinfo *hinfo)
 }
 EXPORT_SYMBOL(tcf_hash_lookup);
 
-u32 tcf_hash_new_index(u32 *idx_gen, struct tcf_hashinfo *hinfo)
+u32 tcf_hash_new_index(struct tcf_hashinfo *hinfo)
 {
-	u32 val = *idx_gen;
+	u32 val = hinfo->index;
 
 	do {
 		if (++val == 0)
 			val = 1;
 	} while (tcf_hash_lookup(val, hinfo));
 
-	*idx_gen = val;
+	hinfo->index = val;
 	return val;
 }
 EXPORT_SYMBOL(tcf_hash_new_index);
@@ -216,7 +216,7 @@ EXPORT_SYMBOL(tcf_hash_check);
 
 struct tcf_common *tcf_hash_create(u32 index, struct nlattr *est,
 				   struct tc_action *a, int size, int bind,
-				   u32 *idx_gen, struct tcf_hashinfo *hinfo)
+				   struct tcf_hashinfo *hinfo)
 {
 	struct tcf_common *p = kzalloc(size, GFP_KERNEL);
 
@@ -228,7 +228,7 @@ struct tcf_common *tcf_hash_create(u32 index, struct nlattr *est,
 
 	spin_lock_init(&p->tcfc_lock);
 	INIT_HLIST_NODE(&p->tcfc_head);
-	p->tcfc_index = index ? index : tcf_hash_new_index(idx_gen, hinfo);
+	p->tcfc_index = index ? index : tcf_hash_new_index(hinfo);
 	p->tcfc_tm.install = jiffies;
 	p->tcfc_tm.lastuse = jiffies;
 	if (est) {
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 5d350c5..4784761 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -37,7 +37,6 @@
 #include <net/tc_act/tc_csum.h>
 
 #define CSUM_TAB_MASK 15
-static u32 csum_idx_gen;
 static struct tcf_hashinfo csum_hash_info;
 
 static const struct nla_policy csum_policy[TCA_CSUM_MAX + 1] = {
@@ -67,7 +66,7 @@ static int tcf_csum_init(struct net *n, struct nlattr *nla, struct nlattr *est,
 	pc = tcf_hash_check(parm->index, a, bind, &csum_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*p), bind,
-				     &csum_idx_gen, &csum_hash_info);
+				     &csum_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		p = to_tcf_csum(pc);
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 1e6e0e7..63cf32c 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -24,7 +24,6 @@
 #include <net/tc_act/tc_gact.h>
 
 #define GACT_TAB_MASK	15
-static u32 gact_idx_gen;
 static struct tcf_hashinfo gact_hash_info;
 
 #ifdef CONFIG_GACT_PROB
@@ -90,7 +89,7 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
 	pc = tcf_hash_check(parm->index, a, bind, &gact_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*gact),
-				     bind, &gact_idx_gen, &gact_hash_info);
+				     bind, &gact_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		ret = ACT_P_CREATED;
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 8344380..b11a7ff 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -29,7 +29,6 @@
 
 
 #define IPT_TAB_MASK     15
-static u32 ipt_idx_gen;
 static struct tcf_hashinfo ipt_hash_info;
 
 static int ipt_init_target(struct xt_entry_target *t, char *table, unsigned int hook)
@@ -129,7 +128,7 @@ static int tcf_ipt_init(struct net *net, struct nlattr *nla, struct nlattr *est,
 	pc = tcf_hash_check(index, a, bind, &ipt_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(index, est, a, sizeof(*ipt), bind,
-				     &ipt_idx_gen, &ipt_hash_info);
+				     &ipt_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		ret = ACT_P_CREATED;
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 199fc98..a6a0af9 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -30,7 +30,6 @@
 #include <linux/if_arp.h>
 
 #define MIRRED_TAB_MASK     7
-static u32 mirred_idx_gen;
 static LIST_HEAD(mirred_list);
 static struct tcf_hashinfo mirred_hash_info;
 
@@ -107,7 +106,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 		if (dev == NULL)
 			return -EINVAL;
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*m), bind,
-				     &mirred_idx_gen, &mirred_hash_info);
+				     &mirred_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		ret = ACT_P_CREATED;
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 409fe71..f717d46 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -30,7 +30,6 @@
 
 
 #define NAT_TAB_MASK	15
-static u32 nat_idx_gen;
 
 static struct tcf_hashinfo nat_hash_info;
 
@@ -61,7 +60,7 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
 	pc = tcf_hash_check(parm->index, a, bind, &nat_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*p), bind,
-				     &nat_idx_gen, &nat_hash_info);
+				     &nat_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		p = to_tcf_nat(pc);
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index aa5347c..f325a24 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -24,7 +24,6 @@
 #include <net/tc_act/tc_pedit.h>
 
 #define PEDIT_TAB_MASK	15
-static u32 pedit_idx_gen;
 
 static struct tcf_hashinfo pedit_hash_info;
 
@@ -63,7 +62,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
 		if (!parm->nkeys)
 			return -EINVAL;
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*p), bind,
-				     &pedit_idx_gen, &pedit_hash_info);
+				     &pedit_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		p = to_pedit(pc);
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 0cc305e..375aa16 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -41,7 +41,6 @@ struct tcf_police {
 	container_of(pc, struct tcf_police, common)
 
 #define POL_TAB_MASK     15
-static u32 police_idx_gen;
 static struct tcf_hashinfo police_hash_info;
 
 /* old policer structure from before tc actions */
@@ -249,7 +248,7 @@ override:
 
 	police->tcfp_t_c = ktime_to_ns(ktime_get());
 	police->tcf_index = parm->index ? parm->index :
-		tcf_hash_new_index(&police_idx_gen, &police_hash_info);
+		tcf_hash_new_index(&police_hash_info);
 	h = tcf_hash(police->tcf_index, POL_TAB_MASK);
 	spin_lock_bh(&police_hash_info.lock);
 	hlist_add_head(&police->tcf_head, &police_hash_info.htab[h]);
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 2d7a0eb..cc58814 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -25,7 +25,6 @@
 #include <net/tc_act/tc_defact.h>
 
 #define SIMP_TAB_MASK     7
-static u32 simp_idx_gen;
 static struct tcf_hashinfo simp_hash_info;
 
 #define SIMP_MAX_DATA	32
@@ -118,7 +117,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
 	pc = tcf_hash_check(parm->index, a, bind, &simp_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*d), bind,
-				     &simp_idx_gen, &simp_hash_info);
+				     &simp_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 90ed04a..9b0b75a 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -28,7 +28,6 @@
 #include <net/tc_act/tc_skbedit.h>
 
 #define SKBEDIT_TAB_MASK     15
-static u32 skbedit_idx_gen;
 static struct tcf_hashinfo skbedit_hash_info;
 
 static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
@@ -104,7 +103,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 	pc = tcf_hash_check(parm->index, a, bind, &skbedit_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*d), bind,
-				     &skbedit_idx_gen, &skbedit_hash_info);
+				     &skbedit_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 
-- 
1.8.3.1

[Patch net-next 3/4] net_sched: mirred: remove action when the target device is gone

[Cong Wang]

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.

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 |  1 +
 net/sched/act_mirred.c         | 27 ++++++++++++++++++---------
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index cfe2943..374be24 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -10,6 +10,7 @@ struct tcf_mirred {
 	int			tcfm_ok_push;
 	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 a6a0af9..33a4893 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -128,6 +128,7 @@ 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_act = a;
 	}
 	spin_unlock_bh(&m->tcf_lock);
 	if (ret == ACT_P_CREATED) {
@@ -161,10 +162,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",
@@ -236,15 +233,27 @@ 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) {
+	if (event == NETDEV_UNREGISTER) {
+		list_for_each_entry_safe(m, tmp, &mirred_list, tcfm_list) {
 			if (m->tcfm_dev == dev) {
-				dev_put(dev);
-				m->tcfm_dev = NULL;
+				struct tcf_common *p = NULL;
+				u32 index = m->common.tcfc_index;
+
+				list_del(&m->tcfm_act->list);
+				while ((p = tcf_hash_lookup(index, &mirred_hash_info)) != NULL) {
+					list_del_init(&m->tcfm_list);
+					if (m->tcfm_dev) {
+						dev_put(m->tcfm_dev);
+						m->tcfm_dev = NULL;
+					}
+					tcf_hash_destroy(p, &mirred_hash_info);
+				}
+				kfree(m->tcfm_act);
 			}
 		}
+	}
 
 	return NOTIFY_DONE;
 }
-- 
1.8.3.1

[Patch net-next 4/4] net_sched: mirred: notify user-space when action gets removed

[Cong Wang]

When the mirred action gets removed on device unregister event,
we still need to notify the user-space via a netlink message.
This patch adjusts tcf_del_notify() so that it can be called
in device unregister event notification.

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/act_api.h  | 1 +
 net/sched/act_api.c    | 9 +++++----
 net/sched/act_mirred.c | 5 +++--
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 6b697c0..cd82656 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -133,5 +133,6 @@ int tcf_action_dump(struct sk_buff *skb, struct list_head *, int, int);
 int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
 int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
 int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int);
+int tcf_del_notify(struct net *, struct nlmsghdr *, struct list_head *, u32);
 #endif /* CONFIG_NET_CLS_ACT */
 #endif
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index e619982..69bc29c 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -832,7 +832,7 @@ noflush_out:
 	return err;
 }
 
-static int
+int
 tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
 	       u32 portid)
 {
@@ -843,8 +843,8 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
 	if (!skb)
 		return -ENOBUFS;
 
-	if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
-			 0, 1) <= 0) {
+	if (tca_get_fill(skb, actions, portid, n ? n->nlmsg_seq : 0, 0,
+			 RTM_DELACTION, 0, 1) <= 0) {
 		kfree_skb(skb);
 		return -EINVAL;
 	}
@@ -853,11 +853,12 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
 	tcf_action_destroy(actions, 0);
 
 	ret = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
-			     n->nlmsg_flags & NLM_F_ECHO);
+			     n ? (n->nlmsg_flags & NLM_F_ECHO) : 0);
 	if (ret > 0)
 		return 0;
 	return ret;
 }
+EXPORT_SYMBOL(tcf_del_notify);
 
 static int
 tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 33a4893..6bf5993 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -234,6 +234,7 @@ static int mirred_device_event(struct notifier_block *unused,
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 	struct tcf_mirred *m, *tmp;
+	LIST_HEAD(local);
 
 	if (event == NETDEV_UNREGISTER) {
 		list_for_each_entry_safe(m, tmp, &mirred_list, tcfm_list) {
@@ -241,7 +242,7 @@ static int mirred_device_event(struct notifier_block *unused,
 				struct tcf_common *p = NULL;
 				u32 index = m->common.tcfc_index;
 
-				list_del(&m->tcfm_act->list);
+				list_move(&m->tcfm_act->list, &local);
 				while ((p = tcf_hash_lookup(index, &mirred_hash_info)) != NULL) {
 					list_del_init(&m->tcfm_list);
 					if (m->tcfm_dev) {
@@ -250,7 +251,7 @@ static int mirred_device_event(struct notifier_block *unused,
 					}
 					tcf_hash_destroy(p, &mirred_hash_info);
 				}
-				kfree(m->tcfm_act);
+				tcf_del_notify(dev_net(dev), NULL, &local, 0);
 			}
 		}
 	}
-- 
1.8.3.1

Re: [Patch net-next 1/4] net_sched: act: clean up notification functions

[Cong Wang]

On Wed, Dec 18, 2013 at 4:36 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> -               if (ret > 0)
> -                       return 0;
> -               return ret;
> -       }
> +       else /* delete */
> +               ret = tcf_del_notify(net, n, &actions, portid);

This piece doesn't look correct. :-) I will fix it.

Re: [Patch net-next 1/4] net_sched: act: clean up notification functions

[Jamal Hadi Salim]

On 12/18/13 19:48, Cong Wang wrote:
> On Wed, Dec 18, 2013 at 4:36 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> -               if (ret > 0)
>> -                       return 0;
>> -               return ret;
>> -       }
>> +       else /* delete */
>> +               ret = tcf_del_notify(net, n, &actions, portid);
>
> This piece doesn't look correct. :-) I will fix it.
>

It is not as trivial and i dont think you have the correct tests to
vet it.
Please give me until this weekend to test your other changes
that are already in.
I also had some changes i wanted to get in. Then I will review
and maybe test this as well...

cheers,
jamal

Re: [Patch net-next 1/4] net_sched: act: clean up notification functions

[Cong Wang]

On Thu, Dec 19, 2013 at 6:40 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 12/18/13 19:48, Cong Wang wrote:
>>
>> On Wed, Dec 18, 2013 at 4:36 PM, Cong Wang <xiyou.wangcong@gmail.com>
>> wrote:
>>>
>>> -               if (ret > 0)
>>> -                       return 0;
>>> -               return ret;
>>> -       }
>>> +       else /* delete */
>>> +               ret = tcf_del_notify(net, n, &actions, portid);
>>
>>
>> This piece doesn't look correct. :-) I will fix it.
>>
>
> It is not as trivial and i dont think you have the correct tests to
> vet it.
> Please give me until this weekend to test your other changes
> that are already in.
> I also had some changes i wanted to get in. Then I will review
> and maybe test this as well...


Sure. Meanwhile, I will test this action deletion case and send
the updated patches for you to test in the weekend.

Thanks!