Date:   Wed,  4 Aug 2021 17:24:55 +0200
From:   Alexander Lobakin <alexandr.lobakin@...el.com>
To:     Shay Agroskin <shayagr@...zon.com>
Cc:     Alexander Lobakin <alexandr.lobakin@...el.com>,
        "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>,
        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>,
        Alexander Duyck <alexanderduyck@...com>,
        Danielle Ratson <danieller@...dia.com>,
        "Ido Schimmel" <idosch@...dia.com>, Andrew Lunn <andrew@...n.ch>,
        Arnd Bergmann <arnd@...db.de>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        Jian Shen <shenjian15@...wei.com>,
        "Petr Vorel" <petr.vorel@...il.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

From: Shay Agroskin <shayagr@...zon.com>
Date: Wed, 4 Aug 2021 16:04:59 +0300

> 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

Feel free to either do that (I'll exclude this patch from that
series then) or you can give me some little tips or examples or
anything on how to improve this one, so ena would stay converted.
Both ways are fine for me.

> thanks,
> Shay

Thanks,
Al

> >  };
> >
> >  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