[Cake] set filtered acks to bulk mode

[Cake] set filtered acks to bulk mode

[Dave Taht]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

The attached patch attempts to deprioritize bulk ack flows in cake.

Once we start accumulating enough acks to filter out, and we start
filtering them out, the "sparse flow optimization" in cake will
start prioritizing the shorter queues. This patch attempts to stop
that, which should give more time for more acks to be eliminated,
and deprioritize bulk ack flows slightly in favor of other traffic
that needs less latency.

Interesting stats from testing would be to measure a change in
ack_drops between the two versions, and changes in the rate of
increase of tcp. The 50x1 settings would be the most dramatic test...

-- 

Dave Täht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619

[-- Attachment #2: 0001-set-filtered-ack-flows-to-bulk-mode.patch --]
[-- Type: text/x-patch, Size: 1220 bytes --]

From dd5bb48a4744cbd5e3081379e1e0954a31fbca02 Mon Sep 17 00:00:00 2001
From: Dave Taht <dave.taht@gmail.com>
Date: Tue, 5 Dec 2017 13:52:30 -0800
Subject: [PATCH] set filtered ack flows to bulk mode

Once we start accumulating enough acks to filter out, and we start
filtering them out, the "sparse flow optimization" in cake will
start prioritizing the shorter queues. This patch attempts to stop
that, which should give more time for more acks to be eliminated,
and deprioritize the ack flows slightly in favor of other traffic
that needs less latency.
---
 sch_cake.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sch_cake.c b/sch_cake.c
index 4010388..72ab02b 100644
--- a/sch_cake.c
+++ b/sch_cake.c
@@ -1492,6 +1492,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 				ack = cake_ack_filter(q, flow);
 
 			if (ack) {
+				flow->set = CAKE_SET_BULK;
 				b->ack_drops++;
 				sch->qstats.drops++;
 				b->bytes += ack->len;
@@ -1532,6 +1533,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 			ack = cake_ack_filter(q, flow);
 
 		if (ack) {
+			flow->set = CAKE_SET_BULK;
 			b->ack_drops++;
 			sch->qstats.drops++;
 			b->bytes += qdisc_pkt_len(ack);
-- 
2.7.4

Re: [Cake] set filtered acks to bulk mode

[Ryan Mounce]

This appears to panic (and has, very indirectly, caused my LEDE router
to wipe itself). Will take a closer look later.

Regards,
Ryan Mounce


On 6 December 2017 at 08:30, Dave Taht <dave.taht@gmail.com> wrote:
> The attached patch attempts to deprioritize bulk ack flows in cake.
>
> Once we start accumulating enough acks to filter out, and we start
> filtering them out, the "sparse flow optimization" in cake will
> start prioritizing the shorter queues. This patch attempts to stop
> that, which should give more time for more acks to be eliminated,
> and deprioritize bulk ack flows slightly in favor of other traffic
> that needs less latency.
>
> Interesting stats from testing would be to measure a change in
> ack_drops between the two versions, and changes in the rate of
> increase of tcp. The 50x1 settings would be the most dramatic test...
>
> --
>
> Dave Täht
> CEO, TekLibre, LLC
> http://www.teklibre.com
> Tel: 1-669-226-2619
>
> _______________________________________________
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake
>

Re: [Cake] set filtered acks to bulk mode

[Jonathan Morton]

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

This is all sorts of wrong.

First, setting the enum isn't sufficient to actually make the flow bulk.
You need to move it to the bulk queue-of-queues and update the stats
counters too.  That's probably why it's crashing for Ryan.

Second, I fail to see why this is necessary or even desirable.  If there
are enough acks to activate the filter, then I assume it should be treated
as bulk already.  Or, if the filter itself thins out the acks enough to
make it sparse, then it still makes sense to deliver those acks timely so
as to maximise the reduced information they carry.  Remember too that Codel
implements head drop precisely to minimise the feedback lag via ack.

- Jonathan Morton

[-- Attachment #2: Type: text/html, Size: 783 bytes --]

Re: [Cake] set filtered acks to bulk mode

[Dave Taht]

On Tue, Dec 5, 2017 at 10:08 PM, Jonathan Morton <chromatix99@gmail.com> wrote:
> This is all sorts of wrong.

Going 1 out of 2 on untested patches for the day is not too horrible.

(sorry for crashing your router Ryan!!)

> First, setting the enum isn't sufficient to actually make the flow bulk.
> You need to move it to the bulk queue-of-queues and update the stats
> counters too.  That's probably why it's crashing for Ryan.

I had figured that would happen on the next round. :(

>
> Second, I fail to see why this is necessary or even desirable.  If there are
> enough acks to activate the filter, then I assume it should be treated as
> bulk already.  Or, if the filter itself thins out the acks enough to make it
> sparse, then it still makes sense to deliver those acks timely

Consider the case where you are emptying the queue on a regular basis.

As this is indeed, a bulk flow at this point, there is no need to give
it any extra boost.

>  so as to
> maximise the reduced information they carry.  Remember too that Codel
> implements head drop precisely to minimise the feedback lag via ack.
>
> - Jonathan Morton



-- 

Dave Täht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619

Re: [Cake] set filtered acks to bulk mode

[Sebastian Moeller]

> On Dec 6, 2017, at 08:05, Dave Taht <dave.taht@gmail.com> wrote:
> 
> On Tue, Dec 5, 2017 at 10:08 PM, Jonathan Morton <chromatix99@gmail.com> wrote:
>> This is all sorts of wrong.
> 
> Going 1 out of 2 on untested patches for the day is not too horrible.
> 
> (sorry for crashing your router Ryan!!)
> 
>> First, setting the enum isn't sufficient to actually make the flow bulk.
>> You need to move it to the bulk queue-of-queues and update the stats
>> counters too.  That's probably why it's crashing for Ryan.
> 
> I had figured that would happen on the next round. :(
> 
>> 
>> Second, I fail to see why this is necessary or even desirable.  If there are
>> enough acks to activate the filter, then I assume it should be treated as
>> bulk already.  Or, if the filter itself thins out the acks enough to make it
>> sparse, then it still makes sense to deliver those acks timely
> 
> Consider the case where you are emptying the queue on a regular basis.
> 
> As this is indeed, a bulk flow at this point, there is no need to give
> it any extra boost.

	Mmmh, I guess I see Jonathan's point more clearly, if the (egress) ack traffic is sparse treat it like that if it is not then not. The fact that the non-filtered ACK queue would be treated as bulk, is I believe immaterial.
Well, unless we have data showing that your proposed mode actually works better in real life. Data, I realize, for the acquisition of which you would need the proposed change... This might be too complicated for me ;)

Best Regards
	Sebastian


> 
>> so as to
>> maximise the reduced information they carry.  Remember too that Codel
>> implements head drop precisely to minimise the feedback lag via ack.
>> 
>> - Jonathan Morton
> 
> 
> 
> -- 
> 
> Dave Täht
> CEO, TekLibre, LLC
> http://www.teklibre.com
> Tel: 1-669-226-2619
> _______________________________________________
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake