Skip to contentSkip to conversation

net_sched: gred: red_calc_qavg() called with current qavg for backlog?

DISCUSSION Ward, David - 0663 - MITLL 3 participants 3 messages Apr 21, 2012 – Apr 24, 2012

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

In net/sched/sch_gred.c:

static int gred_dump(struct Qdisc *sch, struct sk_buff *skb)

{
     struct gred_sched *table = qdisc_priv(sch);
...
     for (i = 0; i < MAX_DPs; i++) {
         struct gred_sched_data *q = table->tab[i];
         struct tc_gred_qopt opt;
...
         opt.qave = red_calc_qavg(&q->parms, &q->vars, q->vars.qavg);

I can't tell if red_calc_qavg is intentionally being passed the current qavg as the backlog (which effectively causes qavg to only be re-calculated if we are idling)? Or should this be:

         opt.qave = red_calc_qavg(&q->parms,
                                  &q->vars,
                                  gred_backlog(table, q, sch));

Thanks,

David

maintainer
#1 lore raw

From: "Ward, David - 0663 - MITLL" <[email protected]>
Date: Sat, 21 Apr 2012 14:46:31 -0400

Quoted 13 lines

In net/sched/sch_gred.c:

static int gred_dump(struct Qdisc *sch, struct sk_buff *skb)

{
     struct gred_sched *table = qdisc_priv(sch);
...
     for (i = 0; i < MAX_DPs; i++) {
         struct gred_sched_data *q = table->tab[i];
         struct tc_gred_qopt opt;
...
         opt.qave = red_calc_qavg(&q->parms, &q->vars, q->vars.qavg);
Quoted 7 lines

I can't tell if red_calc_qavg is intentionally being passed the current qavg as the backlog (which effectively causes qavg to only be re-calculated if we are idling)? Or should this be:

         opt.qave = red_calc_qavg(&q->parms,
                                  &q->vars,
                                  gred_backlog(table, q, sch));

Looking at commit 22b33429ab93155895854e9518a253680a920493 ("[PKT_SCHED]: GRED: Use new generic red interface") it appears that this line is intentional so that the dump reports the same qave as it would have before Thomas's changes.

Thomas?

#2 lore raw
Quoted 20 lines · David Miller

From: "Ward, David - 0663 - MITLL" <[email protected]>
Date: Sat, 21 Apr 2012 14:46:31 -0400

Quoted 17 lines

In net/sched/sch_gred.c:

static int gred_dump(struct Qdisc *sch, struct sk_buff *skb)

{
     struct gred_sched *table = qdisc_priv(sch);
...
     for (i = 0; i < MAX_DPs; i++) {
         struct gred_sched_data *q = table->tab[i];
         struct tc_gred_qopt opt;
...
         opt.qave = red_calc_qavg(&q->parms, &q->vars, q->vars.qavg);

I can't tell if red_calc_qavg is intentionally being passed the current qavg as the backlog (which effectively causes qavg to only be re-calculated if we are idling)? Or should this be:

Quoted 8 lines
Quoted 3 lines
         opt.qave = red_calc_qavg(&q->parms,
                                  &q->vars,
                                  gred_backlog(table, q, sch));

Looking at commit 22b33429ab93155895854e9518a253680a920493 ("[PKT_SCHED]: GRED: Use new generic red interface") it appears that this line is intentional so that the dump reports the same qave as it would have before Thomas's changes.

When doing the convert, I simply preserved behaviour. I am not sure on what basis the original behaviour has been written.