: sch_ingress cleanups

PATCH In review Patrick McHardy 4 participants 14 messages Jan 20, 2008 – Jan 21, 2008

Subsystems (3)

Networking Network drivers Traffic control (tc)

lore

Share & tools

lore

Patrick McHardy

author

2008-01-20 14:32 UTC #0 lore raw

cover : sch_ingress cleanups

These patches clean up sch_ingress by reformatting the code to match the usual kernel style, removing lots of debugging statements that seem unnecessary after years of stable operation, removing some "future use" stuff that doesn't seem likely to ever get used and some minor additional cleanups.

 net/sched/Kconfig       |    1 +
 net/sched/sch_ingress.c |  232 ++++++++---------------------------------------
 2 files changed, 41 insertions(+), 192 deletions(-)

Patrick McHardy (9):
[NET_SCHED]: sch_ingress: formatting fixes
[NET_SCHED]: sch_ingress: remove excessive debugging [NET_SCHED]: sch_ingress: remove qdisc_priv() wrapper [NET_SCHED]: sch_ingress: remove unused inner qdisc [NET_SCHED]: sch_ingress: return proper error code in ingress_graft() [NET_SCHED]: sch_ingress: remove unnecessary ops [NET_SCHED]: sch_ingress: move dependencies to Kconfig [NET_SCHED]: sch_ingress: avoid a few #ifdefs [NET_SCHED]: sch_ingress: remove useless printk

Patrick McHardy

author

2008-01-20 14:32 UTC #1 lore raw

PATCH 1/9 : sch_ingress: formatting fixes

commit e237be7bc5b3009d1021e0764d1745e9a4c3cefb

Author: Patrick McHardy <[email protected]>
Date:   Wed Jan 16 12:22:06 2008 +0100

[NET_SCHED]: sch_ingress: formatting fixes

Signed-off-by Patrick McHardy <[email protected]>

net/sched/sch_ingress.c +39 ~~-64~~

index 89c32a9..4880d44 100644
@@ -22,23 +22,20 @@
 #undef DEBUG_INGRESS
 
 #ifdef DEBUG_INGRESS  /* control */
-#define DPRINTK(format,args...) printk(KERN_DEBUG format,##args)
+#define DPRINTK(format, args...) printk(KERN_DEBUG format,##args)
 #else
-#define DPRINTK(format,args...)
+#define DPRINTK(format, args...)
 #endif
 
 #if 0  /* data */
-#define D2PRINTK(format,args...) printk(KERN_DEBUG format,##args)
+#define D2PRINTK(format, args...) printk(KERN_DEBUG format,##args)
 #else
-#define D2PRINTK(format,args...)
+#define D2PRINTK(format, args...)
 #endif
 
-
 #define PRIV(sch) qdisc_priv(sch)
 
-
-/* Thanks to Doron Oz for this hack
-*/
+/* Thanks to Doron Oz for this hack */
 #ifndef CONFIG_NET_CLS_ACT
 #ifdef CONFIG_NETFILTER
 static int nf_registered;

@@ -50,12 +47,10 @@ struct ingress_qdisc_data {
 	struct tcf_proto	*filter_list;
 };
 
-
 /* ------------------------- Class/flow operations ------------------------- */
 
-
-static int ingress_graft(struct Qdisc *sch,unsigned long arg,
-    struct Qdisc *new,struct Qdisc **old)
+static int ingress_graft(struct Qdisc *sch, unsigned long arg,
+			 struct Qdisc *new, struct Qdisc **old)
 {
 #ifdef DEBUG_INGRESS
 	struct ingress_qdisc_data *p = PRIV(sch);

@@ -67,37 +62,33 @@ static int ingress_graft(struct Qdisc *sch,unsigned long arg,
 	return 1;
 }
 
-
 static struct Qdisc *ingress_leaf(struct Qdisc *sch, unsigned long arg)
 {
 	return NULL;
 }
 
-
-static unsigned long ingress_get(struct Qdisc *sch,u32 classid)
+static unsigned long ingress_get(struct Qdisc *sch, u32 classid)
 {
 #ifdef DEBUG_INGRESS
 	struct ingress_qdisc_data *p = PRIV(sch);
 #endif
-	DPRINTK("ingress_get(sch %p,[qdisc %p],classid %x)\n", sch, p, classid);
+	DPRINTK("ingress_get(sch %p,[qdisc %p],classid %x)\n",
+		sch, p, classid);
 	return TC_H_MIN(classid) + 1;
 }
 
-
 static unsigned long ingress_bind_filter(struct Qdisc *sch,
-    unsigned long parent, u32 classid)
+					 unsigned long parent, u32 classid)
 {
 	return ingress_get(sch, classid);
 }
 
