netdev - Re: [RFC net-next 0/2] net: sched: add Flow Queue PIE AQM

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: <87d0m4puc2.fsf@toke.dk>
Date:   Tue, 02 Apr 2019 12:36:29 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Gautam Ramakrishnan <gautamramk@...il.com>
Cc:     Jamal Hadi Salim <jhs@...atatu.com>, davem@...emloft.net,
        netdev@...r.kernel.org,
        "Mohit P . Tahiliani" <tahiliani@...k.edu.in>,
        "Sachin D . Patil" <sdp.sachin@...il.com>,
        Mohit Bhasi <mohitbhasi1998@...il.com>,
        "V . Saicharan" <vsaicharan1998@...il.com>,
        Leslie Monis <lesliemonis@...il.com>,
        Dave Taht <dave.taht@...il.com>
Subject: Re: [RFC net-next 0/2] net: sched: add Flow Queue PIE AQM

Gautam Ramakrishnan <gautamramk@...il.com> writes:

> On Mon, Apr 01, 2019 at 02:04:19PM +0200, Toke Høiland-Jørgensen wrote:
>> Gautam Ramakrishnan <gautamramk@...il.com> writes:
>>
>> > Flow Queue PIE packet scheduler
>> >
>> > This patch series implements the Flow Queue Proportional
>> > Integral Controller (FQ-PIE) active queue management
>> > algorithm. It is an enhancement over the PIE algorithm.
>> > It integrates the PIE aqm with a deficit round robin
>> > scheme.
>> >
>> > It is implemented over RFC 8033 aligned version of PIE.
>> > In this patch, Little's law is used for queue delay
>> > estimation like PIE. This patch doesn't yet have the
>> > timestamp based queue delay estimator like freebsd, that
>> > Dave Taht thinks is better.
>> >
>> > We are looking for suggestions on cleanups and
>> > improvements for this patch.
>>
>> Is the fq_ part basically identical to fq_codel? Might make sense to
>> share some code in that case; might not be a trivial refactoring,
>> though...
>>
>> -Toke
>
> The fq_ part is mostly the same, except for the batch dropping. But
> the structure of flows are different. Any particular suggestions on
> the refactoring?

Well, fq_pie_enqueue() is quite close to fq_codel_enqueue(); same thing
for *_dequeue(). So it should be possible to divide them up into a
common part, followed by the bits that differ. Then the common parts can
be factored out into a common function to be included by both.

You don't have to do the actual factoring out of the common bits in this
patch series, but it would be good if you could structure the fq_pie
code so it is as close to the fq_codel code as possible, to make it
easier to split out common parts later. This includes the enqueue and
dequeue functions, as well as the data structures. Doing this also has
the added bonus that you'll have to think about (and preferably explain
the reasoning behind) any deviations between the two variants.

I'll send a couple of concrete suggestions in a separate email.

-Toke