Skip to content

Commit 257310e

Browse files
pkitszelanguy11
authored andcommitted
ice: fix stats being updated by way too large values
Simplify stats accumulation logic to fix the case where we don't take previous stat value into account, we should always respect it. Main netdev stats of our PF (Tx/Rx packets/bytes) were reported orders of magnitude too big during OpenStack reconfiguration events, possibly other reconfiguration cases too. The regression was reported to be between 6.1 and 6.2, so I was almost certain that on of the two "preserve stats over reset" commits were the culprit. While reading the code, it was found that in some cases we will increase the stats by arbitrarily large number (thanks to ignoring "-prev" part of condition, after zeroing it). Note that this fixes also the case where we were around limits of u64, but that was not the regression reported. Full disclosure: I remember suggesting this particular piece of code to Ben a few years ago, so blame on me. Fixes: 2fd5e43 ("ice: Accumulate HW and Netdev statistics over reset") Reported-by: Nebojsa Stevanovic <nebojsa.stevanovic@gcore.com> Link: https://lore.kernel.org/intel-wired-lan/VI1PR02MB439744DEDAA7B59B9A2833FE912EA@VI1PR02MB4397.eurprd02.prod.outlook.com Reported-by: Christian Rohmann <christian.rohmann@inovex.de> Link: https://lore.kernel.org/intel-wired-lan/f38a6ca4-af05-48b1-a3e6-17ef2054e525@inovex.de Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> Reviewed-by: Simon Horman <horms@kernel.org> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel) Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
1 parent ee14cc9 commit 257310e

1 file changed

Lines changed: 11 additions & 13 deletions

File tree

drivers/net/ethernet/intel/ice/ice_main.c

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6737,6 +6737,7 @@ static void ice_update_vsi_ring_stats(struct ice_vsi *vsi)
67376737
{
67386738
struct rtnl_link_stats64 *net_stats, *stats_prev;
67396739
struct rtnl_link_stats64 *vsi_stats;
6740+
struct ice_pf *pf = vsi->back;
67406741
u64 pkts, bytes;
67416742
int i;
67426743

@@ -6782,21 +6783,18 @@ static void ice_update_vsi_ring_stats(struct ice_vsi *vsi)
67826783
net_stats = &vsi->net_stats;
67836784
stats_prev = &vsi->net_stats_prev;
67846785

6785-
/* clear prev counters after reset */
6786-
if (vsi_stats->tx_packets < stats_prev->tx_packets ||
6787-
vsi_stats->rx_packets < stats_prev->rx_packets) {
6788-
stats_prev->tx_packets = 0;
6789-
stats_prev->tx_bytes = 0;
6790-
stats_prev->rx_packets = 0;
6791-
stats_prev->rx_bytes = 0;
6786+
/* Update netdev counters, but keep in mind that values could start at
6787+
* random value after PF reset. And as we increase the reported stat by
6788+
* diff of Prev-Cur, we need to be sure that Prev is valid. If it's not,
6789+
* let's skip this round.
6790+
*/
6791+
if (likely(pf->stat_prev_loaded)) {
6792+
net_stats->tx_packets += vsi_stats->tx_packets - stats_prev->tx_packets;
6793+
net_stats->tx_bytes += vsi_stats->tx_bytes - stats_prev->tx_bytes;
6794+
net_stats->rx_packets += vsi_stats->rx_packets - stats_prev->rx_packets;
6795+
net_stats->rx_bytes += vsi_stats->rx_bytes - stats_prev->rx_bytes;
67926796
}
67936797

6794-
/* update netdev counters */
6795-
net_stats->tx_packets += vsi_stats->tx_packets - stats_prev->tx_packets;
6796-
net_stats->tx_bytes += vsi_stats->tx_bytes - stats_prev->tx_bytes;
6797-
net_stats->rx_packets += vsi_stats->rx_packets - stats_prev->rx_packets;
6798-
net_stats->rx_bytes += vsi_stats->rx_bytes - stats_prev->rx_bytes;
6799-
68006798
stats_prev->tx_packets = vsi_stats->tx_packets;
68016799
stats_prev->tx_bytes = vsi_stats->tx_bytes;
68026800
stats_prev->rx_packets = vsi_stats->rx_packets;

0 commit comments

Comments
 (0)