From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6F4CC4338F for ; Wed, 4 Aug 2021 12:36:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BED4860F22 for ; Wed, 4 Aug 2021 12:36:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238168AbhHDMhG (ORCPT ); Wed, 4 Aug 2021 08:37:06 -0400 Received: from mail.kernel.org ([198.145.29.99]:51020 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237413AbhHDMhG (ORCPT ); Wed, 4 Aug 2021 08:37:06 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id A5D9B60E8D; Wed, 4 Aug 2021 12:36:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1628080613; bh=vBpefUuV9lPRsEP836BJTZ6bfER+5hsJBPPFMvDOds8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=BSpuy+9oxQBnoYzJOoSXbZG8vAm1B+S8OtdkUPiUtvHZ5j8rXSHuo1cz5S+eIpPwH 8tY0Pz9SFrVSHvW56DyPW7UudNz6jLBUug0H1SvFHzCgQNd+xep5hYVpshE25Qcihf PXgJuTJ2WN11AC/sSgeqlpkg8AqpdcCoRb5TQF1SPUAplqFFGF7635d6/7f29nSMad Z5RQ6zAwEZ2QfnVWwMz6QYbfHeToYEfPDzXfFUVWxrZC9I7wdIAMUDFDY7ODWpunjd yqaSaR6eGF8oFmIAKnhRYMql4L+AG6oB2ghzShjJd1TaBkar9KNUcJ33vDRNXOOVBc FVa1dZ/72AXIw== Date: Wed, 4 Aug 2021 05:36:50 -0700 From: Jakub Kicinski To: Saeed Mahameed Cc: Alexander Lobakin , "David S. Miller" , Jesse Brandeburg , Lukasz Czapnik , Marcin Kubiak , Michal Kubiak , Michal Swiatkowski , Jonathan Corbet , Netanel Belgazal , Arthur Kiyanovski , Guy Tzalik , Saeed Bishara , Ioana Ciornei , Claudiu Manoil , Thomas Petazzoni , Marcin Wojtas , Russell King , Edward Cree , Martin Habets , "Michael S. Tsirkin" , Jason Wang , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , KP Singh , Shay Agroskin , Sameeh Jubran , Alexander Duyck , Danielle Ratson , Ido Schimmel , Andrew Lunn , Vladyslav Tarasiuk , Arnd Bergmann , Andrew Morton , Jian Shen , Petr Vorel , Dan Murphy , Yangbo Lu , Michal Kubecek , Zheng Yongjun , Heiner Kallweit , YueHaibing , Johannes Berg , netdev@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, bpf@vger.kernel.org Subject: Re: [PATCH net-next 03/21] ethtool, stats: introduce standard XDP statistics Message-ID: <20210804053650.22aa8a5b@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> In-Reply-To: References: <20210803163641.3743-1-alexandr.lobakin@intel.com> <20210803163641.3743-4-alexandr.lobakin@intel.com> <20210803134900.578b4c37@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 03 Aug 2021 16:57:22 -0700 Saeed Mahameed wrote: > On Tue, 2021-08-03 at 13:49 -0700, Jakub Kicinski wrote: > > On Tue,=C2=A0 3 Aug 2021 18:36:23 +0200 Alexander Lobakin wrote: =20 > > > Most of the driver-side XDP enabled drivers provide some statistics > > > on XDP programs runs and different actions taken (number of passes, > > > drops, redirects etc.). =20 > >=20 > > Could you please share the statistics to back that statement up? > > Having uAPI for XDP stats is pretty much making the recommendation=20 > > that drivers should implement such stats. The recommendation from > > Alexei and others back in the day (IIRC) was that XDP programs should > > implement stats, not the drivers, to avoid duplication. >=20 > There are stats "mainly errors*" that are not even visible or reported > to the user prog,=20 Fair point, exceptions should not be performance critical. > for that i had an idea in the past to attach an > exception_bpf_prog provided by the user, where driver/stack will report > errors to this special exception_prog. Or maybe we should turn trace_xdp_exception() into a call which unconditionally collects exception stats? I think we can reasonably expect the exception_bpf_prog to always be attached, right? > > > Regarding that it's almost pretty the same across all the drivers > > > (which is obvious), we can implement some sort of "standardized" > > > statistics using Ethtool standard stats infra to eliminate a lot > > > of code and stringsets duplication, different approaches to count > > > these stats and so on. =20 > >=20 > > I'm not 100% sold on the fact that these should be ethtool stats.=20 > > Why not rtnl_fill_statsinfo() stats? Current ethtool std stats are=20 > > all pretty Ethernet specific, and all HW stats. Mixing HW and SW > > stats > > is what we're trying to get away from. >=20 > XDP is going to always be eBPF based ! why not just report such stats > to a special BPF_MAP ? BPF stack can collect the stats from the driver > and report them to this special MAP upon user request. Do you mean replacing the ethtool-netlink / rtnetlink etc. with a new BPF_MAP? I don't think adding another category of uAPI thru=20 which netdevice stats are exposed would do much good :( Plus it=20 doesn't address the "yet another cacheline" concern. To my understanding the need for stats recognizes the fact that (in large organizations) fleet monitoring is done by different teams than XDP development. So XDP team may have all the stats they need, but the team doing fleet monitoring has no idea how to get to them. To bridge the two worlds we need a way for the infra team to ask the XDP for well-defined stats. Maybe we should take a page from the BPF iterators book and create a program type for bridging the two worlds? Called by networking core when duping stats to extract from the existing BPF maps all the relevant stats and render them into a well known struct? Users' XDP design can still use a single per-cpu map with all the stats if they so choose, but there's a way to implement more optimal designs and still expose well-defined stats. Maybe that's too complex, IDK.