Skip to contentSkip to conversation

sch_prio - fix compile warning + some logic rearrangement in enqueue

PATCH In review Dmitry Torokhov 3 participants 6 messages Jun 29, 2004 1 message not in the archive

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

PATCH 1/1 sch_prio - fix compile warning + some logic rearrangement in enqueue

Hi,

When CONFIG_NET_CLS_ACT is not set 'result' variable in prio_classify is unused. Also I was looking over the rest of the module and had hard time understanding the logic in prio_enqueue - I rearranged it a bit for better readability. Plus there are some formatting changes.

Please consider for inclusion.

Signature · 99 lines
Dmitry


===================================================================


[email protected], 2004-06-29 00:45:12-05:00, [email protected]
  NET: sch_prio
       - fix compile warning in prio_classify
       - rearrange prio_enqueue conditional compilation maze
       - minor formatting changes

  Signed-off-by: Dmitry Torokhov <[email protected]>

 sch_prio.c |   30 ++++++++++++++----------------
 1 files changed, 14 insertions(+), 16 deletions(-)


===================================================================



diff -Nru a/net/sched/sch_prio.c b/net/sched/sch_prio.c
--- a/net/sched/sch_prio.c	2004-06-29 00:58:52 -05:00
+++ b/net/sched/sch_prio.c	2004-06-29 00:58:52 -05:00
@@ -47,20 +47,19 @@
 };
 
 
-struct Qdisc *prio_classify(struct sk_buff *skb, struct Qdisc *sch,int *r)
+struct Qdisc *prio_classify(struct sk_buff *skb, struct Qdisc *sch, int *r)
 {
 	struct prio_sched_data *q = (struct prio_sched_data *)sch->data;
 	struct tcf_result res;
 	u32 band;
-	int result = 0;
 
 	band = skb->priority;
 
 	if (TC_H_MAJ(skb->priority) != sch->handle) {
 #ifdef CONFIG_NET_CLS_ACT
-		*r = result = tc_classify(skb, q->filter_list, &res);
+		*r = tc_classify(skb, q->filter_list, &res);
 
-		switch (result) {
+		switch (*r) {
 			case TC_ACT_SHOT:
 			case TC_ACT_STOLEN:
 			case TC_ACT_QUEUED:
@@ -70,22 +69,22 @@
 			case TC_ACT_OK:
 			case TC_ACT_UNSPEC:
 			default:
-			break;
+				break;
 		};
 
-		if (!q->filter_list ) {
+		if (!q->filter_list) {
 #else
 		if (!q->filter_list || tc_classify(skb, q->filter_list, &res)) {
 #endif
 			if (TC_H_MAJ(band))
 				band = 0;
-			return q->queues[q->prio2band[band&TC_PRIO_MAX]];
+			return q->queues[q->prio2band[band & TC_PRIO_MAX]];
 		}
 		band = res.classid;
 	}
 	band = TC_H_MIN(band) - 1;
 	if (band > q->bands)
-		return q->queues[q->prio2band[0]];
+		band = q->prio2band[0];
 
 	return q->queues[band];
 }
@@ -112,17 +111,16 @@
 	}
 
 dropped:
+
 #ifdef CONFIG_NET_CLS_ACT
-	if (TC_ACT_SHOT == ret || NET_XMIT_DROP == ret) {
-#endif
-		sch->stats.drops++;
-		return NET_XMIT_DROP;
-#ifdef CONFIG_NET_CLS_ACT
-	} else {
-		sch->stats.overlimits++; /* abuse, but noone uses it */
-		return NET_XMIT_BYPASS; /* we dont want to confuse TCP */
+	if (ret != TC_ACT_SHOT && ret != NET_XMIT_DROP) {
+		sch->stats.overlimits++;	/* abuse, but noone uses it */
+		return NET_XMIT_BYPASS;		/* we dont want to confuse TCP */
 	}
 #endif
+
+	sch->stats.drops++;
+	return NET_XMIT_DROP;
 }
#1 lore raw
Quoted 4 lines · On Tue, 29 Jun 2004 01:06:36 -0500 Dmitry Torokhov <[email protected]>…

When CONFIG_NET_CLS_ACT is not set 'result' variable in prio_classify is unused. Also I was looking over the rest of the module and had hard time understanding the logic in prio_enqueue - I rearranged it a bit for better readability. Plus there are some formatting changes.

Looks good to me, applied.

Thanks Dmitry.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw
Quoted 4 lines · On 29 Jun 2004 17:42:10 -0400 jamal <[email protected]> wrote:

netdev swallowed my response to Dmitry earlier. Bacchus please flog the admin for me.
This fix will conflict with the other qdisc patch we are discussing. Basically thats what my message to Dmitry was earlier.

Should I back Dmitry's change out for now then?

#4 lore raw
Quoted 1 line · David S. Miller

Should I back Dmitry's change out for now then?

Yes please.

cheers,
jamal

#5 lore raw
Quoted 6 lines · On 29 Jun 2004 18:16:20 -0400 jamal <[email protected]> wrote:
Quoted 1 line · David S. Miller

Should I back Dmitry's change out for now then?

Yes please.

Will do.

#6 lore raw
In reply to jamal
Quoted 7 lines · jamal
Quoted 1 line · David S. Miller

Should I back Dmitry's change out for now then?

Yes please.

I'll be back ;) when your changes are committed.

Signature · 1 line
Dmitry