[NET]: Fix module refcounting of TC actions.

include/linux/pkt_cls.h

@@ -71,16 +71,14 @@ bits 9,10,11: redirect counter -  redirect TTL. Loop avoidance
 #define V_TC_AT(x)       _TC_MAKEVALUE(x,S_TC_AT)
 #define SET_TC_AT(v,n)   ((V_TC_AT(n)) | (v & ~M_TC_AT))
 
-/* Action types */
+/* Action attributes */
 enum
 {
-	TCA_ACT_UNSPEC=0,
-	TCA_ACT_KIND=1,
-	TCA_ACT_OPTIONS=2,
-	TCA_ACT_INDEX=3,
-	TCA_ACT_POLICE=4,
-	/* other actions go here */
-	__TCA_ACT_MAX=255
+	TCA_ACT_UNSPEC,
+	TCA_ACT_KIND,
+	TCA_ACT_OPTIONS,
+	TCA_ACT_INDEX,
+	__TCA_ACT_MAX
 };
 
 #define TCA_ACT_MAX __TCA_ACT_MAX
@@ -105,6 +103,17 @@ enum
 #define TC_ACT_REPEAT		6
 #define TC_ACT_JUMP		0x10000000
 
+/* Action type identifiers*/
+enum
+{
+	TCA_ID_UNSPEC=0,
+	TCA_ID_POLICE=1,
+	/* other actions go here */
+	__TCA_ID_MAX=255
+};
+
+#define TCA_ID_MAX __TCA_ID_MAX
+
 struct tc_police
 {
 	__u32			index;
@@ -213,7 +222,9 @@ struct tc_u32_sel
 	unsigned char		flags;
 	unsigned char		offshift;
 	unsigned char		nkeys;
+#ifdef fix_u32_bug
 	unsigned char		fshift;  /* fold shift */
+#endif
 
 	__u16			offmask;
 	__u16			off;

include/linux/pkt_sched.h

@@ -37,7 +37,13 @@ struct tc_stats
 	__u32	bps;			/* Current flow byte rate */
 	__u32	pps;			/* Current flow packet rate */
 	__u32	qlen;
+#ifdef CONFIG_NET_CLS_ACT
+/* eventually remove the define here; adding this(useful) 
+field at least fixes the 8 byte layout problems we 
+have with MIPS and PPC because we have a u64
+*/
 	__u32	reqs;			/* number of requeues happened */
+#endif
 	__u32	backlog;
 #ifdef __KERNEL__
 	spinlock_t *lock;

include/net/pkt_act.h

@@ -69,20 +69,21 @@ tcf_hash_destroy(struct tcf_st *p)
 	BUG_TRAP(0);
 }
 
-static inline void
+static inline int
 tcf_hash_release(struct tcf_st *p, int bind )
 {
+	int ret = 0;
 	if (p) {
 		if (bind) {
 			p->bindcnt--;
 		}
 		p->refcnt--;
-		if (p->refcnt > 0)
-			MOD_DEC_USE_COUNT;
 	       	if(p->bindcnt <=0 && p->refcnt <= 0) {
 			tcf_hash_destroy(p);
+			ret = 1;
 		}
 	}
+	return ret;
 }
 
 static __inline__ int
@@ -117,7 +118,6 @@ tcf_dump_walker(struct sk_buff *skb, struct netlink_callback *cb,
 			r->rta_len = skb->tail - (u8*)r;
 			n_i++;
 			if (n_i >= TCA_ACT_MAX_PRIO) {
-				printk("Jamal Dump Exceeded batch limit\n");
 				goto done;
 			}
 		}
@@ -148,8 +148,9 @@ tcf_del_walker(struct sk_buff *skb, struct tc_action *a)
 
 		while (p != NULL) {
 			s_p = p->next;
-			printk("tcf_del_walker deleting ..\n");
-			tcf_hash_release(p, 0);
+			if (ACT_P_DELETED == tcf_hash_release(p, 0)) {
+				 module_put(a->ops->owner);
+			}
 			n_i++;
 			p = s_p;
 		}
