From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x843.google.com (mail-qt1-x843.google.com [IPv6:2607:f8b0:4864:20::843]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 700E03CB6C for ; Mon, 7 Jan 2019 14:57:58 -0500 (EST) Received: by mail-qt1-x843.google.com with SMTP id y20so1841105qtm.13 for ; Mon, 07 Jan 2019 11:57:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=P5biEct2+VncLqa3kINqyTzEUIvE1zGKpT+n4kGeUa0=; b=k5oX3ta2XOBvwJ8Mp4r+ShH+R1SPvanj6aQi0S62W3Hh24pA+eEMqyPH83b6ZCSOVu jAnAfkkp8JSZnd3YU1f4iXLTz+GwY5+FadXSXoavpowhrBuadiEv9BTyIt0m/ZUI0aIW B37ORneqb4TNOxlT6etIzMuWKHBTB3ueIb2nFrBo3TPeO+UVDpuLvxc9msks1QRCOJO+ eZSHZWtZwIYTOX+W+KqGFZsQsgE1dbqdcygypbJ/RiuWYt1PtZynU7l0pu7k8335zk5A CNexyRoFV9ttCTCypu448+RrzL0DXO4bPAfvpHE4Jm6PsEQPsKXWIzPqH3RYWqM8f4el 2MYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=P5biEct2+VncLqa3kINqyTzEUIvE1zGKpT+n4kGeUa0=; b=S7DbPtfS4r24W9SpjZpzVBzYPwj43sEo0aaRxLrbx1RKDRl9j83l7Puu8DMiAnLMen IwFNIZsLRf+PIMH0gb/jShNi5JogI35kDCwCvS8F9Qo30p7MXsCcDMudc+AUjB+eHLSV G3tmRTy0hMZeJTM2KOOGo4sFNXFUzkS2VX4iE3Yhm6AZwzgqI7NEtyiB648z2JBhUzMn XwRdfLcH3oLPolet401nBTILMgZJ4Ly3LONGdtfxJzdmrVzP6fmAs34vMLKqIWnO0xgQ 9uY81ICd5c7yPCUZMlef6W0w5jJ5Oe5bLm1DZhyM7ANLQdDtMmBy51+0Fn692BUMBeFs BIng== X-Gm-Message-State: AA+aEWb29rFbtNlqXQGfheb3oF+gZQ0BP47mWK1LtbF7dHPdASBEPygP jWBox3Z5keId0ckcmSRxuHTemYlOr0AqKS+/eY0= X-Google-Smtp-Source: ALg8bN4Lzaz1IBLsHHUuaVgvnV/KhzAAtgW+DvuyzNn3t87STi/KgbfUUSV6vsMZcs53icYDR3PnKdnpHl2YTFEft8s= X-Received: by 2002:ac8:6606:: with SMTP id c6mr60444829qtp.376.1546891077758; Mon, 07 Jan 2019 11:57:57 -0800 (PST) MIME-Version: 1.0 References: <20190107194733.31138-1-toke@toke.dk> <20190107194733.31138-3-toke@toke.dk> In-Reply-To: <20190107194733.31138-3-toke@toke.dk> From: Dave Taht Date: Mon, 7 Jan 2019 11:57:45 -0800 Message-ID: To: =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= Cc: Cake List , =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [Cake] [PATCH 2/4] sched: Fix detection of empty queues in child qdiscs X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Jan 2019 19:57:58 -0000 awesome. I'd (rarely) seen this bug in drr and qfq and never solved it, thus going for the all-in-one fq_codel.... On Mon, Jan 7, 2019 at 11:48 AM Toke H=C3=B8iland-J=C3=B8rgensen wrote: > > From: Toke H=C3=B8iland-J=C3=B8rgensen > > Several qdiscs check on enqueue whether the packet was enqueued to a clas= s > with an empty queue, in which case the class is activated. This is done b= y > checking if the qlen is exactly 1 after enqueue. However, if GSO splittin= g > is enabled in the child qdisc, a single packet can result in a qlen longe= r > than 1. This means the activation check fails, leading to a stalled queue= . > > Fix this by checking if the queue is empty *before* enqueue, and running > the activation logic if this was the case. > > Reported-by: Pete Heist > Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen > --- > net/sched/sch_drr.c | 4 +++- > net/sched/sch_hfsc.c | 4 +++- > net/sched/sch_qfq.c | 4 +++- > 3 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c > index feaf47178653..09b800991065 100644 > --- a/net/sched/sch_drr.c > +++ b/net/sched/sch_drr.c > @@ -354,6 +354,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qd= isc *sch, > struct drr_sched *q =3D qdisc_priv(sch); > struct drr_class *cl; > int err =3D 0; > + bool first; > > cl =3D drr_classify(skb, sch, &err); > if (cl =3D=3D NULL) { > @@ -363,6 +364,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qd= isc *sch, > return err; > } > > + first =3D !cl->qdisc->q.qlen; > err =3D qdisc_enqueue(skb, cl->qdisc, to_free); > if (unlikely(err !=3D NET_XMIT_SUCCESS)) { > if (net_xmit_drop_count(err)) { > @@ -372,7 +374,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qd= isc *sch, > return err; > } > > - if (cl->qdisc->q.qlen =3D=3D 1) { > + if (first) { > list_add_tail(&cl->alist, &q->active); > cl->deficit =3D cl->quantum; > } > diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c > index 6bb8f73a8473..24cc220a3218 100644 > --- a/net/sched/sch_hfsc.c > +++ b/net/sched/sch_hfsc.c > @@ -1542,6 +1542,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch= , struct sk_buff **to_free) > unsigned int len =3D qdisc_pkt_len(skb); > struct hfsc_class *cl; > int uninitialized_var(err); > + bool first; > > cl =3D hfsc_classify(skb, sch, &err); > if (cl =3D=3D NULL) { > @@ -1551,6 +1552,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch= , struct sk_buff **to_free) > return err; > } > > + first =3D !cl->qdisc->q.qlen; > err =3D qdisc_enqueue(skb, cl->qdisc, to_free); > if (unlikely(err !=3D NET_XMIT_SUCCESS)) { > if (net_xmit_drop_count(err)) { > @@ -1560,7 +1562,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch= , struct sk_buff **to_free) > return err; > } > > - if (cl->qdisc->q.qlen =3D=3D 1) { > + if (first) { > if (cl->cl_flags & HFSC_RSC) > init_ed(cl, len); > if (cl->cl_flags & HFSC_FSC) > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c > index 8d5e55d5bed2..29f5c4a24688 100644 > --- a/net/sched/sch_qfq.c > +++ b/net/sched/sch_qfq.c > @@ -1215,6 +1215,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct = Qdisc *sch, > struct qfq_class *cl; > struct qfq_aggregate *agg; > int err =3D 0; > + bool first; > > cl =3D qfq_classify(skb, sch, &err); > if (cl =3D=3D NULL) { > @@ -1236,6 +1237,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct = Qdisc *sch, > } > > gso_segs =3D skb_is_gso(skb) ? skb_shinfo(skb)->gso_segs : 1; > + first =3D !cl->qdisc->q.qlen; > err =3D qdisc_enqueue(skb, cl->qdisc, to_free); > if (unlikely(err !=3D NET_XMIT_SUCCESS)) { > pr_debug("qfq_enqueue: enqueue failed %d\n", err); > @@ -1253,7 +1255,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct = Qdisc *sch, > > agg =3D cl->agg; > /* if the queue was not empty, then done here */ > - if (cl->qdisc->q.qlen !=3D 1) { > + if (!first) { > if (unlikely(skb =3D=3D cl->qdisc->ops->peek(cl->qdisc)) = && > list_first_entry(&agg->active, struct qfq_class, alis= t) > =3D=3D cl && cl->deficit < len) > -- > 2.20.1 > > _______________________________________________ > Cake mailing list > Cake@lists.bufferbloat.net > https://lists.bufferbloat.net/listinfo/cake --=20 Dave T=C3=A4ht CTO, TekLibre, LLC http://www.teklibre.com Tel: 1-831-205-9740