From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.toke.dk (mail.toke.dk [52.28.52.200]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id E6CC93BA8E for ; Fri, 4 Jan 2019 17:34:58 -0500 (EST) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023; t=1546641267; bh=ex0SuLU5iOJbEMaOUCfpatTPxDphpVAWZBMx2Z+BRpU=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=C13nKufxTtS5OKjX8oyXT159t+W+CMKFeN3ussWi0yj73ZNVl4lzgJMMpR2isZESW QRo8NEkOXRL6YVvvRdTkEHPzSZlh0tCHjYGLw020t8wEgi1NoR42Rmp4SH0fduWPTn e0s/Iq7ckSrT/XuR3R11GottY/D3KiYEMTaR//Xzi/2lhlawXt/fjidleOf+2fizfN 08yfuU9KRQRSsu5CupxCgDEvqO1wV+FkXEporTvIosMOIYJ81bMiDiIHoTm9ZF0hVH O0t7BD0atA/DXe9qT4KsMWul3ZOb+TYVugY+H8KomadOCROPgf5k5HgeJvnNXH1o73 9vk4PliM9XFlw== To: Pete Heist Cc: Cake List In-Reply-To: <4C422792-7E51-4DBA-A229-FA7D3F987FB6@heistp.net> References: <5482A3CA-9C36-4DDE-A858-24D8467F70C7@heistp.net> <8736q8yumt.fsf@toke.dk> <4C422792-7E51-4DBA-A229-FA7D3F987FB6@heistp.net> Date: Fri, 04 Jan 2019 23:34:23 +0100 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87zhsgxdao.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Cake] cake infinite loop(?) with hfsc on one-armed router 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: Fri, 04 Jan 2019 22:34:59 -0000 Pete Heist writes: >> On Jan 4, 2019, at 10:34 PM, Toke H=C3=B8iland-J=C3=B8rgensen wrote: >>=20 >> Pete Heist writes: >>=20 >>> Ok, the lockup goes away if you use no-split-gso on the cake qdiscs for= the default traffic (noted below in the drr and hfsc cases with "!!! must = use no-split-gso here !!!"). Only I=E2=80=99d like my 600 =CE=BCs back. :) >>>=20 >>> This smells of a bug Toke fixed on Sep 12, 2018 in 42e87f12ea5c390bf5ee= b658c942bc810046160a, but then reverted in the next commit because it was f= ixed upstream. However, if I re-apply that commit, it still doesn=E2=80=99t= fix it. >>>=20 >>> Perhaps there are more cases where skb_reset_mac_len(skb) needs to be c= alled somewhere for VLAN support? >>>=20 >>> I managed to capture some output from what happens to hfsc: >>>=20 >>> [ 683.864456] ------------[ cut here ]------------ >>> [ 683.869116] WARNING: CPU: 1 PID: 11 at net/sched/sch_hfsc.c:1427 >>> 0xf9ced4ef() >>=20 >> So this seems to be this line: >>=20 >> WARN_ON(next_time =3D=3D 0); >>=20 >> See https://elixir.bootlin.com/linux/v3.16.7/source/net/sched/sch_hfsc.c= #L1427 >>=20 >> Which seems to indicate that HFSC can't find the next class to schedule. >> Not entirely sure why, nor why this only happens with CAKE as a qdisc. >> But I don't think it's actually an infinite loop that's causing it... > > > Ok, fwiw one doesn=E2=80=99t actually need a one-armed router or VLANs to= reproduce this. Just do this: > > tc qdisc add dev $IFACE root handle 1: hfsc default 1 > tc class add dev $IFACE parent 1: classid 1:1 hfsc ls rate $RATE ul rate = $RATE > tc qdisc add dev $IFACE parent 1:1 cake # add split-gso here, or else=E2= =80=A6 > > I=E2=80=99ve tried it as far as 4.9.0-8, but no farther. It=E2=80=99s not= much of a > priority for me now that I have a workaround for it... Ah, I think I know what's going on: On enqueue, HFSC will increase its own internal notion of qlen (q.qlen++ in hfsc_enqueue()), and in dequeue, it will return immediately if this qlen is 0. Now, with GSO packet splitting, a single packet on enqueue can turn in to several packets on dequeue, which means that HFSC will think the queue is empty after dequeueing the first on, and refuse to dequeue any more packets. This basically means that we can't use CAKE as a leaf qdisc with GSO splitting as it stands currently. I *think* the solution is for CAKE to notify its parents; could you try the patch below and see if it helps? -Toke diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c index b910cd5c56f7..77b0ebd673ac 100644 --- a/net/sched/sch_cake.c +++ b/net/sched/sch_cake.c @@ -1617,6 +1617,30 @@ static u32 cake_classify(struct Qdisc *sch, struct c= ake_tin_data **t, =20 static void cake_reconfigure(struct Qdisc *sch); =20 +void adjust_parent_qlen(struct Qdisc *sch, unsigned int n, + unsigned int len) +{ + u32 parentid; + if (n =3D=3D 0 && len =3D=3D 0) + return; + rcu_read_lock(); + while ((parentid =3D sch->parent)) { + if (TC_H_MAJ(parentid) =3D=3D TC_H_MAJ(TC_H_INGRESS)) + break; + + if (sch->flags & TCQ_F_NOPARENT) + break; + sch =3D qdisc_lookup(qdisc_dev(sch), TC_H_MAJ(parentid)); + if (sch =3D=3D NULL) { + WARN_ON_ONCE(parentid !=3D TC_H_ROOT); + break; + } + sch->q.qlen +=3D n; + sch->qstats.backlog +=3D len; + } + rcu_read_unlock(); +} + static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) { @@ -1667,7 +1691,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Q= disc *sch, if (skb_is_gso(skb) && q->rate_flags & CAKE_FLAG_SPLIT_GSO) { struct sk_buff *segs, *nskb; netdev_features_t features =3D netif_skb_features(skb); - unsigned int slen =3D 0; + unsigned int slen =3D 0, numsegs =3D 0; =20 segs =3D skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK); if (IS_ERR_OR_NULL(segs)) @@ -1684,6 +1708,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Q= disc *sch, =20 sch->q.qlen++; slen +=3D segs->len; + numsegs++; q->buffer_used +=3D segs->truesize; b->packets++; segs =3D nskb; @@ -1696,7 +1721,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Q= disc *sch, sch->qstats.backlog +=3D slen; q->avg_window_bytes +=3D slen; =20 - qdisc_tree_reduce_backlog(sch, 1, len); + adjust_parent_qlen(sch, numsegs - 1, slen - len); consume_skb(skb); } else { /* not splitting */