Re: [Intel-wired-lan] [PATCH iwl-next v4 4/6] ice: use u64_stats API to access pkts/bytes in dim sample

2025-12-18 Thread Rinitha, SX
> -Original Message-
> From: Intel-wired-lan  On Behalf Of Jacob 
> Keller
> Sent: 21 November 2025 01:51
> To: Loktionov, Aleksandr ; Lobakin, Aleksander 
> ; Nguyen, Anthony L 
> ; Kitszel, Przemyslaw 
> 
> Cc: Simon Horman ; [email protected]; 
> [email protected]; Keller, Jacob E ; 
> Loktionov, Aleksandr 
> Subject: [Intel-wired-lan] [PATCH iwl-next v4 4/6] ice: use u64_stats API to 
> access pkts/bytes in dim sample
>
> The __ice_update_sample and __ice_get_ethtool_stats functions directly 
> accesses the pkts and bytes counters from the ring stats. A following change 
> is going to update the fields to be u64_stats_t type, and will need to be 
> accessed appropriately. This will ensure that the accesses do not cause 
> load/store tearing.
>
> Add helper functions similar to the ones used for updating the stats values, 
> and use them. This ensures use of the syncp pointer on 32-bit architectures. 
> Once the fields are updated to u64_stats_t, it will then properly avoid tears 
> on all architectures.
>
> Reviewed-by: Aleksandr Loktionov 
> Signed-off-by: Jacob Keller 
> ---
> drivers/net/ethernet/intel/ice/ice_lib.h |  6 +
> drivers/net/ethernet/intel/ice/ice_ethtool.c | 26 +---
> drivers/net/ethernet/intel/ice/ice_lib.c | 36 
> drivers/net/ethernet/intel/ice/ice_txrx.c| 29 +++---
> 4 files changed, 75 insertions(+), 22 deletions(-)
>

Tested-by: Rinitha S  (A Contingent worker at Intel)


Re: [Intel-wired-lan] [PATCH iwl-next v4 4/6] ice: use u64_stats API to access pkts/bytes in dim sample

2025-11-25 Thread Simon Horman
On Thu, Nov 20, 2025 at 12:20:44PM -0800, Jacob Keller wrote:
> The __ice_update_sample and __ice_get_ethtool_stats functions directly
> accesses the pkts and bytes counters from the ring stats. A following
> change is going to update the fields to be u64_stats_t type, and will need
> to be accessed appropriately. This will ensure that the accesses do not
> cause load/store tearing.
> 
> Add helper functions similar to the ones used for updating the stats
> values, and use them. This ensures use of the syncp pointer on 32-bit
> architectures. Once the fields are updated to u64_stats_t, it will then
> properly avoid tears on all architectures.
> 
> Reviewed-by: Aleksandr Loktionov 
> Signed-off-by: Jacob Keller 

This seems like a nice clean up to me.  And I think it makes sense in the
context of where this patch set is going.

Reviewed-by: Simon Horman 


[Intel-wired-lan] [PATCH iwl-next v4 4/6] ice: use u64_stats API to access pkts/bytes in dim sample

2025-11-20 Thread Jacob Keller
The __ice_update_sample and __ice_get_ethtool_stats functions directly
accesses the pkts and bytes counters from the ring stats. A following
change is going to update the fields to be u64_stats_t type, and will need
to be accessed appropriately. This will ensure that the accesses do not
cause load/store tearing.

Add helper functions similar to the ones used for updating the stats
values, and use them. This ensures use of the syncp pointer on 32-bit
architectures. Once the fields are updated to u64_stats_t, it will then
properly avoid tears on all architectures.

Reviewed-by: Aleksandr Loktionov 
Signed-off-by: Jacob Keller 
---
 drivers/net/ethernet/intel/ice/ice_lib.h |  6 +
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 26 +---
 drivers/net/ethernet/intel/ice/ice_lib.c | 36 
 drivers/net/ethernet/intel/ice/ice_txrx.c| 29 +++---
 4 files changed, 75 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h 