-
 static void ingress_put(struct Qdisc *sch, unsigned long cl)
 {
 }
 
-
 static int ingress_change(struct Qdisc *sch, u32 classid, u32 parent,
-    struct rtattr **tca, unsigned long *arg)
+			  struct rtattr **tca, unsigned long *arg)
 {
 #ifdef DEBUG_INGRESS
 	struct ingress_qdisc_data *p = PRIV(sch);

@@ -108,9 +99,7 @@ static int ingress_change(struct Qdisc *sch, u32 classid, u32 parent,
 	return 0;
 }
 
-
-
-static void ingress_walk(struct Qdisc *sch,struct qdisc_walker *walker)
+static void ingress_walk(struct Qdisc *sch, struct qdisc_walker *walker)
 {
 #ifdef DEBUG_INGRESS
 	struct ingress_qdisc_data *p = PRIV(sch);

@@ -119,19 +108,16 @@ static void ingress_walk(struct Qdisc *sch,struct qdisc_walker *walker)
 	DPRINTK("No effect. sch_ingress doesn't maintain classes at the moment");
 }
 
-
-static struct tcf_proto **ingress_find_tcf(struct Qdisc *sch,unsigned long cl)
+static struct tcf_proto **ingress_find_tcf(struct Qdisc *sch, unsigned long cl)
 {
 	struct ingress_qdisc_data *p = PRIV(sch);
 
 	return &p->filter_list;
 }
 
-
 /* --------------------------- Qdisc operations ---------------------------- */
 
-
-static int ingress_enqueue(struct sk_buff *skb,struct Qdisc *sch)
+static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 {
 	struct ingress_qdisc_data *p = PRIV(sch);
 	struct tcf_result res;

@@ -148,20 +134,20 @@ static int ingress_enqueue(struct sk_buff *skb,struct Qdisc *sch)
 	sch->bstats.packets++;
 	sch->bstats.bytes += skb->len;
 	switch (result) {
-		case TC_ACT_SHOT:
-			result = TC_ACT_SHOT;
-			sch->qstats.drops++;
-			break;
-		case TC_ACT_STOLEN:
-		case TC_ACT_QUEUED:
-			result = TC_ACT_STOLEN;
-			break;
-		case TC_ACT_RECLASSIFY:
-		case TC_ACT_OK:
-			skb->tc_index = TC_H_MIN(res.classid);
-		default:
-			result = TC_ACT_OK;
-			break;
+	case TC_ACT_SHOT:
+		result = TC_ACT_SHOT;
+		sch->qstats.drops++;
+		break;
+	case TC_ACT_STOLEN:
+	case TC_ACT_QUEUED:
+		result = TC_ACT_STOLEN;
+		break;
+	case TC_ACT_RECLASSIFY:
+	case TC_ACT_OK:
+		skb->tc_index = TC_H_MIN(res.classid);
+	default:
+		result = TC_ACT_OK;
+		break;
 	}
 #else
 	D2PRINTK("Overriding result to ACCEPT\n");

@@ -173,7 +159,6 @@ static int ingress_enqueue(struct sk_buff *skb,struct Qdisc *sch)
 	return result;
 }
 
-
 static struct sk_buff *ingress_dequeue(struct Qdisc *sch)
 {
 /*

@@ -183,8 +168,7 @@ static struct sk_buff *ingress_dequeue(struct Qdisc *sch)
 	return NULL;
 }
 
-
-static int ingress_requeue(struct sk_buff *skb,struct Qdisc *sch)
+static int ingress_requeue(struct sk_buff *skb, struct Qdisc *sch)
 {
 /*
 	struct ingress_qdisc_data *p = PRIV(sch);

@@ -204,8 +188,7 @@ static unsigned int ingress_drop(struct Qdisc *sch)
 
 #ifndef CONFIG_NET_CLS_ACT
 #ifdef CONFIG_NETFILTER
-static unsigned int
-ing_hook(unsigned int hook, struct sk_buff *skb,
+static unsigned int ing_hook(unsigned int hook, struct sk_buff *skb,
 			     const struct net_device *indev,
 			     const struct net_device *outdev,
 			     int (*okfn)(struct sk_buff *))

@@ -213,7 +196,7 @@ ing_hook(unsigned int hook, struct sk_buff *skb,
 
 	struct Qdisc *q;
 	struct net_device *dev = skb->dev;
-	int fwres=NF_ACCEPT;
+	int fwres = NF_ACCEPT;
 
 	DPRINTK("ing_hook: skb %s dev=%s len=%u\n",
 		skb->sk ? "(owned)" : "(unowned)",

@@ -247,16 +230,15 @@ static struct nf_hook_ops ing_ops[] __read_mostly = {
 		.priority       = NF_IP6_PRI_FILTER + 1,
 	},
 };
-
 #endif
 #endif
 
-static int ingress_init(struct Qdisc *sch,struct rtattr *opt)
+static int ingress_init(struct Qdisc *sch, struct rtattr *opt)
 {
 	struct ingress_qdisc_data *p = PRIV(sch);
 
-/* Make sure either netfilter or preferably CLS_ACT is
-* compiled in */
+	/* Make sure either netfilter or preferably CLS_ACT is
+	 * compiled in */
 #ifndef CONFIG_NET_CLS_ACT
 #ifndef CONFIG_NETFILTER
 	printk("You MUST compile classifier actions into the kernel\n");

@@ -278,12 +260,11 @@ static int ingress_init(struct Qdisc *sch,struct rtattr *opt)
 #endif
 #endif
 
-	DPRINTK("ingress_init(sch %p,[qdisc %p],opt %p)\n",sch,p,opt);
+	DPRINTK("ingress_init(sch %p,[qdisc %p],opt %p)\n", sch, p, opt);
 	p->q = &noop_qdisc;
 	return 0;
 }
 
-
 static void ingress_reset(struct Qdisc *sch)
 {
 	struct ingress_qdisc_data *p = PRIV(sch);

@@ -302,9 +283,6 @@ static void ingress_reset(struct Qdisc *sch)
 
 /* ------------------------------------------------------------- */
 
-
-/* ------------------------------------------------------------- */
-
 static void ingress_destroy(struct Qdisc *sch)
 {
 	struct ingress_qdisc_data *p = PRIV(sch);

@@ -317,13 +295,12 @@ static void ingress_destroy(struct Qdisc *sch)
 #endif
 }
 
-
 static int ingress_dump(struct Qdisc *sch, struct sk_buff *skb)
 {
 	unsigned char *b = skb_tail_pointer(skb);
 	struct rtattr *rta;
 
-	rta = (struct rtattr *) b;
+	rta = (struct rtattr *)b;
 	RTA_PUT(skb, TCA_OPTIONS, 0, NULL);
 	rta->rta_len = skb_tail_pointer(skb) - b;
 	return skb->len;

@@ -339,16 +316,13 @@ static const struct Qdisc_class_ops ingress_class_ops = {
 	.get		=	ingress_get,
 	.put		=	ingress_put,
 	.change		=	ingress_change,
-	.delete		=	NULL,
 	.walk		=	ingress_walk,
 	.tcf_chain	=	ingress_find_tcf,
 	.bind_tcf	=	ingress_bind_filter,
 	.unbind_tcf	=	ingress_put,
-	.dump		=	NULL,
 };
 
 static struct Qdisc_ops ingress_qdisc_ops __read_mostly = {
-	.next		=	NULL,
 	.cl_ops		=	&ingress_class_ops,
 	.id		=	"ingress",
 	.priv_size	=	sizeof(struct ingress_qdisc_data),

@@ -359,7 +333,6 @@ static struct Qdisc_ops ingress_qdisc_ops __read_mostly = {
 	.init		=	ingress_init,
 	.reset		=	ingress_reset,
 	.destroy	=	ingress_destroy,
-	.change		=	NULL,
 	.dump		=	ingress_dump,
 	.owner		=	THIS_MODULE,
 };

@@ -375,6 +348,7 @@ static int __init ingress_module_init(void)
 
 	return ret;
 }
+
 static void __exit ingress_module_exit(void)
 {
 	unregister_qdisc(&ingress_qdisc_ops);

@@ -385,6 +359,7 @@ static void __exit ingress_module_exit(void)
 #endif
 #endif
 }
+
 module_init(ingress_module_init)
 module_exit(ingress_module_exit)
 MODULE_LICENSE("GPL");

Patrick McHardy

author

2008-01-20 14:32 UTC #2 lore raw

PATCH 2/9 : sch_ingress: remove excessive debugging

In reply to Patrick McHardy

commit fdb1b974189eeab782ba3595d1ca550a1a5ee79e

Author: Patrick McHardy <[email protected]>
Date:   Wed Jan 16 12:22:06 2008 +0100

[NET_SCHED]: sch_ingress: remove excessive debugging

Remove excessive debugging statements and some "future use" stuff.

Signed-off-by Patrick McHardy <[email protected]>

net/sched/sch_ingress.c +3 ~~-76~~

index 4880d44..bdda28d 100644
@@ -19,20 +19,6 @@
 #include <net/pkt_sched.h>
 
 
-#undef DEBUG_INGRESS
-
-#ifdef DEBUG_INGRESS  /* control */
-#define DPRINTK(format, args...) printk(KERN_DEBUG format,##args)
-#else
-#define DPRINTK(format, args...)
-#endif
-
-#if 0  /* data */
-#define D2PRINTK(format, args...) printk(KERN_DEBUG format,##args)
-#else
-#define D2PRINTK(format, args...)
-#endif
-
 #define PRIV(sch) qdisc_priv(sch)
 
 /* Thanks to Doron Oz for this hack */

@@ -52,13 +38,6 @@ struct ingress_qdisc_data {
 static int ingress_graft(struct Qdisc *sch, unsigned long arg,
 			 struct Qdisc *new, struct Qdisc **old)
 {
-#ifdef DEBUG_INGRESS
-	struct ingress_qdisc_data *p = PRIV(sch);
-#endif
-
-	DPRINTK("ingress_graft(sch %p,[qdisc %p],new %p,old %p)\n",
-		sch, p, new, old);
-	DPRINTK("\n ingress_graft: You cannot add qdiscs to classes");
 	return 1;
 }

@@ -69,11 +48,6 @@ static struct Qdisc *ingress_leaf(struct Qdisc *sch, unsigned long arg)
 
 static unsigned long ingress_get(struct Qdisc *sch, u32 classid)
 {
-#ifdef DEBUG_INGRESS
-	struct ingress_qdisc_data *p = PRIV(sch);
-#endif
-	DPRINTK("ingress_get(sch %p,[qdisc %p],classid %x)\n",
-		sch, p, classid);
 	return TC_H_MIN(classid) + 1;
 }

@@ -90,22 +64,12 @@ static void ingress_put(struct Qdisc *sch, unsigned long cl)
 static int ingress_change(struct Qdisc *sch, u32 classid, u32 parent,
 			  struct rtattr **tca, unsigned long *arg)
 {
-#ifdef DEBUG_INGRESS
-	struct ingress_qdisc_data *p = PRIV(sch);
-#endif
-	DPRINTK("ingress_change(sch %p,[qdisc %p],classid %x,parent %x),"
-		"arg 0x%lx\n", sch, p, classid, parent, *arg);
-	DPRINTK("No effect. sch_ingress doesn't maintain classes at the moment");
 	return 0;
 }
 
 static void ingress_walk(struct Qdisc *sch, struct qdisc_walker *walker)
 {
-#ifdef DEBUG_INGRESS
-	struct ingress_qdisc_data *p = PRIV(sch);
-#endif
-	DPRINTK("ingress_walk(sch %p,[qdisc %p],walker %p)\n", sch, p, walker);
-	DPRINTK("No effect. sch_ingress doesn't maintain classes at the moment");
+	return;
 }
 
 static struct tcf_proto **ingress_find_tcf(struct Qdisc *sch, unsigned long cl)

@@ -123,9 +87,8 @@ static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	struct tcf_result res;
 	int result;
 
-	D2PRINTK("ingress_enqueue(skb %p,sch %p,[qdisc %p])\n", skb, sch, p);
 	result = tc_classify(skb, p->filter_list, &res);
-	D2PRINTK("result %d class 0x%04x\n", result, res.classid);
+
 	/*
 	 * Unlike normal "enqueue" functions, ingress_enqueue returns a
 	 * firewall FW_* code.

@@ -150,7 +113,6 @@ static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 		break;
 	}
 #else
-	D2PRINTK("Overriding result to ACCEPT\n");
 	result = NF_ACCEPT;
 	sch->bstats.packets++;
 	sch->bstats.bytes += skb->len;

@@ -161,28 +123,16 @@ static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 
 static struct sk_buff *ingress_dequeue(struct Qdisc *sch)
 {
-/*
-	struct ingress_qdisc_data *p = PRIV(sch);
-	D2PRINTK("ingress_dequeue(sch %p,[qdisc %p])\n",sch,PRIV(p));
-*/
 	return NULL;
 }
 
 static int ingress_requeue(struct sk_buff *skb, struct Qdisc *sch)
 {
-/*
-	struct ingress_qdisc_data *p = PRIV(sch);
-	D2PRINTK("ingress_requeue(skb %p,sch %p,[qdisc %p])\n",skb,sch,PRIV(p));
-*/
 	return 0;
 }
 
 static unsigned int ingress_drop(struct Qdisc *sch)
 {
-#ifdef DEBUG_INGRESS
-	struct ingress_qdisc_data *p = PRIV(sch);
-#endif
-	DPRINTK("ingress_drop(sch %p,[qdisc %p])\n", sch, p);
 	return 0;
 }

@@ -198,11 +148,6 @@ static unsigned int ing_hook(unsigned int hook, struct sk_buff *skb,
 	struct net_device *dev = skb->dev;
 	int fwres = NF_ACCEPT;
 
-	DPRINTK("ing_hook: skb %s dev=%s len=%u\n",
-		skb->sk ? "(owned)" : "(unowned)",
-		skb->dev ? skb->dev->name : "(no dev)",
-		skb->len);
-
 	if (dev->qdisc_ingress) {
 		spin_lock(&dev->ingress_lock);
 		if ((q = dev->qdisc_ingress) != NULL)

@@ -259,26 +204,13 @@ static int ingress_init(struct Qdisc *sch, struct rtattr *opt)
 	}
 #endif
 #endif
-
-	DPRINTK("ingress_init(sch %p,[qdisc %p],opt %p)\n", sch, p, opt);
 	p->q = &noop_qdisc;
 	return 0;
 }
 
 static void ingress_reset(struct Qdisc *sch)
 {
-	struct ingress_qdisc_data *p = PRIV(sch);
-
-	DPRINTK("ingress_reset(sch %p,[qdisc %p])\n", sch, p);
-
-/*
-#if 0
-*/
-/* for future use */
-	qdisc_reset(p->q);
-/*
-#endif
-*/
+	return;
 }
 
 /* ------------------------------------------------------------- */

@@ -287,12 +219,7 @@ static void ingress_destroy(struct Qdisc *sch)
 {
 	struct ingress_qdisc_data *p = PRIV(sch);
 
-	DPRINTK("ingress_destroy(sch %p,[qdisc %p])\n", sch, p);
 	tcf_destroy_chain(p->filter_list);
-#if 0
-/* for future use */
-	qdisc_destroy(p->q);
-#endif
 }
 
 static int ingress_dump(struct Qdisc *sch, struct sk_buff *skb)

Patrick McHardy

author

2008-01-20 14:32 UTC #3 lore raw

PATCH 3/9 : sch_ingress: remove qdisc_priv() wrapper

In reply to Patrick McHardy

commit ec46354d4ba87fe98b8510f2d91fff4e106022ec

Author: Patrick McHardy <[email protected]>
Date:   Wed Jan 16 12:22:07 2008 +0100

[NET_SCHED]: sch_ingress: remove qdisc_priv() wrapper

Signed-off-by Patrick McHardy <[email protected]>

net/sched/sch_ingress.c +4 -6

index bdda28d..cb8ba8b 100644
@@ -19,8 +19,6 @@
 #include <net/pkt_sched.h>
 
 
-#define PRIV(sch) qdisc_priv(sch)
-
 /* Thanks to Doron Oz for this hack */
 #ifndef CONFIG_NET_CLS_ACT
 #ifdef CONFIG_NETFILTER

@@ -74,7 +72,7 @@ static void ingress_walk(struct Qdisc *sch, struct qdisc_walker *walker)
 
 static struct tcf_proto **ingress_find_tcf(struct Qdisc *sch, unsigned long cl)
 {
-	struct ingress_qdisc_data *p = PRIV(sch);
+	struct ingress_qdisc_data *p = qdisc_priv(sch);
 
 	return &p->filter_list;
 }

@@ -83,7 +81,7 @@ static struct tcf_proto **ingress_find_tcf(struct Qdisc *sch, unsigned long cl)
 
 static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 {
-	struct ingress_qdisc_data *p = PRIV(sch);
+	struct ingress_qdisc_data *p = qdisc_priv(sch);
 	struct tcf_result res;
 	int result;

@@ -180,7 +178,7 @@ static struct nf_hook_ops ing_ops[] __read_mostly = {
 
 static int ingress_init(struct Qdisc *sch, struct rtattr *opt)
 {
-	struct ingress_qdisc_data *p = PRIV(sch);
+	struct ingress_qdisc_data *p = qdisc_priv(sch);
 
 	/* Make sure either netfilter or preferably CLS_ACT is
 	 * compiled in */

@@ -217,7 +215,7 @@ static void ingress_reset(struct Qdisc *sch)
 
 static void ingress_destroy(struct Qdisc *sch)
 {
-	struct ingress_qdisc_data *p = PRIV(sch);
+	struct ingress_qdisc_data *p = qdisc_priv(sch);
 
 	tcf_destroy_chain(p->filter_list);
 }

Patrick McHardy

author

2008-01-20 14:32 UTC #4 lore raw

PATCH 4/9 : sch_ingress: remove unused inner qdisc

In reply to Patrick McHardy

commit bb582f29b1f403d0df8a4a54daca89eafd2bfb69

Author: Patrick McHardy <[email protected]>
Date:   Wed Jan 16 12:22:07 2008 +0100

[NET_SCHED]: sch_ingress: remove unused inner qdisc

Signed-off-by Patrick McHardy <[email protected]>

net/sched/sch_ingress.c +0 -4

index cb8ba8b..d803cd1 100644
@@ -27,7 +27,6 @@ static int nf_registered;
 #endif
 
 struct ingress_qdisc_data {
-	struct Qdisc		*q;
 	struct tcf_proto	*filter_list;
 };

@@ -178,8 +177,6 @@ static struct nf_hook_ops ing_ops[] __read_mostly = {
 
 static int ingress_init(struct Qdisc *sch, struct rtattr *opt)
 {
-	struct ingress_qdisc_data *p = qdisc_priv(sch);
-
 	/* Make sure either netfilter or preferably CLS_ACT is
 	 * compiled in */
 #ifndef CONFIG_NET_CLS_ACT

@@ -202,7 +199,6 @@ static int ingress_init(struct Qdisc *sch, struct rtattr *opt)
 	}
 #endif
 #endif
-	p->q = &noop_qdisc;
 	return 0;
 }

Patrick McHardy

author

2008-01-20 14:32 UTC #5 lore raw

PATCH 5/9 : sch_ingress: return proper error code in ingress_graft()

In reply to Patrick McHardy

commit e039da57d244f15d4b920de63a20ab516a76ed5b

Author: Patrick McHardy <[email protected]>
Date:   Wed Jan 16 12:22:07 2008 +0100

[NET_SCHED]: sch_ingress: return proper error code in ingress_graft()

Signed-off-by Patrick McHardy <[email protected]>

net/sched/sch_ingress.c +1 -1

index d803cd1..1bbc648 100644
@@ -35,7 +35,7 @@ struct ingress_qdisc_data {
 static int ingress_graft(struct Qdisc *sch, unsigned long arg,
 			 struct Qdisc *new, struct Qdisc **old)
 {
-	return 1;
+	return -EOPNOTSUPP;
 }
 
 static struct Qdisc *ingress_leaf(struct Qdisc *sch, unsigned long arg)

Patrick McHardy

author

2008-01-20 14:32 UTC #6 lore raw

PATCH 6/9 : sch_ingress: remove unnecessary ops

In reply to Patrick McHardy

commit c96158287d0c87fc3e4d005ce8bf2c778a5c2cb6

Author: Patrick McHardy <[email protected]>
Date:   Wed Jan 16 12:22:07 2008 +0100

[NET_SCHED]: sch_ingress: remove unnecessary ops

- ->reset is optional
- sch_api provides identical defaults for ->dequeue/->requeue
- ->drop can't happen since ingress never has a parent qdisc

Signed-off-by Patrick McHardy <[email protected]>

net/sched/sch_ingress.c +0 ~~-24~~

index 1bbc648..c69e7bc 100644
@@ -118,21 +118,6 @@ static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	return result;
 }
 
-static struct sk_buff *ingress_dequeue(struct Qdisc *sch)
-{
-	return NULL;
-}
-
-static int ingress_requeue(struct sk_buff *skb, struct Qdisc *sch)
-{
-	return 0;
-}
-
-static unsigned int ingress_drop(struct Qdisc *sch)
-{
-	return 0;
-}
-
 #ifndef CONFIG_NET_CLS_ACT
 #ifdef CONFIG_NETFILTER
 static unsigned int ing_hook(unsigned int hook, struct sk_buff *skb,

@@ -202,11 +187,6 @@ static int ingress_init(struct Qdisc *sch, struct rtattr *opt)
 	return 0;
 }
 
-static void ingress_reset(struct Qdisc *sch)
-{
-	return;
-}
-
 /* ------------------------------------------------------------- */
 
 static void ingress_destroy(struct Qdisc *sch)

@@ -248,11 +228,7 @@ static struct Qdisc_ops ingress_qdisc_ops __read_mostly = {
 	.id		=	"ingress",
 	.priv_size	=	sizeof(struct ingress_qdisc_data),
 	.enqueue	=	ingress_enqueue,
-	.dequeue	=	ingress_dequeue,
-	.requeue	=	ingress_requeue,
-	.drop		=	ingress_drop,
 	.init		=	ingress_init,
-	.reset		=	ingress_reset,
 	.destroy	=	ingress_destroy,
 	.dump		=	ingress_dump,
 	.owner		=	THIS_MODULE,

Patrick McHardy

author

2008-01-20 14:32 UTC #7 lore raw

PATCH 7/9 : sch_ingress: move dependencies to Kconfig

In reply to Patrick McHardy

commit 4c939213974319d9457dbbf038d38ef1fcf57ef9

Author: Patrick McHardy <[email protected]>
Date:   Wed Jan 16 12:22:08 2008 +0100

[NET_SCHED]: sch_ingress: move dependencies to Kconfig

Instead of complaining at scheduler initialization time, check the dependencies in Kconfig.

Signed-off-by Patrick McHardy <[email protected]>

net/sched/Kconfig +1 -0

index 4eb73ec..a047cd5 100644
@@ -204,6 +204,7 @@ config NET_SCH_NETEM
 
 config NET_SCH_INGRESS
 	tristate "Ingress Qdisc"
+	depends on NET_CLS_ACT || NETFILTER
 	---help---
 	  Say Y here if you want to use classifiers for incoming packets.
 	  If unsure, say Y.

net/sched/sch_ingress.c +1 ~~-10~~

index c69e7bc..b30ca01 100644
@@ -162,19 +162,10 @@ static struct nf_hook_ops ing_ops[] __read_mostly = {
 
 static int ingress_init(struct Qdisc *sch, struct rtattr *opt)
 {
-	/* Make sure either netfilter or preferably CLS_ACT is
-	 * compiled in */
 #ifndef CONFIG_NET_CLS_ACT
-#ifndef CONFIG_NETFILTER
-	printk("You MUST compile classifier actions into the kernel\n");
-	return -EINVAL;
-#else
+#ifdef CONFIG_NETFILTER
 	printk("Ingress scheduler: Classifier actions prefered over netfilter\n");
-#endif
-#endif
 
-#ifndef CONFIG_NET_CLS_ACT
-#ifdef CONFIG_NETFILTER
 	if (!nf_registered) {
 		if (nf_register_hooks(ing_ops, ARRAY_SIZE(ing_ops)) < 0) {
 			printk("ingress qdisc registration error \n");

Patrick McHardy

author

2008-01-20 14:32 UTC #8 lore raw

PATCH 8/9 : sch_ingress: avoid a few

In reply to Patrick McHardy

commit 3320bc3c5d9a6c1dd96d515b7b6a4dee9cd31fd7

Author: Patrick McHardy <[email protected]>
Date:   Thu Jan 17 09:17:58 2008 +0100

[NET_SCHED]: sch_ingress: avoid a few #ifdefs

Move the repeating "ifndef CONFIG_NET_CLS_ACT/ifdef CONFIG_NETFILTER" ifdefs into a single condition.

Signed-off-by Patrick McHardy <[email protected]>

net/sched/sch_ingress.c +4 ~~-12~~

index b30ca01..4c7f7e7 100644
@@ -20,11 +20,9 @@
 
 
 /* Thanks to Doron Oz for this hack */
-#ifndef CONFIG_NET_CLS_ACT
-#ifdef CONFIG_NETFILTER
+#if !defined(CONFIG_NET_CLS_ACT) && defined(CONFIG_NETFILTER)
 static int nf_registered;
 #endif
-#endif
 
 struct ingress_qdisc_data {
 	struct tcf_proto	*filter_list;

@@ -118,8 +116,7 @@ static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	return result;
 }
 
-#ifndef CONFIG_NET_CLS_ACT
-#ifdef CONFIG_NETFILTER
+#if !defined(CONFIG_NET_CLS_ACT) && defined(CONFIG_NETFILTER)
 static unsigned int ing_hook(unsigned int hook, struct sk_buff *skb,
 			     const struct net_device *indev,
 			     const struct net_device *outdev,

@@ -158,12 +155,10 @@ static struct nf_hook_ops ing_ops[] __read_mostly = {
 	},
 };
 #endif
-#endif
 
 static int ingress_init(struct Qdisc *sch, struct rtattr *opt)
 {
-#ifndef CONFIG_NET_CLS_ACT
-#ifdef CONFIG_NETFILTER
+#if !defined(CONFIG_NET_CLS_ACT) && defined(CONFIG_NETFILTER)
 	printk("Ingress scheduler: Classifier actions prefered over netfilter\n");
 
 	if (!nf_registered) {

@@ -174,7 +169,6 @@ static int ingress_init(struct Qdisc *sch, struct rtattr *opt)
 		nf_registered++;
 	}
 #endif
-#endif
 	return 0;
 }

@@ -240,12 +234,10 @@ static int __init ingress_module_init(void)
 static void __exit ingress_module_exit(void)
 {
 	unregister_qdisc(&ingress_qdisc_ops);
-#ifndef CONFIG_NET_CLS_ACT
-#ifdef CONFIG_NETFILTER
+#if !defined(CONFIG_NET_CLS_ACT) && defined(CONFIG_NETFILTER)
 	if (nf_registered)
 		nf_unregister_hooks(ing_ops, ARRAY_SIZE(ing_ops));
 #endif
-#endif
 }
 
 module_init(ingress_module_init)

Patrick McHardy

author

2008-01-20 14:32 UTC #9 lore raw

PATCH 9/9 : sch_ingress: remove useless printk

In reply to Patrick McHardy

commit f8d51e131177c26c1033dfc804f11d9cac7637f4

Author: Patrick McHardy <[email protected]>
Date:   Thu Jan 17 09:17:59 2008 +0100

[NET_SCHED]: sch_ingress: remove useless printk

The printk about ingress qdisc registration error can't be triggered under normal circumstances. Since register_qdisc only fails for two identical registrations, the only way to trigger it is by loading the sch_ingress modules multiple times under different names, in which case we already return -EEXIST to userspace.

Signed-off-by Patrick McHardy <[email protected]>

net/sched/sch_ingress.c +1 -8

index 4c7f7e7..7252571 100644
@@ -221,14 +221,7 @@ static struct Qdisc_ops ingress_qdisc_ops __read_mostly = {
 
 static int __init ingress_module_init(void)
 {
-	int ret = 0;
-
-	if ((ret = register_qdisc(&ingress_qdisc_ops)) < 0) {
-		printk("Unable to register Ingress qdisc\n");
-		return ret;
-	}
-
-	return ret;
+	return register_qdisc(&ingress_qdisc_ops);
 }
 
 static void __exit ingress_module_exit(void)

jamal

Acked-by on the v1 series

2008-01-20 15:31 UTC #10 lore raw

In reply to Patrick McHardy

Quoted 6 lines · Patrick McHardy

These patches clean up sch_ingress by reformatting the code to match the usual kernel style, removing lots of debugging statements that seem unnecessary after years of stable operation, removing some "future use" stuff that doesn't seem likely to ever get used and some minor additional cleanups.

Patrick - if you are in mopping-mode then you will find some of the same things in dsmark (LinuxWay(tm)).

For all patches:

Acked-by Jamal Hadi Salim <[email protected]>

cheers,
jamal

Patrick McHardy

author

2008-01-20 15:36 UTC #11 lore raw

Quoted 10 lines · jamal

Quoted 6 lines · Patrick McHardy
These patches clean up sch_ingress by reformatting the code to match the usual kernel style, removing lots of debugging statements that seem unnecessary after years of stable operation, removing some "future use" stuff that doesn't seem likely to ever get used and some minor additional cleanups.
Patrick - if you are in mopping-mode then you will find some of the same things in dsmark (LinuxWay(tm)).

Indeed, that one could use a bit of cleanup as well. I took care of sch_ingress first since I had these patches queued for multiple years now and always forgot to send them out, but I'll look into sch_dsmark next time I'm in the area :)

Quoted 2 lines

For all patches:
Acked-by Jamal Hadi Salim <[email protected]>

Thanks.

Stephen Hemminger

2008-01-20 16:38 UTC #12 lore raw

Quoted 9 lines · On Sun, 20 Jan 2008 16:35:01 +0100 Patrick McHardy <[email protected]> wrote:

Quoted 8 lines · jamal
Quoted 6 lines · Patrick McHardy
These patches clean up sch_ingress by reformatting the code to match the usual kernel style, removing lots of debugging statements that seem unnecessary after years of stable operation, removing some "future use" stuff that doesn't seem likely to ever get used and some minor additional cleanups.

Quoted 11 lines

Quoted 2 lines
Patrick - if you are in mopping-mode then you will find some of the same things in dsmark (LinuxWay(tm)).
Indeed, that one could use a bit of cleanup as well. I took care of sch_ingress first since I had these patches queued for multiple years now and always forgot to send them out, but I'll look into sch_dsmark next time I'm in the area :)
Quoted 2 lines
For all patches:
Acked-by Jamal Hadi Salim <[email protected]>

I'll fix dsmark, I need it for some other stuff.

Signature · 1 line

Stephen Hemminger <[email protected]>

David Miller

maintainer

2008-01-21 08:14 UTC #13 lore raw

In reply to jamal

From: jamal <[email protected]>
Date: Sun, 20 Jan 2008 10:31:36 -0500

Quoted 2 lines

For all patches:
Acked-by Jamal Hadi Salim <[email protected]>

All 9 patches applied, thanks.