From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-x233.google.com (mail-ob0-x233.google.com [IPv6:2607:f8b0:4003:c01::233]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by huchra.bufferbloat.net (Postfix) with ESMTPS id 9C41A21F345 for ; Tue, 24 Nov 2015 02:48:40 -0800 (PST) Received: by obdgf3 with SMTP id gf3so9973959obd.3 for ; Tue, 24 Nov 2015 02:48:38 -0800 (PST) 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=AdG2J8jgkJtBJxsSZuYhqxtLWxskxLAPZ+7dB361MWw=; b=M64BLFqE+OTxFA9rZu/s0VkJVVvU679u9GP8zai3ELRTK/6eFDvNoATWK+mxDqyzaX s2NdihuBF2kamumPK5F1tSVvrGUnoQSUuLPWjg6EFywj7wbfFKpbSviWKcvx8zrvx24t NCOEVVxFvtq+uRWVpxQ8UjlCRSavd8CfLGMuh0tell0FQr+S4bLRhhEo8N93gcem4hhg KaPW96rakjJzv1Ts4J7diFBE0Cg+W0qFubbqKEBz/0b/55sP+WJjM+NZ56a3LINDCe5K pMwYn+/z8zCZV0I0cJoTmbGE5AVVCw95+kuf2IcP4xBiNJzui4Ym3RPF7mNd+CJTn7yy v3Ww== MIME-Version: 1.0 X-Received: by 10.60.39.200 with SMTP id r8mr5357155oek.81.1448362118790; Tue, 24 Nov 2015 02:48:38 -0800 (PST) Received: by 10.202.50.130 with HTTP; Tue, 24 Nov 2015 02:48:38 -0800 (PST) In-Reply-To: <56542A13.3010307@darbyshire-bryant.me.uk> References: <56542A13.3010307@darbyshire-bryant.me.uk> Date: Tue, 24 Nov 2015 11:48:38 +0100 Message-ID: From: Dave Taht To: Kevin Darbyshire-Bryant Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: "cake@lists.bufferbloat.net" Subject: Re: [Cake] GSO peel behaviour tweaks X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.13 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Nov 2015 10:49:02 -0000 I don't know what this used to look like but it is essentially wrong in both (all?) versions. - q->peel_threshold =3D (q->rate_flags & CAKE_FLAG_ATM) ? - 0 : min(65535U, q->rate_bps >> 12); + q->peel_threshold =3D (q->rate_flags & CAKE_FLAG_ATM) || + q->rate_overhead ? 0 : min(65535U, q->rate_bps >> 1= 2); What we want to do is closer to: A) start peeling once we start accruing or incurring delay in excess of, say, 250usec. At 1Mbit, this is basically peel always. At a gbit, it's peel with roughly two 10 full-size packet offloads in play. There are nuances vs a vs ack GRO stuff (served with a 300 quantum in fq_codel), and in the 10-100Mbit range... A1) So doing nothing at a rate unlimited is wrong A2) Taking the current len * flows as a way to calculate it is wrong A3) I don't know if this was ever "right". It doesn't need to be perfect, but this is far from right... While I am unfond of the rate estimator's overhead, it perhaps could be used to calculate the peel threshold in a saner way... B) always peel when we are trying to do accurate on-wire accounting. As for the other patch... In general random pointer lookups into memory (like the skb->gro pointer) cost more than math as the other two params here are possibly part of a local cache hit already... and I have no idea what the ratio is between gso packets and how often you'd hit the comparison... but see point A2 above... - if (unlikely((len * max_t(u32, b->bulk_flow_count, 1U) > - q->peel_threshold && skb_is_gso(skb)))) { + if (unlikely(skb_is_gso(skb) && + (len * max_t(u32, b->bulk_flow_count, 1U) > + q->peel_threshold))) { Dave T=C3=A4ht Let's go make home routers and wifi faster! With better software! https://www.gofundme.com/savewifi On Tue, Nov 24, 2015 at 10:12 AM, Kevin Darbyshire-Bryant wrote: > I've just pushed 2 commits related to GSO peeling behaviour to master. > > 1st tweak is at worst benign and at best removes a multiply compare for > every packet enqueued. I'd like to think the optimiser in the compiler > would have done what I've done explicitly (in essence check this is a > gso packet 1st before thinking about peeling it) but when I checked on > x86_64 there was a definite difference in produced code. > > 2nd tweak is *not* benign. In essence this forces peeling if either ATM > framing or packet overhead is specified. Previously only ATM framing > forced peeling. I think this is more correct but unfortunately will be > slower. > > Commits can be reverted - feel free :-) > > > _______________________________________________ > Cake mailing list > Cake@lists.bufferbloat.net > https://lists.bufferbloat.net/listinfo/cake >