* [PATCH] net_sched: gred: Fix oops in gred_dump() in WRED mode
@ 2012-04-15 22:31 David Ward
2012-04-15 22:31 ` [PATCH] net_sched: red: Make minor corrections to comments David Ward
2012-04-17 3:51 ` [PATCH] net_sched: gred: Fix oops in gred_dump() in WRED mode David Miller
0 siblings, 2 replies; 6+ messages in thread
From: David Ward @ 2012-04-15 22:31 UTC (permalink / raw)
To: netdev; +Cc: David Ward
A parameter set exists for WRED mode, called wred_set, to hold the same
values for qavg and qidlestart across all VQs. The WRED mode values had
been previously held in the VQ for the default DP. After these values
were moved to wred_set, the VQ for the default DP was no longer created
automatically (so that it could be omitted on purpose, to have packets
in the default DP enqueued directly to the device without using RED).
However, gred_dump() was overlooked during that change; in WRED mode it
still reads qavg/qidlestart from the VQ for the default DP, which might
not even exist. As a result, this command sequence will cause an oops:
tc qdisc add dev $DEV handle $HANDLE parent $PARENT gred setup \
DPs 3 default 2 grio
tc qdisc change dev $DEV handle $HANDLE gred DP 0 prio 8 $RED_OPTIONS
tc qdisc change dev $DEV handle $HANDLE gred DP 1 prio 8 $RED_OPTIONS
This fixes gred_dump() in WRED mode to use the values held in wred_set.
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
net/sched/sch_gred.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index 55e3310..ab620bf 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -567,11 +567,8 @@ static int gred_dump(struct Qdisc *sch, struct sk_buff *skb)
opt.packets = q->packetsin;
opt.bytesin = q->bytesin;
- if (gred_wred_mode(table)) {
- q->vars.qidlestart =
- table->tab[table->def]->vars.qidlestart;
- q->vars.qavg = table->tab[table->def]->vars.qavg;
- }
+ if (gred_wred_mode(table))
+ gred_load_wred_set(table, q);
opt.qave = red_calc_qavg(&q->parms, &q->vars, q->vars.qavg);
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] net_sched: red: Make minor corrections to comments
2012-04-15 22:31 [PATCH] net_sched: gred: Fix oops in gred_dump() in WRED mode David Ward
@ 2012-04-15 22:31 ` David Ward
2012-04-16 13:06 ` Ward, David - 0663 - MITLL
2012-04-17 3:51 ` [PATCH] net_sched: gred: Fix oops in gred_dump() in WRED mode David Miller
1 sibling, 1 reply; 6+ messages in thread
From: David Ward @ 2012-04-15 22:31 UTC (permalink / raw)
To: netdev; +Cc: David Ward
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
include/net/red.h | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/net/red.h b/include/net/red.h
index 77d4c37..c0bc25e 100644
--- a/include/net/red.h
+++ b/include/net/red.h
@@ -28,7 +28,7 @@
the inertia of the algorithm. To allow larger bursts, W should be
decreased.
- if (avg > th_max) -> packet marked (dropped).
+ if (avg > th_max) -> packet marked (dropped) with probability max_P.
if (avg < th_min) -> packet passes.
if (th_min < avg < th_max) we calculate probability:
@@ -245,7 +245,7 @@ static inline unsigned long red_calc_qavg_from_idle_time(const struct red_parms
*
* dummy packets as a burst after idle time, i.e.
*
- * p->qavg *= (1-W)^m
+ * v->qavg *= (1-W)^m
*
* This is an apparently overcomplicated solution (f.e. we have to
* precompute a table to make this calculation in reasonable time)
@@ -279,7 +279,7 @@ static inline unsigned long red_calc_qavg_no_idle_time(const struct red_parms *p
unsigned int backlog)
{
/*
- * NOTE: p->qavg is fixed point number with point at Wlog.
+ * NOTE: v->qavg is fixed point number with point at Wlog.
* The formula below is equvalent to floating point
* version:
*
@@ -390,7 +390,7 @@ static inline void red_adaptative_algo(struct red_parms *p, struct red_vars *v)
if (red_is_idling(v))
qavg = red_calc_qavg_from_idle_time(p, v);
- /* p->qavg is fixed point number with point at Wlog */
+ /* v->qavg is fixed point number with point at Wlog */
qavg >>= p->Wlog;
if (qavg > p->target_max && p->max_P <= MAX_P_MAX)
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net_sched: red: Make minor corrections to comments
2012-04-15 22:31 ` [PATCH] net_sched: red: Make minor corrections to comments David Ward
@ 2012-04-16 13:06 ` Ward, David - 0663 - MITLL
2012-04-16 13:17 ` [PATCH v2] " David Ward
0 siblings, 1 reply; 6+ messages in thread
From: Ward, David - 0663 - MITLL @ 2012-04-16 13:06 UTC (permalink / raw)
To: netdev@vger.kernel.org
On 04/15/2012 06:31 PM, Ward, David - 0663 - MITLL wrote:
> diff --git a/include/net/red.h b/include/net/red.h
> index 77d4c37..c0bc25e 100644
> --- a/include/net/red.h
> +++ b/include/net/red.h
> @@ -28,7 +28,7 @@
> the inertia of the algorithm. To allow larger bursts, W should be
> decreased.
>
> - if (avg> th_max) -> packet marked (dropped).
> + if (avg> th_max) -> packet marked (dropped) with probability max_P.
> if (avg< th_min) -> packet passes.
> if (th_min< avg< th_max) we calculate probability:
>
Sorry, this line was correct as it was. (Misleading man page tc-red(8)
says "Because probability is normally not set to 100%, the queue size
might conceivably rise above max bytes")
Re-sending with just the other changes (and a patch for that man page).
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] net_sched: red: Make minor corrections to comments
2012-04-16 13:06 ` Ward, David - 0663 - MITLL
@ 2012-04-16 13:17 ` David Ward
2012-04-17 3:53 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: David Ward @ 2012-04-16 13:17 UTC (permalink / raw)
To: netdev; +Cc: David Ward
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
include/net/red.h | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/red.h b/include/net/red.h
index 77d4c37..ef46058 100644
--- a/include/net/red.h
+++ b/include/net/red.h
@@ -245,7 +245,7 @@ static inline unsigned long red_calc_qavg_from_idle_time(const struct red_parms
*
* dummy packets as a burst after idle time, i.e.
*
- * p->qavg *= (1-W)^m
+ * v->qavg *= (1-W)^m
*
* This is an apparently overcomplicated solution (f.e. we have to
* precompute a table to make this calculation in reasonable time)
@@ -279,7 +279,7 @@ static inline unsigned long red_calc_qavg_no_idle_time(const struct red_parms *p
unsigned int backlog)
{
/*
- * NOTE: p->qavg is fixed point number with point at Wlog.
+ * NOTE: v->qavg is fixed point number with point at Wlog.
* The formula below is equvalent to floating point
* version:
*
@@ -390,7 +390,7 @@ static inline void red_adaptative_algo(struct red_parms *p, struct red_vars *v)
if (red_is_idling(v))
qavg = red_calc_qavg_from_idle_time(p, v);
- /* p->qavg is fixed point number with point at Wlog */
+ /* v->qavg is fixed point number with point at Wlog */
qavg >>= p->Wlog;
if (qavg > p->target_max && p->max_P <= MAX_P_MAX)
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net_sched: gred: Fix oops in gred_dump() in WRED mode
2012-04-15 22:31 [PATCH] net_sched: gred: Fix oops in gred_dump() in WRED mode David Ward
2012-04-15 22:31 ` [PATCH] net_sched: red: Make minor corrections to comments David Ward
@ 2012-04-17 3:51 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2012-04-17 3:51 UTC (permalink / raw)
To: david.ward; +Cc: netdev
From: David Ward <david.ward@ll.mit.edu>
Date: Sun, 15 Apr 2012 18:31:45 -0400
> A parameter set exists for WRED mode, called wred_set, to hold the same
> values for qavg and qidlestart across all VQs. The WRED mode values had
> been previously held in the VQ for the default DP. After these values
> were moved to wred_set, the VQ for the default DP was no longer created
> automatically (so that it could be omitted on purpose, to have packets
> in the default DP enqueued directly to the device without using RED).
>
> However, gred_dump() was overlooked during that change; in WRED mode it
> still reads qavg/qidlestart from the VQ for the default DP, which might
> not even exist. As a result, this command sequence will cause an oops:
>
> tc qdisc add dev $DEV handle $HANDLE parent $PARENT gred setup \
> DPs 3 default 2 grio
> tc qdisc change dev $DEV handle $HANDLE gred DP 0 prio 8 $RED_OPTIONS
> tc qdisc change dev $DEV handle $HANDLE gred DP 1 prio 8 $RED_OPTIONS
>
> This fixes gred_dump() in WRED mode to use the values held in wred_set.
>
> Signed-off-by: David Ward <david.ward@ll.mit.edu>
Applied and queued up for -stable, thanks David.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-17 3:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-15 22:31 [PATCH] net_sched: gred: Fix oops in gred_dump() in WRED mode David Ward
2012-04-15 22:31 ` [PATCH] net_sched: red: Make minor corrections to comments David Ward
2012-04-16 13:06 ` Ward, David - 0663 - MITLL
2012-04-16 13:17 ` [PATCH v2] " David Ward
2012-04-17 3:53 ` David Miller
2012-04-17 3:51 ` [PATCH] net_sched: gred: Fix oops in gred_dump() in WRED mode David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).