Skip to contentSkip to conversation

PKT_SCHED: overall net/pkt_cls.h and net/pkt_sched.h cleanup

PATCH Applied Thomas Graf 3 participants 17 messages Oct 26, 2004 – Oct 27, 2004

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover PKT_SCHED: overall net/pkt_cls.h and net/pkt_sched.h cleanup

Dave,

This patchset attempts to clean up the mess in net/pkt_sched.h and net/pkt_cls.h grown over the last years.

It introduces net/sch_generic.h containing all generic sch/cls bits and act_api.h containing pubic action/policer bits.

The current architecture which is basically to have net/pkt_sched.h include net/pkt_cls.h and everyone just including net/pkt_sched.h makes it impossible to add any generic classifer bits into pkt_cls.h and acted just like if we had put it into one big file.

Tested for nearly a week with various combinations of action/policer configurations.

See patch descriptions for more details.

Dave, Jamal: The only difference to the previous patchset is that it doesn't touch qdisc_tree_lock and qdisc_lookup anymore but let cls_api.c include net/pkt_sched.h which makes more sense. I splitted up the patches a bit more to improve readability for the review.

#1 lore raw

PATCH 1/11 PKT_SCHED: linux/pkt_cls.h depends on linux/pkt_sched.h

include/linux/pkt_cls.h +2 -0
@@ -1,6 +1,8 @@
 #ifndef __LINUX_PKT_CLS_H
 #define __LINUX_PKT_CLS_H
 
+#include <linux/pkt_sched.h>
+
 /* I think i could have done better macros ; for now this is stolen from
  * some arch/mips code - jhs
 */
#2 lore raw

PATCH 2/11 PKT_SCHED: Add net/sch_generic.h with generic sched definitions

In reply to Thomas Graf

Adds net/sch_generic.h containing all bits directly referenced by schedulers and classifiers. Map of users:

Group 1)
struct qdisc_rate_table sch_*, tcf_police -> cls_*

Could theoretically be moved to pkt_sched.h with Patch 4 but the long term goal it so make net/act_api.h to being dependent on net/pkt_sched.h

Group 2)

 struct Qdisc                    sch_*, tcf_proto -> cls_*
 struct Qdisc_ops                sch_*, tcf_proto -> cls_*
 struct Qdisc_class_ops          sch_*, tcf_proto -> cls_*
 struct tcf_proto_ops            sch_*, cls_*
 struct tcf_proto                sch_*, cls_*

Obviously used by schedulers but also by classifiers to bind classes (tcf_proto -> Qdisc -> Qdisc_ops -> Qdisc_class_ops). tcf_proto OTOH is used by schedulers, (neat cross usage).

Group 3)
struct tcf_result sch_*, cls_*

Used to communicate between scheduler and classifer to report classify result.

Group 4)

 sch_tree_(un)lock               sch_*, cls_*
 tcf_tree_(un)lock               sch_*, cls_*
 qdisc(un)lock_tree              sch_*, cls_*

Randomly used in schedulers and classifiers.

Group 5)
tcf_destroy sch_*, cls_api.c

Coud theoretically be moved to pkt_sched.h if we include pkt_sched.h in cls_api.c but this is really generic and it seemd cleaner to have it here. This would be the only classifier related action in pkt_sched.h

The following types are referenced in sch_generic.h but defined in either pkt_cls.h or pkt_sched.h because they will never be used by the other side:

- qdisc_walker (pkt_sched.h)
- tcf_walker (pkt_cls.h)

include/net/sch_generic.h +175 -0
@@ -0,0 +1,175 @@
+#ifndef __NET_SCHED_GENERIC_H
+#define __NET_SCHED_GENERIC_H
+
+#include <linux/config.h>
+#include <linux/netdevice.h>
+#include <linux/types.h>
+#include <linux/rcupdate.h>
+#include <linux/module.h>
+#include <linux/rtnetlink.h>
+#include <linux/pkt_sched.h>
+#include <linux/pkt_cls.h>
+#include <net/gen_stats.h>
+
+struct Qdisc_ops;
+struct qdisc_walker;
+struct tcf_walker;
+struct module;
+
+struct qdisc_rate_table
+{
+	struct tc_ratespec rate;
+	u32		data[256];
+	struct qdisc_rate_table *next;
+	int		refcnt;
+};
+
+struct Qdisc
+{
+	int 			(*enqueue)(struct sk_buff *skb, struct Qdisc *dev);
+	struct sk_buff *	(*dequeue)(struct Qdisc *dev);
+	unsigned		flags;
+#define TCQ_F_BUILTIN	1
+#define TCQ_F_THROTTLED	2
+#define TCQ_F_INGRESS	4
+	int			padded;
+	struct Qdisc_ops	*ops;
+	u32			handle;
+	u32			parent;
+	atomic_t		refcnt;
+	struct sk_buff_head	q;
+	struct net_device	*dev;
+	struct list_head	list;
+
+	struct gnet_stats_basic	bstats;
+	struct gnet_stats_queue	qstats;
+	struct gnet_stats_rate_est	rate_est;
+	spinlock_t		*stats_lock;
+	struct rcu_head 	q_rcu;
+	int			(*reshape_fail)(struct sk_buff *skb,
+					struct Qdisc *q);
+
+	/* This field is deprecated, but it is still used by CBQ
+	 * and it will live until better solution will be invented.
+	 */
+	struct Qdisc		*__parent;
+};
+
+struct Qdisc_class_ops
+{
+	/* Child qdisc manipulation */
+	int			(*graft)(struct Qdisc *, unsigned long cl,
+					struct Qdisc *, struct Qdisc **);
+	struct Qdisc *		(*leaf)(struct Qdisc *, unsigned long cl);
+
+	/* Class manipulation routines */
+	unsigned long		(*get)(struct Qdisc *, u32 classid);
+	void			(*put)(struct Qdisc *, unsigned long);
+	int			(*change)(struct Qdisc *, u32, u32,
+					struct rtattr **, unsigned long *);
+	int			(*delete)(struct Qdisc *, unsigned long);
+	void			(*walk)(struct Qdisc *, struct qdisc_walker * arg);
+
+	/* Filter manipulation */
+	struct tcf_proto **	(*tcf_chain)(struct Qdisc *, unsigned long);
+	unsigned long		(*bind_tcf)(struct Qdisc *, unsigned long,
+					u32 classid);
+	void			(*unbind_tcf)(struct Qdisc *, unsigned long);
+
+	/* rtnetlink specific */
+	int			(*dump)(struct Qdisc *, unsigned long,
+					struct sk_buff *skb, struct tcmsg*);
+	int			(*dump_stats)(struct Qdisc *, unsigned long,
+					struct gnet_dump *);
+};
+
+struct Qdisc_ops
+{
+	struct Qdisc_ops	*next;
+	struct Qdisc_class_ops	*cl_ops;
+	char			id[IFNAMSIZ];
+	int			priv_size;
+
+	int 			(*enqueue)(struct sk_buff *, struct Qdisc *);
+	struct sk_buff *	(*dequeue)(struct Qdisc *);
+	int 			(*requeue)(struct sk_buff *, struct Qdisc *);
+	unsigned int		(*drop)(struct Qdisc *);
+
+	int			(*init)(struct Qdisc *, struct rtattr *arg);
+	void			(*reset)(struct Qdisc *);
+	void			(*destroy)(struct Qdisc *);
+	int			(*change)(struct Qdisc *, struct rtattr *arg);
+
+	int			(*dump)(struct Qdisc *, struct sk_buff *);
+	int			(*dump_stats)(struct Qdisc *, struct gnet_dump *);
+
+	struct module		*owner;
+};
+
+
+struct tcf_result
+{
+	unsigned long	class;
+	u32		classid;
+};
+
+struct tcf_proto_ops
+{
+	struct tcf_proto_ops	*next;
+	char			kind[IFNAMSIZ];
+
+	int			(*classify)(struct sk_buff*, struct tcf_proto*,
+					struct tcf_result *);
+	int			(*init)(struct tcf_proto*);
+	void			(*destroy)(struct tcf_proto*);
+
+	unsigned long		(*get)(struct tcf_proto*, u32 handle);
+	void			(*put)(struct tcf_proto*, unsigned long);
+	int			(*change)(struct tcf_proto*, unsigned long,
+					u32 handle, struct rtattr **,
+					unsigned long *);
+	int			(*delete)(struct tcf_proto*, unsigned long);
+	void			(*walk)(struct tcf_proto*, struct tcf_walker *arg);
+
+	/* rtnetlink specific */
+	int			(*dump)(struct tcf_proto*, unsigned long,
+					struct sk_buff *skb, struct tcmsg*);
+
+	struct module		*owner;
+};
+
+struct tcf_proto
+{
+	/* Fast access part */
+	struct tcf_proto	*next;
+	void			*root;
+	int			(*classify)(struct sk_buff*, struct tcf_proto*,
+					struct tcf_result *);
+	u32			protocol;
+
+	/* All the rest */
+	u32			prio;
+	u32			classid;
+	struct Qdisc		*q;
+	void			*data;
+	struct tcf_proto_ops	*ops;
+};
+
+
+extern void qdisc_lock_tree(struct net_device *dev);
+extern void qdisc_unlock_tree(struct net_device *dev);
+
+#define sch_tree_lock(q)	qdisc_lock_tree((q)->dev)
+#define sch_tree_unlock(q)	qdisc_unlock_tree((q)->dev)
+#define tcf_tree_lock(tp)	qdisc_lock_tree((tp)->q->dev)
+#define tcf_tree_unlock(tp)	qdisc_unlock_tree((tp)->q->dev)
+
+static inline void
+tcf_destroy(struct tcf_proto *tp)
+{
+	tp->ops->destroy(tp);
+	module_put(tp->ops->owner);
+	kfree(tp);
+}
+
+#endif
#3 lore raw

PATCH 3/11 PKT_SCHED: Remove obsolete definitions in pkt_cls.h

In reply to Thomas Graf

Removes all obsolete definitions in pkt_cls.h now in sch_generic.h and includes them via sch_generic.h.

include/net/pkt_cls.h +1 -59
@@ -1,36 +1,11 @@
 #ifndef __NET_PKT_CLS_H
 #define __NET_PKT_CLS_H
 
-
 #include <linux/pkt_cls.h>
-
-struct rtattr;
-struct tcmsg;
+#include <net/sch_generic.h>
 
 /* Basic packet classifier frontend definitions. */
 
-struct tcf_result
-{
-	unsigned long	class;
-	u32		classid;
-};
-
-struct tcf_proto
-{
-	/* Fast access part */
-	struct tcf_proto	*next;
-	void			*root;
-	int			(*classify)(struct sk_buff*, struct tcf_proto*, struct tcf_result *);
-	u32			protocol;
-
-	/* All the rest */
-	u32			prio;
-	u32			classid;
-	struct Qdisc		*q;
-	void			*data;
-	struct tcf_proto_ops	*ops;
-};
-
 struct tcf_walker
 {
 	int	stop;
@@ -39,29 +14,6 @@
 	int	(*fn)(struct tcf_proto *, unsigned long node, struct tcf_walker *);
 };
 
