[Intel-wired-lan] [PATCH net v2] ice: Reset stats on queue change

[Intel-wired-lan] [PATCH net v2] ice: Reset stats on queue change

[Benjamin Mikailenko]

Commit 288ecf491b16 ("ice: Accumulate ring statistics over reset")
implemented functionality for interface statistics to persist over reset.
This avoided the case where a reset happens in the background (for example
a tx hang occourred triggering a reset), statistics set to zero, and the
user checks ring statistics expecting them to be populated.

In the case of changing queues (for example "ethtool -L ethX combined 5"),
this change caused statistics in the remaining queues (0-4) to persist. In
this case, statistics should reset.

Fix this by clearing statistics when the number of queues changes.

Fixes: 288ecf491b16 ("ice: Accumulate ring statistics over reset")
Signed-off-by: Benjamin Mikailenko <benjamin.mikailenko@intel.com>
---
v2: Edited commit message

 drivers/net/ethernet/intel/ice/ice_lib.c | 65 ++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 703f73e54561..ce791d90800d 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1683,6 +1683,67 @@ static int ice_vsi_alloc_ring_stats(struct ice_vsi *vsi)
 	return -ENOMEM;
 }
 
+/**
+ * ice_reset_tx_ring_stats - Reset all Tx ring stats of a given ring
+ * @tx_ring: the ring whose stats needs to be cleared
+ */
+static void ice_reset_tx_ring_stats(struct ice_tx_ring *tx_ring)
+{
+	struct ice_ring_stats *ring_stats;
+
+	ring_stats = tx_ring->ring_stats;
+	if (!ring_stats)
+		return;
+
+	memset(&ring_stats->stats, 0,
+	       sizeof(ring_stats->stats));
+	memset(&ring_stats->tx_stats, 0,
+	       sizeof(ring_stats->tx_stats));
+	ring_stats->tx_stats.prev_pkt = -1;
+}
+
+/**
+ * ice_reset_rx_ring_stats - Reset all Rx ring stats of a given ring
+ * @rx_ring: the ring whose stats needs to be cleared
+ */
+static void ice_reset_rx_ring_stats(struct ice_rx_ring *rx_ring)
+{
+	struct ice_ring_stats *ring_stats;
+
+	ring_stats = rx_ring->ring_stats;
+	if (!ring_stats)
+		return;
+
+	memset(&ring_stats->stats, 0,
+	       sizeof(ring_stats->stats));
+	memset(&ring_stats->rx_stats, 0,
+	       sizeof(ring_stats->rx_stats));
+}
+
+/**
+ * ice_vsi_reset_stats - Reset all stats of a given VSI
+ * @vsi: the VSI whose stats needs to be cleared
+ */
+static void ice_vsi_reset_stats(struct ice_vsi *vsi)
+{
+	int i;
+
+	if (vsi->type == ICE_VSI_CHNL)
+		return;
+
+	memset(&vsi->net_stats, 0, sizeof(vsi->net_stats));
+	memset(&vsi->eth_stats, 0, sizeof(vsi->eth_stats));
+	memset(&vsi->eth_stats_prev, 0, sizeof(vsi->eth_stats_prev));
+
+	ice_for_each_txq(vsi, i)
+		ice_reset_tx_ring_stats(vsi->tx_rings[i]);
+
+	ice_for_each_rxq(vsi, i)
+		ice_reset_rx_ring_stats(vsi->rx_rings[i]);
+
+	vsi->stat_offsets_loaded = false;
+}
+
 /**
  * ice_vsi_manage_rss_lut - disable/enable RSS
  * @vsi: the VSI being changed
@@ -3608,6 +3669,10 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, bool init_vsi)
 	if (ice_vsi_realloc_stat_arrays(vsi, prev_txq, prev_rxq))
 		goto err_vectors;
 
+	/* Reset stats if queues changed */
+	if (vsi->num_txq != prev_txq || vsi->num_rxq != prev_rxq)
+		ice_vsi_reset_stats(vsi);
+
 	ice_vsi_rebuild_set_coalesce(vsi, coalesce, prev_num_q_vectors);
 	kfree(coalesce);
 
-- 
2.34.3

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net v2] ice: Reset stats on queue change

[Alexander Lobakin]

From: Benjamin Mikailenko <benjamin.mikailenko@intel.com>
Date: Tue, 20 Dec 2022 13:46:18 -0500

> Commit 288ecf491b16 ("ice: Accumulate ring statistics over reset")
> implemented functionality for interface statistics to persist over reset.
> This avoided the case where a reset happens in the background (for example
> a tx hang occourred triggering a reset), statistics set to zero, and the
> user checks ring statistics expecting them to be populated.
> 
> In the case of changing queues (for example "ethtool -L ethX combined 5"),
> this change caused statistics in the remaining queues (0-4) to persist. In
> this case, statistics should reset.
> 
> Fix this by clearing statistics when the number of queues changes.

Oh, sorry for the bad wording in my reply to v1. My question was
mainly why is it an issue that the statistics for the queues that
the user didn't touch doesn't get reset? As an user, I'd expect
the stats to persist if I didn't do anything with a queue, and in
case of reducing or increasing the queue count let's say between
3 and 8 I don't expect queues 0-2 to be changed anyhow.
So I realize what this commit is about and what it does do, but
I don't see any clear reasons for now why we should do that.

> 
> Fixes: 288ecf491b16 ("ice: Accumulate ring statistics over reset")
> Signed-off-by: Benjamin Mikailenko <benjamin.mikailenko@intel.com>
> ---
> v2: Edited commit message
> 
>  drivers/net/ethernet/intel/ice/ice_lib.c | 65 ++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)

[...]

> -- 
> 2.34.3

Thanks,
Olek
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan