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

[Shay Agroskin]

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