-struct module;
-
-struct tcf_proto_ops
-{
-	struct tcf_proto_ops	*next;
-	char			kind[IFNAMSIZ];
-
-	int			(*classify)(struct sk_buff*, struct tcf_proto*, struct tcf_result *);
-	int			(*init)(struct tcf_proto*);
-	void			(*destroy)(struct tcf_proto*);
-
-	unsigned long		(*get)(struct tcf_proto*, u32 handle);
-	void			(*put)(struct tcf_proto*, unsigned long);
-	int			(*change)(struct tcf_proto*, unsigned long, u32 handle, struct rtattr **, unsigned long *);
-	int			(*delete)(struct tcf_proto*, unsigned long);
-	void			(*walk)(struct tcf_proto*, struct tcf_walker *arg);
-
-	/* rtnetlink specific */
-	int			(*dump)(struct tcf_proto*, unsigned long, struct sk_buff *skb, struct tcmsg*);
-
-	struct module		*owner;
-};
-
 /* Main classifier routine: scans classifier chain attached
    to this qdisc, (optionally) tests for protocol and asks
    specific classifiers.
@@ -107,18 +59,8 @@
 	return -1;
 }
 
-static inline void tcf_destroy(struct tcf_proto *tp)
-{
-	tp->ops->destroy(tp);
-	module_put(tp->ops->owner);
-	kfree(tp);
-}
-
 extern int register_tcf_proto_ops(struct tcf_proto_ops *ops);
 extern int unregister_tcf_proto_ops(struct tcf_proto_ops *ops);
 extern int ing_filter(struct sk_buff *skb);
 
-
-
-
 #endif
#4 lore raw

PATCH 4/11 PKT_SCHED: Add net/act_api.h with public action/policer bits

In reply to Thomas Graf

Adds net/act_api.h containing all public action/policer bits used by schedulers and classifiers

include/net/act_api.h +122 -0
@@ -0,0 +1,122 @@
+#ifndef __NET_ACT_API_H
+#define __NET_ACT_API_H
+
+/*
+ * Public police action API for classifiers/qdiscs
+ */
+
+#include <net/sch_generic.h>
+#include <net/pkt_sched.h>
+
+struct tcf_police
+{
+	struct tcf_police *next;
+	int		refcnt;
+#ifdef CONFIG_NET_CLS_ACT
+	int		bindcnt;
+#endif
+	u32		index;
+	int		action;
+	int		result;
+	u32		ewma_rate;
+	u32		burst;
+	u32		mtu;
+	u32		toks;
+	u32		ptoks;
+	psched_time_t	t_c;
+	spinlock_t	lock;
+	struct qdisc_rate_table *R_tab;
+	struct qdisc_rate_table *P_tab;
+
+	struct tc_stats	stats;
+	spinlock_t	*stats_lock;
+};
+
+#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; \
+	int refcnt; \
+	int bindcnt; \
+	u32 capab; \
+	int action; \
+	struct tcf_t tm; \
+	struct tc_stats stats; \
+	spinlock_t *stats_lock; \
+	spinlock_t lock
+
+
+struct tc_action
+{
+	void *priv;
+	struct tc_action_ops *ops;
+	__u32   type;   /* for backward compat(TCA_OLD_COMPAT) */
+	__u32   order; 
+	struct tc_action *next;
+};
+
+#define TCA_CAP_NONE 0
+struct tc_action_ops
+{
+	struct tc_action_ops *next;
+	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);
+	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 *);
+};
+
+extern int tcf_register_action(struct tc_action_ops *a);
+extern int tcf_unregister_action(struct tc_action_ops *a);
+extern void tcf_action_destroy(struct tc_action *a, int bind);
+extern int tcf_action_exec(struct sk_buff *skb, struct tc_action *a, struct tcf_result *res);
+extern int tcf_action_init(struct rtattr *rta, struct rtattr *est, struct tc_action *a,char *n, int ovr, int bind);
+extern int tcf_action_init_1(struct rtattr *rta, struct rtattr *est, struct tc_action *a,char *n, int ovr, int bind);
+extern int tcf_action_dump(struct sk_buff *skb, struct tc_action *a, int, int);
+extern int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
+extern int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
+extern int tcf_action_copy_stats (struct sk_buff *,struct tc_action *);
+extern int tcf_act_police_locate(struct rtattr *rta, struct rtattr *est,struct tc_action *,int , int );
+extern int tcf_act_police_dump(struct sk_buff *, struct tc_action *, int, int);
+extern int tcf_act_police(struct sk_buff **skb, struct tc_action *a);
+#endif /* CONFIG_NET_CLS_ACT */
+
+extern int tcf_police(struct sk_buff *skb, struct tcf_police *p);
+extern int qdisc_copy_stats(struct sk_buff *skb, struct tc_stats *st, spinlock_t *lock);
+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 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 && !p->bindcnt) {
+			tcf_police_destroy(p);
+			ret = 1;
+		}
+	}
+#else
+	if (p && --p->refcnt == 0)
+		tcf_police_destroy(p);
+
+#endif /* CONFIG_NET_CLS_ACT */
+	return ret;
+}
+
+#endif
#5 lore raw