@@ -250,7 +251,6 @@ tcf_hash_create(struct tc_st *parm, struct rtattr *est, struct tc_action *a, int
 		p->bindcnt = 1;
 	}
 
-	MOD_INC_USE_COUNT;
 	spin_lock_init(&p->lock);
 	p->stats.lock = &p->lock;
 	p->index = parm->index ? : tcf_hash_new_index();

include/net/pkt_sched.h

@@ -414,6 +414,8 @@ struct tcf_police
 
 #ifdef CONFIG_NET_CLS_ACT
 
+#define ACT_P_CREATED 1
+#define ACT_P_DELETED 1
 #define tca_gen(name) \
 struct tcf_##name *next; \
 	u32 index; \
@@ -442,10 +444,11 @@ struct tc_action_ops
 	char    kind[IFNAMSIZ];
 	__u32   type; /* TBD to match kind */
 	__u32 	capab;  /* capabilities includes 4 bit version */
+	struct module		*owner;
 	int     (*act)(struct sk_buff **, struct tc_action *);
 	int     (*get_stats)(struct sk_buff *, struct tc_action *);
 	int     (*dump)(struct sk_buff *, struct tc_action *,int , int);
-	void     (*cleanup)(struct tc_action *, int bind);
+	int     (*cleanup)(struct tc_action *, int bind);
 	int     (*lookup)(struct tc_action *, u32 );
 	int     (*init)(struct rtattr *,struct rtattr *,struct tc_action *, int , int );
 	int     (*walk)(struct sk_buff *, struct netlink_callback *, int , struct tc_action *);
@@ -472,24 +475,26 @@ extern void tcf_police_destroy(struct tcf_police *p);
 extern struct tcf_police * tcf_police_locate(struct rtattr *rta, struct rtattr *est);
 extern int tcf_police_dump(struct sk_buff *skb, struct tcf_police *p);
 
-static inline void tcf_police_release(struct tcf_police *p, int bind)
+static inline int tcf_police_release(struct tcf_police *p, int bind)
 {
+	int ret = 0;
 #ifdef CONFIG_NET_CLS_ACT
 	if (p) {
 		if (bind) {
 			 p->bindcnt--;
 		}
 		p->refcnt--;
-		if (p->refcnt > 0)
-			MOD_DEC_USE_COUNT;
-		if (p->refcnt <= 0 && !p->bindcnt)
+		if (p->refcnt <= 0 && !p->bindcnt) {
 			tcf_police_destroy(p);
+			ret = 1;
+		}
 	}
 #else
 	if (p && --p->refcnt == 0)
 		tcf_police_destroy(p);
 
 #endif
+	return ret;
 }
 
 extern struct Qdisc noop_qdisc;

net/sched/Kconfig

@@ -280,6 +280,7 @@ config CLS_U32_PERF
 	help
 	  gathers stats that could be used to tune u32 classifier perfomance.
 	  Requires a new iproute2
+	  You MUST NOT turn this on if you dont have an update iproute2.
 
 config NET_CLS_IND
 	bool "classify input device (slows things u32/fw) "
@@ -289,6 +290,7 @@ config NET_CLS_IND
           metadata action appears because it slows things a little
           Available only for u32 and fw classifiers.
 	  Requires a new iproute2
+	  You MUST NOT turn this on if you dont have an update iproute2.
 
 config NET_CLS_RSVP
 	tristate "Special RSVP classifier"
@@ -321,22 +323,24 @@ config NET_CLS_RSVP6
 
 config NET_CLS_ACT
 	bool '  Packet ACTION ' 
-	depends on NET_CLS && NET_QOS
+	depends on EXPERIMENTAL && NET_CLS && NET_QOS
 	---help---
 	This option requires you have a new iproute2. It enables
 	tc extensions which can be used with tc classifiers.
 	Only the u32 and fw classifiers are supported at the moment.