b/drivers/net/ethernet/intel/ice/ice_lib.h
index 2cb1eb98b9da..49454d98dcfe 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_lib.h
@@ -92,6 +92,12 @@ void ice_update_tx_ring_stats(struct ice_tx_ring *ring, u64 
pkts, u64 bytes);
 
 void ice_update_rx_ring_stats(struct ice_rx_ring *ring, u64 pkts, u64 bytes);
 
+void ice_fetch_tx_ring_stats(const struct ice_tx_ring *ring,
+u64 *pkts, u64 *bytes);
+
+void ice_fetch_rx_ring_stats(const struct ice_rx_ring *ring,
+u64 *pkts, u64 *bytes);
+
 void ice_write_intrl(struct ice_q_vector *q_vector, u8 intrl);
 void ice_write_itr(struct ice_ring_container *rc, u16 itr);
 void ice_set_q_vector_intrl(struct ice_q_vector *q_vector);
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c 
b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 0bc6f31a2b06..6c93e0e91ef5 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -1942,25 +1942,35 @@ __ice_get_ethtool_stats(struct net_device *netdev,
rcu_read_lock();
 
ice_for_each_alloc_txq(vsi, j) {
+   u64 pkts, bytes;
+
tx_ring = READ_ONCE(vsi->tx_rings[j]);
-   if (tx_ring && tx_ring->ring_stats) {
-   data[i++] = tx_ring->ring_stats->stats.pkts;
-   data[i++] = tx_ring->ring_stats->stats.bytes;
-   } else {
+   if (!tx_ring || !tx_ring->ring_stats) {
data[i++] = 0;
data[i++] = 0;
+   continue;
}
+
+   ice_fetch_tx_ring_stats(tx_ring, &pkts, &bytes);
+
+   data[i++] = pkts;
+   data[i++] = bytes;
}
 
ice_for_each_alloc_rxq(vsi, j) {
+   u64 pkts, bytes;
+
rx_ring = READ_ONCE(vsi->rx_rings[j]);
-   if (rx_ring && rx_ring->ring_stats) {
-   data[i++] = rx_ring->ring_stats->stats.pkts;
-   data[i++] = rx_ring->ring_stats->stats.bytes;
-   } else {
+   if (!rx_ring || !rx_ring->ring_stats) {
data[i++] = 0;
data[i++] = 0;
+   continue;
}
+
+   ice_fetch_rx_ring_stats(rx_ring, &pkts, &bytes);
+
+   data[i++] = pkts;
+   data[i++] = bytes;
}
 
rcu_read_unlock();
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c 
b/drivers/net/ethernet/intel/ice/ice_lib.c
index 85bc18600c17..1e3946a1dd36 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -3474,6 +3474,42 @@ void ice_update_rx_ring_stats(struct ice_rx_ring 
*rx_ring, u64 pkts, u64 bytes)
u64_stats_update_end(&rx_ring->ring_stats->syncp);
 }
 
+/**
+ * ice_fetch_tx_ring_stats - Fetch Tx ring packet and byte counters
+ * @ring: ring to update
+ * @pkts: number of processed packets
+ * @bytes: number of processed bytes
+ */
+void ice_fetch_tx_ring_stats(const struct ice_tx_ring *ring,
+u64 *pkts, u64 *bytes)
+{
+   unsigned int start;
+
+   do  {
+   start = u64_stats_fetch_begin(&ring->ring_stats->syncp);
+   *pkts = ring->ring_stats->pkts;
+   *bytes = ring->ring_stats->bytes;
+   } while (u64_stats_fetch_retry(&ring->ring_stats->syncp, start));
+}
+
+/**
+ * ice_fetch_rx_ring_stats - Fetch Rx ring packet and byte counters
+ * @ring: ring to read
+ * @pkts: number of processed packets
+ * @bytes: number of processed bytes
+ */
+void ice_fetch_rx_ring_stats(const struct ice_rx_ring *ring,
+u64 *pkts, u64 *bytes)
+{
+   unsigned int start;
+
+   do  {
+   start = u64_stats_fetch_begin(&ring->ring_stats->syncp);
+   *pkts = ring->ring_stat