PATCH 5/11 PKT_SCHED: Remove obsolete definitions in pkt_sched.h

In reply to Thomas Graf

Removes all obsolete definitions in pkt_sched.h now in sch_generic.h or act_api.h.

include/net/pkt_sched.h +1 -218
@@ -1,18 +1,7 @@
 #ifndef __NET_PKT_SCHED_H
 #define __NET_PKT_SCHED_H
 
-#include <linux/config.h>
-#include <linux/netdevice.h>
-#include <linux/types.h>
-#include <linux/pkt_sched.h>
-#include <linux/rcupdate.h>
-#include <net/pkt_cls.h>
-#include <linux/module.h>
-#include <linux/rtnetlink.h>
-#include <net/gen_stats.h>
-
-struct rtattr;
-struct Qdisc;
+#include <net/sch_generic.h>
 
 struct qdisc_walker
 {
@@ -22,86 +11,8 @@
 	int	(*fn)(struct Qdisc *, unsigned long cl, struct qdisc_walker *);
 };
 
-struct Qdisc_class_ops
-{
-	/* Child qdisc manipulation */
-	int			(*graft)(struct Qdisc *, unsigned long cl, struct Qdisc *, struct Qdisc **);
-	struct Qdisc *		(*leaf)(struct Qdisc *, unsigned long cl);
-
-	/* Class manipulation routines */
-	unsigned long		(*get)(struct Qdisc *, u32 classid);
-	void			(*put)(struct Qdisc *, unsigned long);
-	int			(*change)(struct Qdisc *, u32, u32, struct rtattr **, unsigned long *);
-	int			(*delete)(struct Qdisc *, unsigned long);
-	void			(*walk)(struct Qdisc *, struct qdisc_walker * arg);
-
-	/* Filter manipulation */
-	struct tcf_proto **	(*tcf_chain)(struct Qdisc *, unsigned long);
-	unsigned long		(*bind_tcf)(struct Qdisc *, unsigned long, u32 classid);
-	void			(*unbind_tcf)(struct Qdisc *, unsigned long);
-
-	/* rtnetlink specific */
-	int			(*dump)(struct Qdisc *, unsigned long, struct sk_buff *skb, struct tcmsg*);
-	int			(*dump_stats)(struct Qdisc *, unsigned long, struct gnet_dump *);
-};
-
-struct module;
-
-struct Qdisc_ops
-{
-	struct Qdisc_ops	*next;
-	struct Qdisc_class_ops	*cl_ops;
-	char			id[IFNAMSIZ];
-	int			priv_size;
-
-	int 			(*enqueue)(struct sk_buff *, struct Qdisc *);
-	struct sk_buff *	(*dequeue)(struct Qdisc *);
-	int 			(*requeue)(struct sk_buff *, struct Qdisc *);
-	unsigned int		(*drop)(struct Qdisc *);
-
-	int			(*init)(struct Qdisc *, struct rtattr *arg);
-	void			(*reset)(struct Qdisc *);
-	void			(*destroy)(struct Qdisc *);
-	int			(*change)(struct Qdisc *, struct rtattr *arg);
-
-	int			(*dump)(struct Qdisc *, struct sk_buff *);
-	int			(*dump_stats)(struct Qdisc *, struct gnet_dump *);
-
-	struct module		*owner;
-};
-
 extern rwlock_t qdisc_tree_lock;
 
-struct Qdisc
-{
-	int 			(*enqueue)(struct sk_buff *skb, struct Qdisc *dev);
-	struct sk_buff *	(*dequeue)(struct Qdisc *dev);
-	unsigned		flags;
-#define TCQ_F_BUILTIN	1
-#define TCQ_F_THROTTLED	2
-#define TCQ_F_INGRESS	4
-	int			padded;
-	struct Qdisc_ops	*ops;
-	u32			handle;
-	u32			parent;
-	atomic_t		refcnt;
-	struct sk_buff_head	q;
-	struct net_device	*dev;
-	struct list_head	list;
-
-	struct gnet_stats_basic	bstats;
-	struct gnet_stats_queue	qstats;
-	struct gnet_stats_rate_est	rate_est;
-	spinlock_t		*stats_lock;
-	struct rcu_head 	q_rcu;
-	int			(*reshape_fail)(struct sk_buff *skb, struct Qdisc *q);
-
-	/* This field is deprecated, but it is still used by CBQ
-	 * and it will live until better solution will be invented.
-	 */
-	struct Qdisc		*__parent;
-};
-
 #define	QDISC_ALIGN		32
 #define	QDISC_ALIGN_CONST	(QDISC_ALIGN - 1)
 
@@ -111,22 +22,6 @@
 			      & ~QDISC_ALIGN_CONST);
 }
 
-struct qdisc_rate_table
-{
-	struct tc_ratespec rate;
-	u32		data[256];
-	struct qdisc_rate_table *next;
-	int		refcnt;
-};
-
-extern void qdisc_lock_tree(struct net_device *dev);
-extern void qdisc_unlock_tree(struct net_device *dev);
-
-#define sch_tree_lock(q)	qdisc_lock_tree((q)->dev)
-#define sch_tree_unlock(q)	qdisc_unlock_tree((q)->dev)
-#define tcf_tree_lock(tp)	qdisc_lock_tree((tp)->q->dev)
-#define tcf_tree_unlock(tp)	qdisc_unlock_tree((tp)->q->dev)
-
 #define cls_set_class(tp, clp, cl) tcf_set_class(tp, clp, cl)
 static inline unsigned long
 __cls_set_class(unsigned long *clp, unsigned long cl)
@@ -311,118 +206,6 @@
 
 #endif /* !CONFIG_NET_SCH_CLK_GETTIMEOFDAY */
 
-struct tcf_police
-{
-	struct tcf_police *next;
-	int		refcnt;
-#ifdef CONFIG_NET_CLS_ACT
-	int		bindcnt;
-#endif
-	u32		index;
-	int		action;
-	int		result;
-	u32		ewma_rate;
-	u32		burst;
-	u32		mtu;
-	u32		toks;
-	u32		ptoks;
-	psched_time_t	t_c;
-	spinlock_t	lock;
-	struct qdisc_rate_table *R_tab;
-	struct qdisc_rate_table *P_tab;
-
-	struct tc_stats	stats;
-	spinlock_t	*stats_lock;
-};
-
-#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; \
-	int refcnt; \
-	int bindcnt; \
-	u32 capab; \
-	int action; \
-	struct tcf_t tm; \
-	struct tc_stats stats; \
-	spinlock_t *stats_lock; \
-	spinlock_t lock
-
-
-struct tc_action
-{
-	void *priv;
-	struct tc_action_ops *ops;
-	__u32   type;   /* for backward compat(TCA_OLD_COMPAT) */
-	__u32   order; 
-	struct tc_action *next;
-};
-
-#define TCA_CAP_NONE 0
-struct tc_action_ops
-{
-	struct tc_action_ops *next;
-	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);
-	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 *);
-};
-
-extern int tcf_register_action(struct tc_action_ops *a);
-extern int tcf_unregister_action(struct tc_action_ops *a);
-extern void tcf_action_destroy(struct tc_action *a, int bind);
-extern int tcf_action_exec(struct sk_buff *skb, struct tc_action *a, struct tcf_result *res);
-extern int tcf_action_init(struct rtattr *rta, struct rtattr *est, struct tc_action *a,char *n, int ovr, int bind);
-extern int tcf_action_init_1(struct rtattr *rta, struct rtattr *est, struct tc_action *a,char *n, int ovr, int bind);
-extern int tcf_action_dump(struct sk_buff *skb, struct tc_action *a, int, int);
-extern int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
-extern int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
-extern int tcf_action_copy_stats (struct sk_buff *,struct tc_action *);
-extern int tcf_act_police_locate(struct rtattr *rta, struct rtattr *est,struct tc_action *,int , int );
-extern int tcf_act_police_dump(struct sk_buff *, struct tc_action *, int, int);
-extern int tcf_act_police(struct sk_buff **skb, struct tc_action *a);
-#endif
-
-extern unsigned long tcf_set_class(struct tcf_proto *tp, unsigned long *clp, 
-				   unsigned long cl);
-extern int tcf_police(struct sk_buff *skb, struct tcf_police *p);
-extern int qdisc_copy_stats(struct sk_buff *skb, struct tc_stats *st, spinlock_t *lock);
-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 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 && !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;
 extern struct Qdisc_ops noop_qdisc_ops;
 extern struct Qdisc_ops pfifo_qdisc_ops;
#6 lore raw

PATCH 6/11 PKT_SCHED: Transform prototypes to be extern

In reply to Thomas Graf
include/net/pkt_sched.h +18 -15
@@ -211,21 +211,24 @@
 extern struct Qdisc_ops pfifo_qdisc_ops;
 extern struct Qdisc_ops bfifo_qdisc_ops;
 
-int register_qdisc(struct Qdisc_ops *qops);
-int unregister_qdisc(struct Qdisc_ops *qops);
-struct Qdisc *qdisc_lookup(struct net_device *dev, u32 handle);
-struct Qdisc *qdisc_lookup_class(struct net_device *dev, u32 handle);
-void dev_init_scheduler(struct net_device *dev);
-void dev_shutdown(struct net_device *dev);
-void dev_activate(struct net_device *dev);
-void dev_deactivate(struct net_device *dev);
-void qdisc_reset(struct Qdisc *qdisc);
-void qdisc_destroy(struct Qdisc *qdisc);
-struct Qdisc * qdisc_create_dflt(struct net_device *dev, struct Qdisc_ops *ops);
-int qdisc_new_estimator(struct tc_stats *stats, spinlock_t *stats_lock, struct rtattr *opt);
-void qdisc_kill_estimator(struct tc_stats *stats);
-struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r, struct rtattr *tab);
-void qdisc_put_rtab(struct qdisc_rate_table *tab);
+extern int register_qdisc(struct Qdisc_ops *qops);
+extern int unregister_qdisc(struct Qdisc_ops *qops);
+extern struct Qdisc *qdisc_lookup(struct net_device *dev, u32 handle);
+extern struct Qdisc *qdisc_lookup_class(struct net_device *dev, u32 handle);
+extern void dev_init_scheduler(struct net_device *dev);
+extern void dev_shutdown(struct net_device *dev);
+extern void dev_activate(struct net_device *dev);
+extern void dev_deactivate(struct net_device *dev);
+extern void qdisc_reset(struct Qdisc *qdisc);
+extern void qdisc_destroy(struct Qdisc *qdisc);
+extern struct Qdisc * qdisc_create_dflt(struct net_device *dev,
+	struct Qdisc_ops *ops);
+extern int qdisc_new_estimator(struct tc_stats *stats, spinlock_t *stats_lock,
+	struct rtattr *opt);
+extern void qdisc_kill_estimator(struct tc_stats *stats);
+extern struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r,
+		struct rtattr *tab);
+extern void qdisc_put_rtab(struct qdisc_rate_table *tab);
 
 extern int qdisc_restart(struct net_device *dev);
 
#7 lore raw

PATCH 7/11 PKT_SCHED: Move tc_classify from pkt_cls.h to sch_api.c

In reply to Thomas Graf

tc_classiy has grown too big to be inlined, move it to sch_api.c and export it.

include/net/pkt_cls.h +0 -45
@@ -14,51 +14,6 @@
 	int	(*fn)(struct tcf_proto *, unsigned long node, struct tcf_walker *);
 };
 
-/* Main classifier routine: scans classifier chain attached
-   to this qdisc, (optionally) tests for protocol and asks
-   specific classifiers.
- */
-
-static inline int tc_classify(struct sk_buff *skb, struct tcf_proto *tp, struct tcf_result *res)
-{
-	int err = 0;
-	u32 protocol = skb->protocol;
-#ifdef CONFIG_NET_CLS_ACT
-	struct tcf_proto *otp = tp;
-reclassify:
-#endif
-	protocol = skb->protocol;
-
-	for ( ; tp; tp = tp->next) {
-		if ((tp->protocol == protocol ||
-			tp->protocol == __constant_htons(ETH_P_ALL)) &&
-			(err = tp->classify(skb, tp, res)) >= 0) {
-#ifdef CONFIG_NET_CLS_ACT
-			if ( TC_ACT_RECLASSIFY == err) {
-				__u32 verd = (__u32) G_TC_VERD(skb->tc_verd);
-				tp = otp;
-
-				if (MAX_REC_LOOP < verd++) {
-					printk("rule prio %d protocol %02x reclassify is buggy packet dropped\n",tp->prio&0xffff, ntohs(tp->protocol));
-					return TC_ACT_SHOT;
-				}
-				skb->tc_verd = SET_TC_VERD(skb->tc_verd,verd);
-				goto reclassify;
-			} else {
-				if (skb->tc_verd) 
-					skb->tc_verd = SET_TC_VERD(skb->tc_verd,0);
-				return err;
-			}
-#else
-
-			return err;
-#endif
-            }
-
-	}
-	return -1;
-}
-
 extern int register_tcf_proto_ops(struct tcf_proto_ops *ops);
 extern int unregister_tcf_proto_ops(struct tcf_proto_ops *ops);
 extern int ing_filter(struct sk_buff *skb);
include/net/pkt_sched.h +3 -0
@@ -232,6 +232,9 @@
 
 extern int qdisc_restart(struct net_device *dev);
 
+extern int tc_classify(struct sk_buff *skb, struct tcf_proto *tp,
+	struct tcf_result *res);
+
 /* Calculate maximal size of packet seen by hard_start_xmit
    routine of this device.
  */
net/sched/sch_api.c +47 -0

56 lines not on this page · raw message

#8 lore raw

PATCH 8/11 PKT_SCHED: psched_*_per_* can be static

In reply to Thomas Graf
net/sched/sch_api.c +2 -2
@@ -1145,8 +1145,8 @@
 	return -1;
 }
 
-int psched_us_per_tick = 1;
-int psched_tick_per_us = 1;
+static int psched_us_per_tick = 1;
+static int psched_tick_per_us = 1;
 
 #ifdef CONFIG_PROC_FS
 static int psched_show(struct seq_file *seq, void *v)
#9 lore raw

PATCH 9/11 PKT_SCHED: Cleanup cls_set_class

In reply to Thomas Graf

Clean up a really messy cross reference where a macro cls_set_class in pkt_sched.h would point to a function tcf_set_class in cls_api.c that uses a inlined function __cls_set_class in pkt_sched.h again.

Make tcf_set_class be cls_set_class since it was never used and inline it as well.

include/net/pkt_cls.h +25 -0
@@ -18,4 +18,29 @@
 extern int unregister_tcf_proto_ops(struct tcf_proto_ops *ops);
 extern int ing_filter(struct sk_buff *skb);
 
+static inline unsigned long
+__cls_set_class(unsigned long *clp, unsigned long cl)
+{
+	unsigned long old_cl;
+ 
+	old_cl = *clp;
+	*clp = cl;
+	return old_cl;
+}
+
+static inline long
+cls_set_class(struct tcf_proto *tp, unsigned long *clp, 
+	unsigned long cl)
+{
+	unsigned long old_cl;
+	
+	qdisc_lock_tree(tp->q->dev);
+
+	tcf_tree_lock(tp);
+	old_cl = __cls_set_class(clp, cl);
+	tcf_tree_unlock(tp);
+ 
+	return old_cl;
+}
+
 #endif
include/net/pkt_sched.h +0 -12
@@ -22,18 +22,6 @@
 			      & ~QDISC_ALIGN_CONST);
 }
 
-#define cls_set_class(tp, clp, cl) tcf_set_class(tp, clp, cl)
-static inline unsigned long
-__cls_set_class(unsigned long *clp, unsigned long cl)
-{
-	unsigned long old_cl;
-
-	old_cl = *clp;
-	*clp = cl;
-	return old_cl;
-}
-
-
 /* 
    Timer resolution MUST BE < 10% of min_schedulable_packet_size/bandwidth
    
net/sched/cls_api.c +0 -14
@@ -297,19 +297,6 @@
 	return err;
 }
 
-unsigned long tcf_set_class(struct tcf_proto *tp, unsigned long *clp, 
-			    unsigned long cl)
-{
-	unsigned long old_cl;
-
-	tcf_tree_lock(tp);
-	old_cl = __cls_set_class(clp, cl);
-	tcf_tree_unlock(tp);
-
-	return old_cl;
-}
-
-
 static int
 tcf_fill_node(struct sk_buff *skb, struct tcf_proto *tp, unsigned long fh,
 	      u32 pid, u32 seq, unsigned flags, int event)
@@ -475,4 +462,3 @@
 
 EXPORT_SYMBOL(register_tcf_proto_ops);
 EXPORT_SYMBOL(unregister_tcf_proto_ops);
-EXPORT_SYMBOL(tcf_set_class);
#10 lore raw

Re: [PATCH 9/11] PKT_SCHED: Cleanup cls_set_class

Quoted 5 lines · Thomas Graf
include/net/pkt_cls.h +5 -0
+static inline long
+cls_set_class(struct tcf_proto *tp, unsigned long *clp, 
+	unsigned long cl)
+{
+	unsigned long old_cl;

include/net/pkt_cls.h line 45

Quoted 10 lines
include/net/pkt_cls.h +10 -0
+	
+	qdisc_lock_tree(tp->q->dev);
+
+	tcf_tree_lock(tp);
+	old_cl = __cls_set_class(clp, cl);
+	tcf_tree_unlock(tp);
+ 
+	return old_cl;
+}
+

This locks once too many.
(And it returns an *unsigned* long.)

-Tommy

#11 lore raw

Re: [PATCH 9/11] PKT_SCHED: Cleanup cls_set_class

Quoted 2 lines

This locks once too many.
(And it returns an *unsigned* long.)

Thanks. I wonder how this lock could get in there.

Remove bogus lock and make cls_set_class return unsigned long.

include/net/pkt_cls.h +1 -3
@@ -28,14 +28,12 @@
 	return old_cl;
 }
 
-static inline long
+static inline unsigned long
 cls_set_class(struct tcf_proto *tp, unsigned long *clp, 
 	unsigned long cl)
 {
 	unsigned long old_cl;
 	
-	qdisc_lock_tree(tp->q->dev);
-
 	tcf_tree_lock(tp);
 	old_cl = __cls_set_class(clp, cl);
 	tcf_tree_unlock(tp);
#12 lore raw

Re: [PATCH 9/11] PKT_SCHED: Cleanup cls_set_class

include/net/pkt_cls.h line 38

Quoted 7 lines

cls_set_class(struct tcf_proto *tp, unsigned long *clp,

include/net/pkt_cls.h +0 -2
 	unsigned long cl)
 {
 	unsigned long old_cl;
 
-	qdisc_lock_tree(tp->q->dev);
-

I investigated this bug a bit more to find out why the kernel didn't lock up during my tests and it seems this lock somehow made it into while rediffing to split it up into multiple patches. Sorry folks and thanks again Tommy.

#14 lore raw

PATCH 10/11 PKT_SCHED: Inline psched_tod_diff

In reply to Thomas Graf
include/net/pkt_sched.h +12 -1
@@ -132,7 +132,18 @@
 	   __delta; \
 })
 
-extern int psched_tod_diff(int delta_sec, int bound);
+static inline int
+psched_tod_diff(int delta_sec, int bound)
+{
+	int delta;
+
+	if (bound <= 1000000 || delta_sec > (0x7FFFFFFF/1000000)-1)
+		return bound;
+	delta = delta_sec * 1000000;
+	if (delta > bound)
+		delta = bound;
+	return delta;
+}
 
 #define PSCHED_TDIFF_SAFE(tv1, tv2, bound) \
 ({ \
net/sched/sch_api.c +0 -15
@@ -1172,21 +1172,6 @@
 };	
 #endif
 
-#ifdef CONFIG_NET_SCH_CLK_GETTIMEOFDAY
-int psched_tod_diff(int delta_sec, int bound)
-{
-	int delta;
-
-	if (bound <= 1000000 || delta_sec > (0x7FFFFFFF/1000000)-1)
-		return bound;
-	delta = delta_sec * 1000000;
-	if (delta > bound)
-		delta = bound;
-	return delta;
-}
-EXPORT_SYMBOL(psched_tod_diff);
-#endif
-
 #ifdef CONFIG_NET_SCH_CLK_CPU
 psched_tdiff_t psched_clock_per_hz;
 int psched_clock_scale;
#15 lore raw

PATCH 11/11 : PKT_SCHED: Use new header architecture

In reply to Thomas Graf
include/net/tc_act/tc_gact.h +1 -1
diff -Nru linux-2.6.10-rc1-bk1.orig/include/net/tc_act/tc_gact.h linux-2.6.10-rc1-bk1/include/net/tc_act/tc_gact.h
@@ -1,7 +1,7 @@
 #ifndef __NET_TC_GACT_H
 #define __NET_TC_GACT_H
 
-#include <net/pkt_sched.h>
+#include <net/act_api.h>
 
 struct tcf_gact
 {
include/net/tc_act/tc_mirred.h +1 -1
diff -Nru linux-2.6.10-rc1-bk1.orig/include/net/tc_act/tc_mirred.h linux-2.6.10-rc1-bk1/include/net/tc_act/tc_mirred.h
@@ -1,7 +1,7 @@
 #ifndef __NET_TC_MIR_H
 #define __NET_TC_MIR_H
 
-#include <net/pkt_sched.h>
+#include <net/act_api.h>
 
 struct tcf_mirred
 {
net/sched/act_api.c +2 -1
diff -Nru linux-2.6.10-rc1-bk1.orig/net/sched/act_api.c linux-2.6.10-rc1-bk1/net/sched/act_api.c
@@ -31,7 +31,8 @@
 #include <linux/init.h>
 #include <linux/kmod.h>
 #include <net/sock.h>
-#include <net/pkt_sched.h>
+#include <net/sch_generic.h>
+#include <net/act_api.h>
 
 #if 1 /* control */
 #define DPRINTK(format,args...) printk(KERN_DEBUG format,##args)
