From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f169.google.com (mail-wi0-f169.google.com [209.85.212.169]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client did not present a certificate) by huchra.bufferbloat.net (Postfix) with ESMTPS id 8BC0C21F095; Sun, 26 Aug 2012 14:36:29 -0700 (PDT) Received: by wibhm2 with SMTP id hm2so3053342wib.4 for ; Sun, 26 Aug 2012 14:36:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=gGVdDwee+qk9b8AiPiWGQJ3bkeM1IG2BcLZBS1WX1wc=; b=uQq2Vu9TMMdErNI7u4P3ICMNdqR2Be6T6DldlCLR5ToG7qrYz/uJeClhotBTbs1/02 9U7Zwdou8vbWFztMzB/fdZUd8NeB2gC1VjtgHrNr09w4WwCKmSRGw75Z7jqhloOraE4/ w7XKA78ce1Rb4PG6StTyQgvZXtYImLp9qzjaizLOvj/+4jdnbF7kR+f/8jJLTKhRG/zp tOkoBCbhc5RjQ4NhJH9ziX+r9SnOtwKNPQrE8Z3e9hXBsOU5HdlTwqVd6W1nwBMqEoAZ TqAEDeU5TMI1oJOXTbqrYDUF89Lzs5e3nlJK9so2m+uZBd60j4tCe7ezRSbUsJjIxK6q WJ3g== MIME-Version: 1.0 Received: by 10.216.181.67 with SMTP id k45mr5436804wem.17.1346016987529; Sun, 26 Aug 2012 14:36:27 -0700 (PDT) Received: by 10.223.159.134 with HTTP; Sun, 26 Aug 2012 14:36:27 -0700 (PDT) In-Reply-To: <1345538298.5158.401.camel@edumazet-glaptop> References: <1345538298.5158.401.camel@edumazet-glaptop> Date: Sun, 26 Aug 2012 14:36:27 -0700 Message-ID: From: Dave Taht To: Eric Dumazet Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: codel@lists.bufferbloat.net, cerowrt-devel@lists.bufferbloat.net, Felix Fietkau Subject: Re: [Codel] coping with memory limitations and packet flooding in codel and fq_codel X-BeenThere: codel@lists.bufferbloat.net X-Mailman-Version: 2.1.13 Precedence: list List-Id: CoDel AQM discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 26 Aug 2012 21:36:30 -0000 On Tue, Aug 21, 2012 at 1:38 AM, Eric Dumazet wrot= e: > On Mon, 2012-08-20 at 14:05 -0700, Dave Taht wrote: >> We've had a raft of deployment questions regarding fq_codel, memory >> use, and the impact of unresponsive flows >> over on the cerowrt development list. >> >> sort of summary of the thread here, with some codel related ideas >> >> https://lists.bufferbloat.net/pipermail/cerowrt-devel/2012-August/000423= .html >> >> some early data on wifi interactions here >> >> https://lists.bufferbloat.net/pipermail/cerowrt-devel/2012-August/000407= .html >> > > CC netdev > > codel / fq_codel have a packet limit per qdisc, like any other linux > qdisc. > > DEFAULT_CODEL_LIMIT =3D 1000 packets > > fq_codel : 10240 packets > > For memory constrained boxes, it would be wise to add a 'truesize' limit >From looking over the history of this idea, it does seem to be a good idea for small devices with potentially big queues. http://www.spinics.net/lists/netdev/msg176967.html That said I do tend to agree with davem's summary in fixing the wifi drivers in the first place. The current allocation in the ath9k driver doesn't make any sense in the first place, which (to me) implies magic lies underneath that I'm reluctant to fiddle with, without deep knowledge of how the ath9k driver behaves with wep/wpa/ampdus, etc. > Not a bytes limit based on skb->len sum, because a skb->len =3D 60 bytes > packet might consume far more memory (more than 2 Kbytes) > > ath9k driver is known to deliver very fat skbs (skb->truesize is too > big) > > One 'other way' to limit bloat would be to reallocate skbs when one > qdisc begins to use more than 25 % of its truesize limit > > enqueue(q, skb) > { > if (q->truesize_sum > q->truesize_limit / 4) > skb =3D skb_reduce_truesize(skb, GFP_ATOMIC); > ... > } This basically involves adding truesize accounting to the codel qdisc and settling on the api proposed above, which differs from the originally proposed patch. Is there a reason why we would ever not try this atomically and is this form correct for 3.3 and later kernels? (am about to hack this into skbuff.h and codel.h as I write) +/* + * Caller want to reduce memory needs before queueing skb + * The (expensive) copy should not be be done in fast path. + */ +static struct sk_buff *skb_reduce_truesize(struct sk_buff *skb) +{ + if (skb->truesize > 2 * SKB_TRUESIZE(skb->len)) { + struct sk_buff *nskb; + + nskb =3D skb_copy_expand(skb, skb_headroom(skb), 0, + GFP_ATOMIC | __GFP_NOWARN); + if (nskb) { + __kfree_skb(skb); + skb =3D nskb; + } + } + return skb; +} > This is what some drivers already do in their rx handler (this is called > copybreak), but this would do the copy (and extra cpu cost) only in > stress situations. > > Note : we also have a very big struct skb_shared_info (320 bytes), we > probably could shrink it for packets without fragments. The ath9k allocator failing is at 19xx bytes. --=20 Dave T=E4ht http://www.bufferbloat.net/projects/cerowrt/wiki - "3.3.8-17 is out with fq_codel!"