Message-ID: <pj41zllf5hmkck.fsf@u570694869fb251.ant.amazon.com>
Date:   Wed, 4 Aug 2021 16:04:59 +0300
From:   Shay Agroskin <shayagr@...zon.com>
To:     Alexander Lobakin <alexandr.lobakin@...el.com>
CC:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Lukasz Czapnik <lukasz.czapnik@...el.com>,
        Marcin Kubiak <marcin.kubiak@...el.com>,
        "Michal Kubiak" <michal.kubiak@...el.com>,
        Michal Swiatkowski <michal.swiatkowski@...el.com>,
        Jonathan Corbet <corbet@....net>,
        "Netanel Belgazal" <netanel@...zon.com>,
        Arthur Kiyanovski <akiyano@...zon.com>,
        "Guy Tzalik" <gtzalik@...zon.com>,
        Saeed Bishara <saeedb@...zon.com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        "Thomas Petazzoni" <thomas.petazzoni@...tlin.com>,
        Marcin Wojtas <mw@...ihalf.com>,
        Russell King <linux@...linux.org.uk>,
        Edward Cree <ecree.xilinx@...il.com>,
        Martin Habets <habetsm.xilinx@...il.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        "Jesper Dangaard Brouer" <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        "Andrii Nakryiko" <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        KP Singh <kpsingh@...nel.org>,
        Sameeh Jubran <sameehj@...zon.com>,
        Alexander Duyck <alexanderduyck@...com>,
        Danielle Ratson <danieller@...dia.com>,
        "Ido Schimmel" <idosch@...dia.com>, Andrew Lunn <andrew@...n.ch>,
        "Vladyslav Tarasiuk" <vladyslavt@...dia.com>,
        Arnd Bergmann <arnd@...db.de>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        Jian Shen <shenjian15@...wei.com>,
        "Petr Vorel" <petr.vorel@...il.com>, Dan Murphy <dmurphy@...com>,
        Yangbo Lu <yangbo.lu@....com>,
        Michal Kubecek <mkubecek@...e.cz>,
        Zheng Yongjun <zhengyongjun3@...wei.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        YueHaibing <yuehaibing@...wei.com>,
        Johannes Berg <johannes@...solutions.net>,
        <netdev@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <virtualization@...ts.linux-foundation.org>, <bpf@...r.kernel.org>
Subject: Re: [PATCH net-next 07/21] ethernet, ena: convert to standard XDP stats


Alexander Lobakin <alexandr.lobakin@...el.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@...el.com>
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@...el.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)

Powered by blists - more mailing lists