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

From: Shay Agroskin <redacted>
Date: Wed, 4 Aug 2021 16:04:59 +0300

Alexander Lobakin [off-list ref] writes:

quoted

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 <redacted>
Reviewed-by: Jesse Brandeburg <redacted>
---
 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

quoted

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

quoted

 };

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

...

quoted

 {

@@ -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)