+	  You MUST NOT turn this on if you dont have an update iproute2.
 
 config NET_ACT_POLICE
 	tristate '      Policing Actions' 
-        depends on NET_CLS_ACT
+        depends on NET_CLS_ACT 
         ---help---
         If you are using a newer iproute2 select this one, otherwise use one
-	NET_CLS_POLICE below
+	below to select a policer.
+	  You MUST NOT turn this on if you dont have an update iproute2.
 
 config NET_CLS_POLICE
 	bool "Traffic policing (needed for in/egress)"
-	depends on NET_CLS && NET_QOS && !NET_ACT_POLICE
+	depends on NET_CLS && NET_QOS && NET_ACT_POLICE!=y && NET_ACT_POLICE!=m
 	help
 	  Say Y to support traffic policing (bandwidth limits).  Needed for
 	  ingress and egress rate limiting.

net/sched/act_api.c

@@ -48,11 +48,12 @@ int tcf_register_action(struct tc_action_ops *act)
 
 	write_lock(&act_mod_lock);
 	for (ap = &act_base; (a=*ap)!=NULL; ap = &a->next) {
-		if (strcmp(act->kind, a->kind) == 0) {
+		if (act->type == a->type || (strcmp(act->kind, a->kind) == 0)) {
 			write_unlock(&act_mod_lock);
 			return -EEXIST;
 		}
 	}
+
         act->next = NULL;
 	*ap = act;
 
@@ -89,8 +90,13 @@ struct tc_action_ops *tc_lookup_action_n(char *kind)
 	if (kind) {
 		read_lock(&act_mod_lock);
 		for (a = act_base; a; a = a->next) {
-			if (strcmp(kind,a->kind) == 0) 
+			if (strcmp(kind,a->kind) == 0) {
+				if (!try_module_get(a->owner)) {
+					read_unlock(&act_mod_lock);
+					return NULL;
+				} 
 				break;
+			}
 		}
 		read_unlock(&act_mod_lock);
 	}
@@ -108,8 +114,13 @@ struct tc_action_ops *tc_lookup_action(struct rtattr *kind)
 		read_lock(&act_mod_lock);
 		for (a = act_base; a; a = a->next) {
 
-			if (strcmp((char*)RTA_DATA(kind),a->kind) == 0) 
+			if (strcmp((char*)RTA_DATA(kind),a->kind) == 0){
+				if (!try_module_get(a->owner)) {
+					read_unlock(&act_mod_lock);
+					return NULL;
+				} 
 				break;
+			}
 		}
 		read_unlock(&act_mod_lock);
 	}
@@ -125,8 +136,13 @@ struct tc_action_ops *tc_lookup_action_id(u32 type)
 	if (type) {
 		read_lock(&act_mod_lock);
 		for (a = act_base; a; a = a->next) {
-			if (a->type == type) 
+			if (a->type == type) {
+				if (!try_module_get(a->owner)) {
+					read_unlock(&act_mod_lock);
+					return NULL;
+				} 
 				break;
+			}
 		}
 		read_unlock(&act_mod_lock);
 	}
