[Cake] Wifi Memory limits in small platforms

[Dave Taht]

People have a tendency to try and construct very complicated QoS
systems and then try to run them in limited memory. We see a lot of 6
or 7 class hfsc + sfq or fq_codel systems, which can accumulate
hundreds or thousands of packets before doing a drop. A stress test
like ping -f -s 1500 and ping -f -s 64 hitting every defined queue
(somehow) should be run to make sure you don't OOM, and even, even
then, things like acks coming off an ethernet eat 2k when they are
only 64 bytes in size.

this patch (Not even compile tested) might take some of the memory
pressure off when being flooded in this way. While
it costs cpu, given a choice between ooming and slowing down, I'd
rather slow down. Should have done tbf too.

I wonder if the large number of queues we've seen people try to use
with hfsc has been one of the sources of
frequent reports of flakyness? How many queues do people actually try
to create with it in the field... Certainly sfq's default of 127
packets is better than 1000...

Reducing the truesize could be added to cake and tbf also. As well as
the fq_codel for wifi code, on small platforms.

diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 433f2190960f..c0777ce4a259 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1535,7 +1535,8 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc
*sch, struct sk_buff **to_free)
        struct hfsc_class *cl;
        int uninitialized_var(err);
        bool first;
-
+       if (sch->q.qlen > 128)
+               skb = skb_reduce_truesize(skb);
        cl = hfsc_classify(skb, sch, &err);
        if (cl == NULL) {
                if (err & __NET_XMIT_BYPASS)
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 7bcf20ef9145..40a27392f88e 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -584,6 +584,9 @@ static int htb_enqueue(struct sk_buff *skb, struct
Qdisc *sch,
        struct htb_sched *q = qdisc_priv(sch);
        struct htb_class *cl = htb_classify(skb, sch, &ret);

+       if (sch->q.qlen > 128)
+               skb = skb_reduce_truesize(skb);
+
        if (cl == HTB_DIRECT) {
                /* enqueue to helper queue */
                if (q->direct_queue.qlen < q->direct_qlen) {

On Thu, Aug 22, 2019 at 6:15 AM Dave Taht <dave.taht at gmail.com> wrote:
>
> It's very good to know how much folk have been struggling to keep
> things from OOMing on 32MB platforms. I'd like to hope that the
> unified memory management in cake (vs a collection of QoS qdiscs) and
> the new fq_codel for wifi stuff (cutting it down to 1 alloc from four)
> help, massively on this issue, but until today I was unaware of how
> much the field may have been patching things out.
>
> The default 32MB memory limits in fq_codel comes from the stressing
> about 10GigE networking from google. 4MB is limit in openwrt,
> which is suitable for ~1Gbit, and is sort of there  due to 802.11ac's
> maximum (impossible to hit) of a txop that large.
>
> Something as small as 256K is essentially about 128 full size packets
> (and often, acks from an ethernet device's rx ring eat 2k).
>
> The structure of the new fq_codel for wifi subsystem is "one in the
> hardware, one ready to go, and the rest accumulating". I
> typically see about 13-20 packets in an aggregate. 256k strikes me as
> a bit small.
>
> I haven't checked, but does this patch still exist in openwrt/dd-wrt?
> It had helped a lot when under memory pressure from
> a lot of small packets.
>
> https://github.com/dtaht/cerowrt-3.10/blob/master/target/linux/generic/patches-3.10/657-qdisc_reduce_truesize.patch
>
> Arguably this could be made more aggressive, but it massively reduced
> memory burdens at the time I did it when
> flooding the device, or having lots of acks, and while it cost cpu it
> saved on ooming.
>
> There's two other dubious things in the fq_codel for wifi stack
> presently. Right now the codel target is set too high for p2p use
> (20ms, where 6ms seems more right), and it also flips up to a really
> high target and interval AND turns off ecn when there's more than a
> few stations available (rather than "active") - it's an overly
> conservative figure we used back when we had major issues with
> powersave
> and multicast that I'd hoped we could cut back to normal after we got
> another round of research funding and feedback from the field (which
> didn't happen, and we never got around to making it configurable, and
> being 25x better than it was before seemed "enough")
>
> I was puzzled at battlemesh as to why I had dropping at about 50ms
> delay rather than ecn, and thought it was something
> else, and this morning I'm thinking that folk have been reducing the
> memlimit to 256k rather....



-- 

Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740