On 4/9/2025 2:08 PM, Paul Menzel wrote:
Dear Martyna,


Thank you for your patch.

Am 09.04.25 um 13:36 schrieb Martyna Szapar-Mudlaw:
Introduce a new ethtool statistic to ice driver, `link_down_events`,
to track the number of times the link transitions from up to down.
This counter can help diagnose issues related to link stability,
such as port flapping or unexpected link drops.

The counter increments when a link-down event occurs and is exposed
via ethtool stats as `link_down_events.nic`.

It’d be great if you pasted an example output.

In v2 (which I just submitted) the generic ethtool statistic is used for this, instead of driver specific, so I guess no need to paste the example output now.


Reviewed-by: Michal Kubiak <michal.kub...@intel.com>
Signed-off-by: Martyna Szapar-Mudlaw <martyna.szapar- mud...@linux.intel.com>
---
  drivers/net/ethernet/intel/ice/ice.h         | 1 +
  drivers/net/ethernet/intel/ice/ice_ethtool.c | 1 +
  drivers/net/ethernet/intel/ice/ice_main.c    | 3 +++
  3 files changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ ethernet/intel/ice/ice.h
index 7200d6042590..6304104d1900 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -621,6 +621,7 @@ struct ice_pf {
      u16 globr_count;    /* Global reset count */
      u16 empr_count;        /* EMP reset count */
      u16 pfr_count;        /* PF reset count */
+    u32 link_down_events;

Why not u16?

So now using u32 instead of u16 is more justified, as the v2 uses the generic ethtool stat, where this value is also u32 :)


      u8 wol_ena : 1;        /* software state of WoL */
      u32 wakeup_reason;    /* last wakeup reason */
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/ net/ethernet/intel/ice/ice_ethtool.c
index b0805704834d..7bad0113aa88 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -137,6 +137,7 @@ static const struct ice_stats ice_gstrings_pf_stats[] = {
      ICE_PF_STAT("mac_remote_faults.nic", stats.mac_remote_faults),
      ICE_PF_STAT("fdir_sb_match.nic", stats.fd_sb_match),
      ICE_PF_STAT("fdir_sb_status.nic", stats.fd_sb_status),
+    ICE_PF_STAT("link_down_events.nic", link_down_events),
      ICE_PF_STAT("tx_hwtstamp_skipped", ptp.tx_hwtstamp_skipped),
      ICE_PF_STAT("tx_hwtstamp_timeouts", ptp.tx_hwtstamp_timeouts),
      ICE_PF_STAT("tx_hwtstamp_flushed", ptp.tx_hwtstamp_flushed),
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ ethernet/intel/ice/ice_main.c
index a03e1819e6d5..d68dd2a3f4a6 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1144,6 +1144,9 @@ ice_link_event(struct ice_pf *pf, struct ice_port_info *pi, bool link_up,
      if (link_up == old_link && link_speed == old_link_speed)
          return 0;
+    if (!link_up && old_link)
+        pf->link_down_events++;
+
      ice_ptp_link_change(pf, link_up);
      if (ice_is_dcb_active(pf)) {

The diff looks good.

Thank you for the review,
Martyna


Kind regards,

Paul



Reply via email to