Re: [PATCH net-next 07/21] ethernet, ena: convert to standard XDP stats

From: Shay Agroskin <shayagr@amazon.com>
To: Alexander Lobakin <alexandr.lobakin@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	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>,
	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>,
	<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 07/21] ethernet, ena: convert to standard XDP stats
Date: Wed, 4 Aug 2021 16:04:59 +0300	[thread overview]
Message-ID: <pj41zllf5hmkck.fsf@u570694869fb251.ant.amazon.com> (raw)
In-Reply-To: <20210803163641.3743-8-alexandr.lobakin@intel.com>

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

>
>
>
> Its 6 XDP per-channel counters align just fine with the standard
> stats.
> Drop them from the custom Ethtool statistics and expose to the
> standard stats infra instead.
>
> Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
>  drivers/net/ethernet/amazon/ena/ena_ethtool.c | 46 
>  ++++++++++++++++---
>  1 file changed, 40 insertions(+), 6 deletions(-)

Hi,
thanks for making this patch. I like the idea of splitting stats 
into a per-queue basis

>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c 
> b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> index 851a198cec82..1b6563641575 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> @@ -90,12 +90,6 @@ static const struct ena_stats 
> ena_stats_rx_strings[] = {
>         ENA_STAT_RX_ENTRY(bad_req_id),
>         ENA_STAT_RX_ENTRY(empty_rx_ring),
>         ENA_STAT_RX_ENTRY(csum_unchecked),
> -       ENA_STAT_RX_ENTRY(xdp_aborted),
> -       ENA_STAT_RX_ENTRY(xdp_drop),
> -       ENA_STAT_RX_ENTRY(xdp_pass),
> -       ENA_STAT_RX_ENTRY(xdp_tx),
> -       ENA_STAT_RX_ENTRY(xdp_invalid),
> -       ENA_STAT_RX_ENTRY(xdp_redirect),
>

The ena_stats_rx_strings array is (indirectly) accessed through 
ena_get_stats() function which is used for both fetching ethtool 
stats and
for sharing the stats with the device in case of an error (through 
ena_dump_stats_ex() function).

The latter use is broken by removing the XDP specific stats from 
ena_stats_rx_strings array.

I can submit an adaptation for the new system later (similar to 
mlx5) if you prefer

thanks,
Shay

>  };
>
>  static const struct ena_stats ena_stats_ena_com_strings[] = {
> @@ -324,6 +318,44 @@ static void ena_get_ethtool_strings(struct 
> net_device *netdev,
>         }
>  }
>
> +static int ena_get_std_stats_channels(struct net_device 
> *netdev, u32 sset)
> +{
> +       const struct ena_adapter *adapter = netdev_priv(netdev);
> +
> +       switch (sset) {
> +       case ETH_SS_STATS_XDP:
> +               return adapter->num_io_queues;
> +       default:
> +               return -EOPNOTSUPP;
> +       }
> +}
> +
> +static void ena_get_xdp_stats(struct net_device *netdev,
> +                             struct ethtool_xdp_stats 
> *xdp_stats)
> +{
> +       const struct ena_adapter *adapter = netdev_priv(netdev);
> +       const struct u64_stats_sync *syncp;
> +       const struct ena_stats_rx *stats;
> +       struct ethtool_xdp_stats *iter;
> +       u32 i;
> +
...
>  {
> @@ -916,6 +948,8 @@ static const struct ethtool_ops 
> ena_ethtool_ops = {
>         .get_tunable            = ena_get_tunable,
>         .set_tunable            = ena_set_tunable,
>         .get_ts_info            = ethtool_op_get_ts_info,
> +       .get_std_stats_channels = ena_get_std_stats_channels,
> +       .get_xdp_stats          = ena_get_xdp_stats,
>  };
>
>  void ena_set_ethtool_ops(struct net_device *netdev)