netdev - Re: [PATCH] fq_codel: dont reinit flow state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAA93jw6mdAaomNfNWt0D4MZYFqXZ3=P+HYe9AEkvwNkAH84GSQ@mail.gmail.com>
Date:	Sat, 1 Sep 2012 11:12:29 -0700
From:	Dave Taht <dave.taht@...il.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] fq_codel: dont reinit flow state

partial NAK.

There is no need to reset dropped either, and resetting quantum or not
is an interesting question.

(in both cases they can be initted at init time)

Lastly, stats->maxpacket is presently a shared resource in fq_codel,
and the initial setting of 256 was an artifact some test ns2 code
(according to kathie). In the case of ack traffic owning a queue, this
can be 64 or 66 bytes (or larger on ipv6 acks).

It's totally unclear if this distinction is important in fq_codel at
present. :/. Obviously in codel which is a pure FIFO, maxpacket will
hit the max size really fast.

A related problem on maxpacket is if someone turns off tso/gso/ufo, it
remains set to the largest packet seen forever until the qdisc is
re-initialized.
This leads to really puzzling behavior... (I'll submit a patch for
this shortly but not change fq_codel's
centralized stats)

I'd be happy (for now) if dropped was preserved,
and to fiddle with the other ideas a while longer.

On Sat, Sep 1, 2012 at 6:19 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> When fq_codel builds a new flow, it should not reset codel state.
>
> Codel algo needs to get previous values (lastcount, drop_next) to get
> proper behavior.
>
> Signed-off-by: Dave Taht <dave.taht@...il.com>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> ---
>  net/sched/sch_fq_codel.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
> index 9fc1c62..4e606fc 100644
> --- a/net/sched/sch_fq_codel.c
> +++ b/net/sched/sch_fq_codel.c
> @@ -191,7 +191,6 @@ static int fq_codel_enqueue(struct sk_buff *skb, struct Qdisc *sch)
>
>         if (list_empty(&flow->flowchain)) {
>                 list_add_tail(&flow->flowchain, &q->new_flows);
> -               codel_vars_init(&flow->cvars);
>                 q->new_flow_count++;
>                 flow->deficit = q->quantum;
>                 flow->dropped = 0;
> @@ -418,6 +417,7 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt)
>                         struct fq_codel_flow *flow = q->flows + i;
>
>                         INIT_LIST_HEAD(&flow->flowchain);
> +                       codel_vars_init(&flow->cvars);
>                 }
>         }
>         if (sch->limit >= 1)
>
>

-- 
Dave Täht
http://www.bufferbloat.net/projects/cerowrt/wiki - "3.3.8-17 is out
with fq_codel!"
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html