From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id E2CBF3B29D for ; Tue, 10 Dec 2024 20:28:04 -0500 (EST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id D189C5C5C74; Wed, 11 Dec 2024 01:27:21 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 807F0C4CED6; Wed, 11 Dec 2024 01:28:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1733880484; bh=Vl2EMTCaUtEnOeOF48k96OqejBMhlk+jdcj3u36qXDI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=jXhj8XuQzZr+Uk45OA53prcuJRA/LJLLZ2IoAGmNGCVshJnGCbOrnlKV63rB2ZbRA S+2dBUtCQ7zxPwTfggnSVjSQt0TW+rV6qp7b7XPowXVmZDBYENjT8AqCzTeuHmGThO BivubXlWv4LKTcHy9Fl2sgtpQmVF6RA1DtqnwQUn56tSTYIDVghCMsqtnJwG2InfaT etKtl4nuGsmICEqS1jAFc06fMgMrsyKA5JyNmNHov/IztG0JrTyPditnXaz8gZQd/q 9Ci4g+ECL8lov+SKpM08ex0A1hZq5akHBjQ31hzqSuqU5n41cQVrMhLDOe/jwgT12W Wbr6c+nGIRi2Q== Date: Tue, 10 Dec 2024 17:28:02 -0800 From: Jakub Kicinski To: Toke =?UTF-8?B?SMO4aWxhbmQtSsO4cmdlbnNlbg==?= Cc: Dave Taht , Jiri Pirko , netdev@vger.kernel.org, Jamal Hadi Salim , cake@lists.bufferbloat.net, Eric Dumazet , Simon Horman , Cong Wang , Paolo Abeni , "David S. Miller" Message-ID: <20241210172802.410c76a6@kernel.org> In-Reply-To: <87a5d46i9c.fsf@toke.dk> References: <20241209-cake-drop-reason-v1-1-19205f6d1f19@redhat.com> <20241209155157.6a817bc5@kernel.org> <87a5d46i9c.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Cake] [PATCH net-next] net_sched: sch_cake: Add drop reasons 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: Wed, 11 Dec 2024 01:28:05 -0000 On Tue, 10 Dec 2024 09:42:55 +0100 Toke H=C3=B8iland-J=C3=B8rgensen wrote: > > While I initially agreed with making this generic, preserving the qdisc= from > > where the drop came lets you safely inspect the cb block (timestamp, et= c), > > format of which varies by qdisc. You also get insight as to which > > qdisc was dropping. > > > > Downside is we'll end up with SKB_DROP_REASON_XXX_OVERLIMIT for > > each of the qdiscs. Etc. =20 >=20 > Yeah, I agree that a generic "dropped by AQM" reason will be too generic > without knowing which qdisc dropped it. Why does type of the qdisc matter if the qdisc was overlimit? > I guess any calls directly to kfree_skb_reason() from the qdisc will > provide the calling function, but for qdisc_drop_reason() the drop > will be deferred to __dev_queue_xmit(), so no way of knowing where > the drop came from, AFAICT? Can you tell me why I'd need to inspect the skb->cb[] in cake if packet is overlimit? Actually, none of the fields of the cb are initialized when the packet is dropped for overlimit, AFAIU. If someone is doing serious / advanced debug they mostly care about access to the qdisc and can trivially check if its ops match the expected symbol. (Speaking from experience, I've been debugging FQ packet loss on Nov 23rd.) If someone is just doing high level drop attribution having to list all possible qdiscs under "qdisc discard" is purely pain. Can we start with OVERLIMIT and CONGESTION as generic values and we can specialize if anyone has a clear need?