Hello Jakub and Toke,

Thanks for the comments. We shall make the required changes and
resubmit v4 once we get some more feedback from the mailing list,
probably by tomorrow.

Thank You,
Gautam

On 1/13/20, Toke Høiland-Jørgensen [off-list ref] wrote:

Jakub Kicinski [off-list ref] writes:

quoted

On Mon, 13 Jan 2020 12:44:38 +0100, Toke Høiland-Jørgensen wrote:

quoted

Jakub Kicinski [off-list ref] writes:

quoted

On Fri, 10 Jan 2020 11:56:57 +0530, gautamramk@gmail.com wrote:

quoted

From: "Mohit P. Tahiliani" <redacted>

Principles:
  - Packets are classified on flows.
  - This is a Stochastic model (as we use a hash, several flows might
                                be hashed on the same slot)
  - Each flow has a PIE managed queue.
  - Flows are linked onto two (Round Robin) lists,
    so that new flows have priority on old ones.
  - For a given flow, packets are not reordered.
  - Drops during enqueue only.
  - ECN capability is off by default.
  - ECN threshold is at 10% by default.
  - Uses timestamps to calculate queue delay by default.

Usage:
tc qdisc ... fq_pie [ limit PACKETS ] [ flows NUMBER ]
                    [ alpha NUMBER ] [ beta NUMBER ]
                    [ target TIME us ] [ tupdate TIME us ]
                    [ memory_limit BYTES ] [ quantum BYTES ]
                    [ ecnprob PERCENTAGE ] [ [no]ecn ]
                    [ [no]bytemode ] [ [no_]dq_rate_estimator ]

defaults:
  limit: 10240 packets, flows: 1024
  alpha: 1/8, beta : 5/4
  target: 15 ms, tupdate: 15 ms (in jiffies)
  memory_limit: 32 Mb, quantum: device MTU
  ecnprob: 10%, ecn: off
  bytemode: off, dq_rate_estimator: off

Some reviews below, but hopefully someone who knows more about qdiscs
will still review :)

I looked it over, and didn't find anything you hadn't already pointed
out below. It's pretty obvious that this started out as a copy of
sch_fq_codel. Which is good, because that's pretty solid. And bad,
because that means it introduces another almost-identical qdisc without
sharing any of the code...

I think it would be worthwhile to try to consolidate things at some
point. Either by just merging code from fq_{codel,pie}, but another
option would be to express fq_codel and fq_pie using the fq{,_impl}.h
includes. Maybe even sch_cake as well, but that may take a bit more
work. Not sure if we should require this before merging fq_pie, or just
leave it as a possible enhancement for later? WDYT?

Tricky :/ No strong opinion on my side. I'm already a little weary of
added function calls in the fast path (e.g. pie_drop_early()), but using
some static inlines wouldn't hurt... Then again since fq_codel doesn't
use fq{,_impl}.h it indeed seems like a bigger project to clean things
up.

Yeah, definitely a bigger project; and I do worry about regressions.
Especially since fq{,_impl}.h relies heavily on indirect calls...

quoted

IMHO if this qdisc works and is useful it could probably be merged as
is. Hopefully we can get an opinion on this from Stephen or others.

OK, fine with me :)

-Toke

-- 
-------------
Gautam |