All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Gautam Ramakrishnan <gautamramk@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>,
	davem@davemloft.net, netdev@vger.kernel.org,
	"Mohit P . Tahiliani" <tahiliani@nitk.edu.in>,
	"Sachin D . Patil" <sdp.sachin@gmail.com>,
	Mohit Bhasi <mohitbhasi1998@gmail.com>,
	"V . Saicharan" <vsaicharan1998@gmail.com>,
	Leslie Monis <lesliemonis@gmail.com>,
	Dave Taht <dave.taht@gmail.com>
Subject: Re: [RFC net-next 0/2] net: sched: add Flow Queue PIE AQM
Date: Tue, 02 Apr 2019 12:36:29 +0200	[thread overview]
Message-ID: <87d0m4puc2.fsf@toke.dk> (raw)
In-Reply-To: <CADAms0xgD+uzGUDJL35sAc+Yx9aS_5sNxXXSdCqebF4TkrzGpw@mail.gmail.com>

Gautam Ramakrishnan <gautamramk@gmail.com> writes:

> On Mon, Apr 01, 2019 at 02:04:19PM +0200, Toke Høiland-Jørgensen wrote:
>> Gautam Ramakrishnan <gautamramk@gmail.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

      reply	other threads:[~2019-04-02 10:36 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-31 17:40 [RFC net-next 0/2] net: sched: add Flow Queue PIE AQM Gautam Ramakrishnan
2019-03-31 17:40 ` [RFC net-next 1/2] net: sched: pie: refactor PIE Gautam Ramakrishnan
2019-03-31 17:40 ` [RFC net-next 2/2] net: sched: fq_pie: Flow Queue PIE AQM Gautam Ramakrishnan
2019-04-02 10:49   ` Toke Høiland-Jørgensen
2019-04-02 15:57     ` Gautam Ramakrishnan
2019-04-02 17:25       ` Toke Høiland-Jørgensen
2019-04-08  5:31         ` Gautam Ramakrishnan
2019-04-08  5:37           ` Dave Taht
2019-04-08  5:52             ` Dave Taht
2019-04-08  8:54           ` Toke Høiland-Jørgensen
2019-04-01 12:04 ` [RFC net-next 0/2] net: sched: add " Toke Høiland-Jørgensen
2019-04-01 13:31   ` Gautam Ramakrishnan
2019-04-02 10:36     ` Toke Høiland-Jørgensen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d0m4puc2.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=dave.taht@gmail.com \
    --cc=davem@davemloft.net \
    --cc=gautamramk@gmail.com \
    --cc=jhs@mojatatu.com \
    --cc=lesliemonis@gmail.com \
    --cc=mohitbhasi1998@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=sdp.sachin@gmail.com \
    --cc=tahiliani@nitk.edu.in \
    --cc=vsaicharan1998@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.