net/sched/cls_api.c +1 -0
@@ -36,6 +36,7 @@
 #include <linux/kmod.h>
 #include <net/sock.h>
 #include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
 
 #if 0 /* control */
 #define DPRINTK(format,args...) printk(KERN_DEBUG format,##args)
net/sched/cls_fw.c +2 -1
diff -Nru linux-2.6.10-rc1-bk1.orig/net/sched/cls_fw.c linux-2.6.10-rc1-bk1/net/sched/cls_fw.c
@@ -43,7 +43,8 @@
 #include <net/route.h>
 #include <linux/skbuff.h>
 #include <net/sock.h>
-#include <net/pkt_sched.h>
+#include <net/act_api.h>
+#include <net/pkt_cls.h>
 
 struct fw_head
 {
net/sched/cls_route.c +2 -1
diff -Nru linux-2.6.10-rc1-bk1.orig/net/sched/cls_route.c linux-2.6.10-rc1-bk1/net/sched/cls_route.c
@@ -33,7 +33,8 @@
 #include <net/route.h>
 #include <linux/skbuff.h>
 #include <net/sock.h>
-#include <net/pkt_sched.h>
+#include <net/act_api.h>
+#include <net/pkt_cls.h>
 
 /*
    1. For now we assume that route tags < 256.
net/sched/cls_rsvp.c +2 -1
diff -Nru linux-2.6.10-rc1-bk1.orig/net/sched/cls_rsvp.c linux-2.6.10-rc1-bk1/net/sched/cls_rsvp.c
@@ -32,7 +32,8 @@
 #include <net/route.h>
 #include <linux/skbuff.h>
 #include <net/sock.h>
-#include <net/pkt_sched.h>
+#include <net/act_api.h>
+#include <net/pkt_cls.h>
 
 #define RSVP_DST_LEN	1
 #define RSVP_ID		"rsvp"
net/sched/cls_rsvp6.c +2 -1
diff -Nru linux-2.6.10-rc1-bk1.orig/net/sched/cls_rsvp6.c linux-2.6.10-rc1-bk1/net/sched/cls_rsvp6.c
@@ -33,7 +33,8 @@
 #include <net/route.h>
 #include <linux/skbuff.h>
 #include <net/sock.h>
-#include <net/pkt_sched.h>
+#include <net/act_api.h>
+#include <net/pkt_cls.h>
 
 #define RSVP_DST_LEN	4
 #define RSVP_ID		"rsvp6"
net/sched/cls_tcindex.c +2 -1
diff -Nru linux-2.6.10-rc1-bk1.orig/net/sched/cls_tcindex.c linux-2.6.10-rc1-bk1/net/sched/cls_tcindex.c
@@ -12,7 +12,8 @@
 #include <linux/errno.h>
 #include <linux/netdevice.h>
 #include <net/ip.h>
-#include <net/pkt_sched.h>
+#include <net/act_api.h>
+#include <net/pkt_cls.h>
 #include <net/route.h>
 
 
net/sched/cls_u32.c +2 -1
diff -Nru linux-2.6.10-rc1-bk1.orig/net/sched/cls_u32.c linux-2.6.10-rc1-bk1/net/sched/cls_u32.c
@@ -54,7 +54,8 @@
 #include <net/route.h>
 #include <linux/skbuff.h>
 #include <net/sock.h>
-#include <net/pkt_sched.h>
+#include <net/act_api.h>
+#include <net/pkt_cls.h>
 
 
 struct tc_u_knode
net/sched/police.c +1 -1
diff -Nru linux-2.6.10-rc1-bk1.orig/net/sched/police.c linux-2.6.10-rc1-bk1/net/sched/police.c
@@ -31,7 +31,7 @@
 #include <linux/rtnetlink.h>
 #include <linux/init.h>
 #include <net/sock.h>
-#include <net/pkt_sched.h>
+#include <net/act_api.h>
 
 #define L2T(p,L)   ((p)->R_tab->data[(L)>>(p)->R_tab->rate.cell_log])
 #define L2T_P(p,L) ((p)->P_tab->data[(L)>>(p)->P_tab->rate.cell_log])
maintainer
#16 lore raw
In reply to Thomas Graf
Quoted 7 lines · On Wed, 27 Oct 2004 00:21:48 +0200 Thomas Graf <[email protected]> wrote:

This patchset attempts to clean up the mess in net/pkt_sched.h and net/pkt_cls.h grown over the last years.

It introduces net/sch_generic.h containing all generic sch/cls bits and act_api.h containing pubic action/policer bits.

Quoted 8 lines

The current architecture which is basically to have net/pkt_sched.h include net/pkt_cls.h and everyone just including net/pkt_sched.h makes it impossible to add any generic classifer bits into pkt_cls.h and acted just like if we had put it into one big file.

Tested for nearly a week with various combinations of action/policer configurations.

Very nice work, all applied. Thanks Thomas.