@@ -177,7 +193,10 @@ void tcf_action_destroy(struct tc_action *act, int bind)
 		if (a && a->ops && a->ops->cleanup) {
 			DPRINTK("tcf_action_destroy destroying %p next %p\n", a,a->next?a->next:NULL);
 			act = act->next;
-			a->ops->cleanup(a, bind);
+			if (ACT_P_DELETED == a->ops->cleanup(a, bind)) {
+				module_put(a->ops->owner);
+			}
+			
 			a->ops = NULL;  
 			kfree(a);
 		} else { /*FIXME: Remove later - catch insertion bugs*/
@@ -270,7 +289,6 @@ int tcf_action_init_1(struct rtattr *rta, struct rtattr *est, struct tc_action *
 
 	int err = -EINVAL;
 
-
 	if (NULL == name) {
 		if (rtattr_parse(tb, TCA_ACT_MAX, RTA_DATA(rta), RTA_PAYLOAD(rta))<0)
 			goto err_out;
@@ -306,27 +324,37 @@ int tcf_action_init_1(struct rtattr *rta, struct rtattr *est, struct tc_action *
 	}
 
 	if (NULL == a) {
-		goto err_out;
+		goto err_mod;
 	}
 
 	/* backward compatibility for policer */
 	if (NULL == name) {
 		err = a_o->init(tb[TCA_ACT_OPTIONS-1], est, a, ovr, bind);
 		if (0 > err ) {
-			return -EINVAL;
+			err = -EINVAL;
+			goto err_mod;
 		}
 	} else {
 		err = a_o->init(rta, est, a, ovr, bind);
 		if (0 > err ) {
-			return -EINVAL;
+			err = -EINVAL;
+			goto err_mod;
 		}
 	}
 
-	DPRINTK("tcf_action_init_1: sucess  %s\n",act_name);
-
+	/* module count goes up only when brand new policy is created
+	   if it exists and is only bound to in a_o->init() then
+           ACT_P_CREATED is not returned (a zero is).
+        */
+	if (ACT_P_CREATED != err) {
+		module_put(a_o->owner);
+	} 
 	a->ops = a_o;
+	DPRINTK("tcf_action_init_1: successfull %s \n",act_name);
 
 	return 0;
+err_mod:
+	module_put(a_o->owner);
 err_out:
 	return err;
 }
@@ -350,7 +378,7 @@ int tcf_action_init(struct rtattr *rta, struct rtattr *est, struct tc_action *a,
 					err = -ENOMEM;
 					goto bad_ret;
 				}
-				 memset(act, 0,sizeof(*act));
+				memset(act, 0,sizeof(*act));
 			}
 			act->next = NULL;
 			if (0 > tcf_action_init_1(tb[i],est,act,name,ovr,bind)) {
@@ -489,15 +517,21 @@ int tcf_action_get_1(struct rtattr *rta, struct tc_action *a, struct nlmsghdr *n
 	}
 
 	if (NULL == a) {
-		goto err_out;
+		goto err_mod;
 	}
 
 	a->ops = a_o;
 
-	if (NULL == a_o->lookup || 0 == a_o->lookup(a, index))
-		return -EINVAL;
+	if (NULL == a_o->lookup || 0 == a_o->lookup(a, index)) {
+		a->ops = NULL;
+		err = -EINVAL;
+		goto err_mod;
+	}
 
+	module_put(a_o->owner);
 	return 0;
+err_mod:
+	module_put(a_o->owner);
 err_out:
 	return err;
 }
@@ -621,6 +655,7 @@ int tca_action_flush(struct rtattr *rta, struct nlmsghdr *n, u32 pid)
 
 	nlh->nlmsg_len = skb->tail - b;
 	nlh->nlmsg_flags |= NLM_F_ROOT;
+	module_put(a->ops->owner);
 	kfree(a);
 	err = rtnetlink_send(skb, pid, RTMGRP_TC, n->nlmsg_flags&NLM_F_ECHO);
 	if (err > 0)
@@ -630,6 +665,7 @@ int tca_action_flush(struct rtattr *rta, struct nlmsghdr *n, u32 pid)
 
 
 rtattr_failure:
+	module_put(a->ops->owner);
 nlmsg_failure:
 err_out:
 	kfree_skb(skb);
@@ -852,7 +888,7 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 char *
 find_dump_kind(struct nlmsghdr *n)
 {
-	struct rtattr *tb1, *tb2[TCA_ACT_MAX_PRIO+1];
+	struct rtattr *tb1, *tb2[TCA_ACT_MAX+1];
 	struct rtattr *tb[TCA_ACT_MAX_PRIO + 1];
 	struct rtattr *rta[TCAA_MAX + 1];
 	struct rtattr *kind = NULL;
@@ -941,10 +977,12 @@ tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
 	nlh->nlmsg_len = skb->tail - b;
 	if (NETLINK_CB(cb->skb).pid && ret) 
 		nlh->nlmsg_flags |= NLM_F_MULTI;
+	module_put(a_o->owner);
 	return skb->len;
 
 rtattr_failure:
 nlmsg_failure:
+	module_put(a_o->owner);
 	skb_trim(skb, b - skb->data);
 	return skb->len;
 }

net/sched/cls_u32.c

@@ -101,12 +101,14 @@ static struct tc_u_common *u32_list;
 
 static __inline__ unsigned u32_hash_fold(u32 key, struct tc_u32_sel *sel)
 {
+#ifdef fix_u32_bug
 	unsigned h = (key & sel->hmask)>>sel->fshift;
+#else
+	unsigned h = (key & sel->hmask);
 
-	/*
 	h ^= h>>16;
 	h ^= h>>8;
-	*/
+#endif
 	return h;
 }
 
@@ -684,6 +686,14 @@ static int u32_change(struct tcf_proto *tp, unsigned long base, u32 handle,
 		return -EINVAL;
 
 	s = RTA_DATA(tb[TCA_U32_SEL-1]);
+
+#ifdef CONFIG_CLS_U32_PERF
+	if (RTA_PAYLOAD(tb[TCA_U32_SEL-1]) < 
+		(s->nkeys*sizeof(struct tc_u32_key)) + sizeof(struct tc_u32_sel)) {
+			printk("Please upgrade your iproute2 tools or compile proper options in!\n");
+			return -EINVAL;
+}
+#endif
 	n = kmalloc(sizeof(*n) + s->nkeys*sizeof(struct tc_u32_key), GFP_KERNEL);
 	if (n == NULL)
 		return -ENOBUFS;
@@ -775,18 +785,7 @@ static int u32_dump(struct tcf_proto *tp, unsigned long fh,
 		if (n->ht_down)
 			RTA_PUT(skb, TCA_U32_LINK, 4, &n->ht_down->handle);
 #ifdef CONFIG_CLS_U32_PERF2
-		printk("fh %x cnt %lu hit %lu\n",n->handle,n->sel.rcnt,n->sel.rhit);
 		n->sel.rcnt = n->sel.rhit = 0;
-		/* dump key cnt */
-		{
-			int i = 0;
-			struct tc_u32_key *key = n->sel.keys;
-			for (i = n->sel.nkeys; i>0; i--, key++) {
-				printk("\t key%d success %lu\n",i,key->kcnt);
-				key->cnt = 0;
-			}
-		}
-
 #endif
 #ifdef CONFIG_NET_CLS_ACT
 		/* again for backward compatible mode - we want

net/sched/police.c

@@ -166,6 +166,7 @@ void tcf_police_destroy(struct tcf_police *p)
 int tcf_act_police_locate(struct rtattr *rta, struct rtattr *est,struct tc_action *a, int ovr, int bind)
 {
 	unsigned h;
+	int ret = 0;
 	struct rtattr *tb[TCA_POLICE_MAX];
 	struct tc_police *parm;
 	struct tcf_police *p;
@@ -195,7 +196,7 @@ int tcf_act_police_locate(struct rtattr *rta, struct rtattr *est,struct tc_actio
 			goto override;
 		}
 		spin_unlock(&p->lock);
-		return 1; 
+		return ret; 
 	}
 
 	p = kmalloc(sizeof(*p), GFP_KERNEL);
@@ -203,7 +204,7 @@ int tcf_act_police_locate(struct rtattr *rta, struct rtattr *est,struct tc_actio
 		return -1;
 
 	memset(p, 0, sizeof(*p));
-	MOD_INC_USE_COUNT;
+	ret = 1;
 	p->refcnt = 1;
 	spin_lock_init(&p->lock);
 	p->stats.lock = &p->lock;
@@ -211,11 +212,13 @@ int tcf_act_police_locate(struct rtattr *rta, struct rtattr *est,struct tc_actio
 		p->bindcnt = 1;
 override:
 	if (parm->rate.rate) {
-		if ((p->R_tab = qdisc_get_rtab(&parm->rate, tb[TCA_POLICE_RATE-1])) == NULL)
+		if ((p->R_tab = qdisc_get_rtab(&parm->rate, tb[TCA_POLICE_RATE-1])) == NULL) {
 			goto failure;
+		}
 		if (parm->peakrate.rate &&
-		    (p->P_tab = qdisc_get_rtab(&parm->peakrate, tb[TCA_POLICE_PEAKRATE-1])) == NULL)
+		    (p->P_tab = qdisc_get_rtab(&parm->peakrate, tb[TCA_POLICE_PEAKRATE-1])) == NULL) {
 			goto failure;
+		}
 	}
 	if (tb[TCA_POLICE_RESULT-1])
 		p->result = *(int*)RTA_DATA(tb[TCA_POLICE_RESULT-1]);
@@ -236,7 +239,7 @@ int tcf_act_police_locate(struct rtattr *rta, struct rtattr *est,struct tc_actio
 
 	if (ovr) {
 		spin_unlock(&p->lock);
-		return 1;
+		return ret;
 	}
 	PSCHED_GET_TIME(p->t_c);
 	p->index = parm->index ? : tcf_police_new_index();
@@ -250,9 +253,8 @@ int tcf_act_police_locate(struct rtattr *rta, struct rtattr *est,struct tc_actio
 	tcf_police_ht[h] = p;
 	write_unlock_bh(&police_lock);
 
-	MOD_INC_USE_COUNT;
 	a->priv = (void *)p;
-	return 1;  
+	return ret;  
 
 failure:
 	if (p->R_tab)
@@ -263,12 +265,14 @@ int tcf_act_police_locate(struct rtattr *rta, struct rtattr *est,struct tc_actio
 	return -1;
 }
 
-void tcf_act_police_cleanup(struct tc_action *a, int bind)
+int tcf_act_police_cleanup(struct tc_action *a, int bind)
 {
 	struct tcf_police *p;
 	p = PRIV(a);
 	if (NULL != p) 
-		tcf_police_release(p, bind);
+		return tcf_police_release(p, bind);
+
+	return 0;
 }
 
 int tcf_act_police_stats(struct sk_buff *skb, struct tc_action *a)
@@ -392,8 +396,9 @@ MODULE_LICENSE("GPL");
 struct tc_action_ops act_police_ops = {
 	NULL,
 	"police",
-	TCA_ACT_POLICE, 
+	TCA_ID_POLICE, 
 	TCA_CAP_NONE, 
+	THIS_MODULE,
 	tcf_act_police,
 	tcf_act_police_stats,
 	tcf_act_police_dump,
@@ -437,7 +442,6 @@ struct tcf_police * tcf_police_locate(struct rtattr *rta, struct rtattr *est)
 
 	if (parm->index && (p = tcf_police_lookup(parm->index)) != NULL) {
 		p->refcnt++;
-		MOD_INC_USE_COUNT;
 		return p;
 	}
 
@@ -483,7 +487,6 @@ struct tcf_police * tcf_police_locate(struct rtattr *rta, struct rtattr *est)
 	p->next = tcf_police_ht[h];
 	tcf_police_ht[h] = p;
 	write_unlock_bh(&police_lock);
-	MOD_INC_USE_COUNT;
 	return p;
 
 failure:

net/sched/sch_prio.c

@@ -129,11 +129,12 @@ prio_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 static int
 prio_requeue(struct sk_buff *skb, struct Qdisc* sch)
 {
-	//struct prio_sched_data *q = (struct prio_sched_data *)sch->data;
 	struct Qdisc *qdisc;
 	int ret = NET_XMIT_DROP;
 
+#ifdef CONFIG_NET_CLS_ACT
 	sch->stats.reqs++;
+#endif
 	qdisc = prio_classify(skb, sch, &ret);
 	if (qdisc == NULL)
 		goto dropped;