[PATCH iwl-net] ice: Reset stats on queues num change

[PATCH iwl-net] ice: Reset stats on queues num change

[Przemek Kitszel]

Reset VSI stats on queues number change.

Commit 288ecf491b16 ("ice: Accumulate ring statistics over reset")
implemented functionality for interface statistics to persist over reset,
but it left stats persisting over queue count reconfiguration.

Following scenario is fixed here:
 # Observe statistics for Tx/Rx queues
ethtool -S ethX
 # change number of queues
ethtool -L ethX combined 10
 # Observe statistics for Tx/Rx queues (after reset)
ethtool -S ethX

Ben has left a note where to place the VSI stats reset,
what made this fix much easier to do.

Note that newly allocated structs (case of num_txq > prev_txq) don't
need zeroing.

Fixes: 288ecf491b16 ("ice: Accumulate ring statistics over reset")
Suggested-by: Benjamin Mikailenko <benjamin.mikailenko@intel.com>
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_lib.c | 33 ++++++++++++++++++++----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 00e3afd507a4..09942bdea25d 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -3130,13 +3130,15 @@ ice_vsi_rebuild_set_coalesce(struct ice_vsi *vsi,
 }
 
 /**
- * ice_vsi_realloc_stat_arrays - Frees unused stat structures
+ * ice_vsi_adjust_stat_arrays - Adjust VSI stat structures
  * @vsi: VSI pointer
  * @prev_txq: Number of Tx rings before ring reallocation
  * @prev_rxq: Number of Rx rings before ring reallocation
+ *
+ * Zero stat structures before reuse, free redundant ones.
  */
 static void
-ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi, int prev_txq, int prev_rxq)
+ice_vsi_adjust_stat_arrays(struct ice_vsi *vsi, int prev_txq, int prev_rxq)
 {
 	struct ice_vsi_stats *vsi_stat;
 	struct ice_pf *pf = vsi->back;
@@ -3149,7 +3151,17 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi, int prev_txq, int prev_rxq)
 
 	vsi_stat = pf->vsi_stats[vsi->idx];
 
-	if (vsi->num_txq < prev_txq) {
+	if (vsi->num_txq != prev_txq) {
+		/* first, reset structs that we will reuse */
+		int reuse_q_cnt = min_t(int, vsi->num_txq, prev_txq);
+
+		for (i = 0; i < reuse_q_cnt; i++) {
+			struct ice_ring_stats *rs = vsi_stat->tx_ring_stats[i];
+
+			if (rs)
+				memset(rs, 0, sizeof(*rs));
+		}
+		/* second, free redundant ones */
 		for (i = vsi->num_txq; i < prev_txq; i++) {
 			if (vsi_stat->tx_ring_stats[i]) {
 				kfree_rcu(vsi_stat->tx_ring_stats[i], rcu);
@@ -3158,7 +3170,16 @@ ice_vsi_realloc_stat_arrays(struct ice_vsi *vsi, int prev_txq, int prev_rxq)
 		}
 	}
 
-	if (vsi->num_rxq < prev_rxq) {
+	/* apply very same logic as for tx */
+	if (vsi->num_rxq != prev_rxq) {
+		int reuse_q_cnt = min_t(int, vsi->num_rxq, prev_rxq);
+
+		for (i = 0; i < reuse_q_cnt; i++) {
+			struct ice_ring_stats *rs = vsi_stat->rx_ring_stats[i];
+
+			if (rs)
+				memset(rs, 0, sizeof(*rs));
+		}
 		for (i = vsi->num_rxq; i < prev_rxq; i++) {
 			if (vsi_stat->rx_ring_stats[i]) {
 				kfree_rcu(vsi_stat->rx_ring_stats[i], rcu);
@@ -3222,7 +3243,9 @@ int ice_vsi_rebuild(struct ice_vsi *vsi, u32 vsi_flags)
 		return ice_schedule_reset(pf, ICE_RESET_PFR);
 	}
 
-	ice_vsi_realloc_stat_arrays(vsi, prev_txq, prev_rxq);
+	ice_vsi_adjust_stat_arrays(vsi, prev_txq, prev_rxq);
+	if (vsi->num_txq != prev_txq || vsi->num_rxq != prev_rxq)
+		vsi->stat_offsets_loaded = false;
 
 	ice_vsi_rebuild_set_coalesce(vsi, coalesce, prev_num_q_vectors);
 	kfree(coalesce);

base-commit: 9d23aac8a85f69239e585c8656c6fdb21be65695
-- 
2.40.1

Re: [PATCH iwl-net] ice: Reset stats on queues num change

[Simon Horman]

On Fri, Jul 14, 2023 at 07:47:21AM -0400, Przemek Kitszel wrote:
> Reset VSI stats on queues number change.
> 
> Commit 288ecf491b16 ("ice: Accumulate ring statistics over reset")
> implemented functionality for interface statistics to persist over reset,
> but it left stats persisting over queue count reconfiguration.
> 
> Following scenario is fixed here:
>  # Observe statistics for Tx/Rx queues
> ethtool -S ethX
>  # change number of queues
> ethtool -L ethX combined 10
>  # Observe statistics for Tx/Rx queues (after reset)
> ethtool -S ethX
> 
> Ben has left a note where to place the VSI stats reset,
> what made this fix much easier to do.
> 
> Note that newly allocated structs (case of num_txq > prev_txq) don't
> need zeroing.
> 
> Fixes: 288ecf491b16 ("ice: Accumulate ring statistics over reset")
> Suggested-by: Benjamin Mikailenko <benjamin.mikailenko@intel.com>
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [Intel-wired-lan] [PATCH iwl-net] ice: Reset stats on queues num change

[Alexander Lobakin]

From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Date: Fri, 14 Jul 2023 07:47:21 -0400

> Reset VSI stats on queues number change.

Please don't.

> 
> Commit 288ecf491b16 ("ice: Accumulate ring statistics over reset")
> implemented functionality for interface statistics to persist over reset,
> but it left stats persisting over queue count reconfiguration.

And? Why do you guys think we need to zero stats for the turned off
queues? Let the stats have the same lifetime as pci_dev has.

> 
> Following scenario is fixed here:
>  # Observe statistics for Tx/Rx queues
> ethtool -S ethX
>  # change number of queues
> ethtool -L ethX combined 10
>  # Observe statistics for Tx/Rx queues (after reset)
> ethtool -S ethX
> 
> Ben has left a note where to place the VSI stats reset,
> what made this fix much easier to do.
> 
> Note that newly allocated structs (case of num_txq > prev_txq) don't
> need zeroing.
> 
> Fixes: 288ecf491b16 ("ice: Accumulate ring statistics over reset")
> Suggested-by: Benjamin Mikailenko <benjamin.mikailenko@intel.com>
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
[...]

Thanks,
Olek