From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id E9BCF3B25D; Mon, 2 May 2016 03:47:25 -0400 (EDT) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CAD048553B; Mon, 2 May 2016 07:47:24 +0000 (UTC) Received: from localhost (ovpn-200-47.brq.redhat.com [10.40.200.47]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u427lLu9032107; Mon, 2 May 2016 03:47:22 -0400 Date: Mon, 2 May 2016 09:47:18 +0200 From: Jesper Dangaard Brouer To: Eric Dumazet Cc: Jonathan Morton , make-wifi-fast@lists.bufferbloat.net, "codel@lists.bufferbloat.net" , ath10k Message-ID: <20160502094718.56d8e7b0@redhat.com> In-Reply-To: <1462132553.5535.207.camel@edumazet-glaptop3.roam.corp.google.com> References: <1462125592.5535.194.camel@edumazet-glaptop3.roam.corp.google.com> <865DA393-262D-40B6-A9D3-1B978CD5F6C6@gmail.com> <1462128385.5535.200.camel@edumazet-glaptop3.roam.corp.google.com> <1462132553.5535.207.camel@edumazet-glaptop3.roam.corp.google.com> Organization: Red Hat Inc. MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Mailman-Approved-At: Mon, 02 May 2016 15:49:25 -0400 Subject: Re: [Make-wifi-fast] [Codel] fq_codel_drop vs a udp flood X-BeenThere: make-wifi-fast@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 May 2016 07:47:26 -0000 On Sun, 01 May 2016 12:55:53 -0700 Eric Dumazet wrote: > On Sun, 2016-05-01 at 11:46 -0700, Eric Dumazet wrote: > > > Just drop half backlog packets instead of 1, (maybe add a cap of 64 > > packets to avoid too big burts of kfree_skb() which might add cpu > > spikes) and problem is gone. > > > > I used following patch and it indeed solved the issue in my tests. > > (Not the DDOS case, but when few fat flows are really bad citizens) > > diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c > index a5e420b3d4ab..0cb8699624bc 100644 > --- a/net/sched/sch_fq_codel.c > +++ b/net/sched/sch_fq_codel.c > @@ -135,11 +135,11 @@ static inline void flow_queue_add(struct fq_codel_flow *flow, > skb->next = NULL; > } > > -static unsigned int fq_codel_drop(struct Qdisc *sch) > +static unsigned int fq_codel_drop(struct Qdisc *sch, unsigned int max) > { > struct fq_codel_sched_data *q = qdisc_priv(sch); > struct sk_buff *skb; > - unsigned int maxbacklog = 0, idx = 0, i, len; > + unsigned int maxbacklog = 0, idx = 0, i, len = 0; > struct fq_codel_flow *flow; > > /* Queue is full! Find the fat flow and drop packet from it. > @@ -153,15 +153,26 @@ static unsigned int fq_codel_drop(struct Qdisc *sch) > idx = i; > } > } > + /* As the search was painful, drop half bytes of this fat flow. > + * Limit to max packets to not inflict too big latencies, > + * as kfree_skb() might be quite expensive. > + */ > + maxbacklog >>= 1; > + > flow = &q->flows[idx]; > - skb = dequeue_head(flow); > - len = qdisc_pkt_len(skb); > + for (i = 0; i < max;) { > + skb = dequeue_head(flow); > + len += qdisc_pkt_len(skb); > + kfree_skb(skb); > + i++; > + if (len >= maxbacklog) > + break; > + } What about using bulk free of SKBs here? There is a very high probability that we are hitting SLUB slowpath, which involves a locked cmpxchg_double per packet. Instead we can amortize this cost via kmem_cache_free_bulk(). Maybe extend kfree_skb_list() to hide the slab/kmem_cache call? > + sch->qstats.drops += i; > + sch->qstats.backlog -= len; > q->backlogs[idx] -= len; > - sch->q.qlen--; > - qdisc_qstats_drop(sch); > - qdisc_qstats_backlog_dec(sch, skb); > - kfree_skb(skb); > - flow->dropped++; > + sch->q.qlen -= i; > + flow->dropped += i; > return idx; > } -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer