Re: [PATCH net-next 03/21] ethtool, stats: introduce standard XDP statistics - Toke Høiland-Jørgensen

From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Alexander Lobakin <alexandr.lobakin@intel.com>
Cc: Alexander Lobakin <alexandr.lobakin@intel.com>,
	Saeed Mahameed <saeed@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Lukasz Czapnik <lukasz.czapnik@intel.com>,
	Marcin Kubiak <marcin.kubiak@intel.com>,
	Michal Kubiak <michal.kubiak@intel.com>,
	Michal Swiatkowski <michal.swiatkowski@intel.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Netanel Belgazal <netanel@amazon.com>,
	Arthur Kiyanovski <akiyano@amazon.com>,
	Guy Tzalik <gtzalik@amazon.com>,
	Saeed Bishara <saeedb@amazon.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Marcin Wojtas <mw@semihalf.com>,
	Russell King <linux@armlinux.org.uk>,
	Edward Cree <ecree.xilinx@gmail.com>,
	Martin Habets <habetsm.xilinx@gmail.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, KP Singh <kpsingh@kernel.org>,
	Shay Agroskin <shayagr@amazon.com>,
	Sameeh Jubran <sameehj@amazon.com>,
	Alexander Duyck <alexanderduyck@fb.com>,
	Danielle Ratson <danieller@nvidia.com>,
	Ido Schimmel <idosch@nvidia.com>, Andrew Lunn <andrew@lunn.ch>,
	Vladyslav Tarasiuk <vladyslavt@nvidia.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jian Shen <shenjian15@huawei.com>,
	Petr Vorel <petr.vorel@gmail.com>, Dan Murphy <dmurphy@ti.com>,
	Yangbo Lu <yangbo.lu@nxp.com>, Michal Kubecek <mkubecek@suse.cz>,
	Zheng Yongjun <zhengyongjun3@huawei.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	YueHaibing <yuehaibing@huawei.com>,
	Johannes Berg <johannes@sipsolutions.net>,
	Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
	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
Date: Mon, 08 Nov 2021 19:09:08 +0100	[thread overview]
Message-ID: <87cznar03f.fsf@toke.dk> (raw)
In-Reply-To: <20211108132113.5152-1-alexandr.lobakin@intel.com>

Alexander Lobakin <alexandr.lobakin@intel.com> writes:

> From: Toke Høiland-Jørgensen <toke@redhat.com>
> Date: Mon, 08 Nov 2021 12:37:54 +0100
>
>> Alexander Lobakin <alexandr.lobakin@intel.com> writes:
>> 
>> > From: Alexander Lobakin <alexandr.lobakin@intel.com>
>> > Date: Tue, 26 Oct 2021 11:23:23 +0200
>> >
>> >> From: Saeed Mahameed <saeed@kernel.org>
>> >> Date: Tue, 03 Aug 2021 16:57:22 -0700
>> >> 
>> >> [ snip ]
>> >> 
>> >> > 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.
>> >> 
>> >> I really dig this idea now. How do you see it?
>> >> <ifindex:channel:stat_id> as a key and its value as a value or ...?
>> >
>> > Ideas, suggestions, anyone?
>> 
>> I don't like the idea of putting statistics in a map instead of the
>> regular statistics counters. Sure, for bespoke things people want to put
>> into their XDP programs, use a map, but for regular packet/byte
>> counters, update the regular counters so XDP isn't "invisible".
>
> I wanted to provide an `ip link` command for getting these stats
> from maps and printing them in a usual format as well, but seems
> like that's an unneeded overcomplication of things since using
> maps for "regular"/"generic" XDP stats really has no reason except
> for "XDP means eBPF means maps".

Yeah, don't really see why it would have to: to me, one of the benefits
of XDP is being integrated closely with the kernel so we can have a
"fast path" *without* reinventing everything...

>> As Jesper pointed out, batching the updates so the global counters are
>> only updated once per NAPI cycle is the way to avoid a huge performance
>> overhead of this...
>
> That's how I do things currently, seems to work just fine.

Awesome!

-Toke