netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [Intel-wired-lan] [PATCH iwl-next] idpf: add HW timestamping statistics
  2025-08-18 11:20 [PATCH iwl-next] idpf: add HW timestamping statistics Anton Nadezhdin
@ 2025-08-18 10:32 ` Paul Menzel
  0 siblings, 0 replies; 2+ messages in thread
From: Paul Menzel @ 2025-08-18 10:32 UTC (permalink / raw)
  To: Anton Nadezhdin, Milena Olech
  Cc: intel-wired-lan, netdev, anthony.l.nguyen, richardcochran,
	Joshua Hay, Aleksandr Loktionov

Dear Anton, dear Milena,


Thank you for your patch.

Am 18.08.25 um 13:20 schrieb Anton Nadezhdin:
> From: Milena Olech <milena.olech@intel.com>
> 
> Add HW timestamping statistics support - through implementing get_ts_stats.
> Timestamp statistics include correctly timestamped packets, discarded,
> skipped and flushed during PTP release.
> 
> Most of the stats are collected per vport, only requests skipped due to
> lack of free latch index are collected per Tx queue.

Should you resend it’d be great, if you added instructions how to test 
the patch.

> Signed-off-by: Milena Olech <milena.olech@intel.com>
> Co-developed-by: Anton Nadezhdin <anton.nadezhdin@intel.com>
> Signed-off-by: Anton Nadezhdin <anton.nadezhdin@intel.com>
> Reviewed-by: Joshua Hay <joshua.a.hay@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   drivers/net/ethernet/intel/idpf/idpf.h        | 17 +++++++
>   .../net/ethernet/intel/idpf/idpf_ethtool.c    | 51 +++++++++++++++++++
>   drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 11 +++-
>   .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  4 ++
>   4 files changed, 82 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
> index f4c0eaf9bde3..5951ede8c7ea 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf.h
> @@ -256,6 +256,21 @@ enum idpf_vport_flags {
>   	IDPF_VPORT_FLAGS_NBITS,
>   };
>   
> +/**
> + * struct idpf_tstamp_stats - Tx timestamp statistics
> + * @stats_sync: See struct u64_stats_sync
> + * @packets: Number of packets successfully timestamped by the hardware
> + * @discarded: Number of Tx skbs discarded due to cached PHC
> + *	       being too old to correctly extend timestamp
> + * @flushed: Number of Tx skbs flushed due to interface closed
> + */
> +struct idpf_tstamp_stats {
> +	struct u64_stats_sync stats_sync;
> +	u64_stats_t packets;
> +	u64_stats_t discarded;
> +	u64_stats_t flushed;
> +};
> +
>   struct idpf_port_stats {
>   	struct u64_stats_sync stats_sync;
>   	u64_stats_t rx_hw_csum_err;
> @@ -322,6 +337,7 @@ struct idpf_fsteer_fltr {
>    * @tx_tstamp_caps: Capabilities negotiated for Tx timestamping
>    * @tstamp_config: The Tx tstamp config
>    * @tstamp_task: Tx timestamping task
> + * @tstamp_stats: Tx timestamping statistics
>    */
>   struct idpf_vport {
>   	u16 num_txq;
> @@ -372,6 +388,7 @@ struct idpf_vport {
>   	struct idpf_ptp_vport_tx_tstamp_caps *tx_tstamp_caps;
>   	struct kernel_hwtstamp_config tstamp_config;
>   	struct work_struct tstamp_task;
> +	struct idpf_tstamp_stats tstamp_stats;
>   };
>   
>   /**
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> index 0eb812ac19c2..d56a4157ad5f 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> @@ -1685,6 +1685,56 @@ static int idpf_get_ts_info(struct net_device *netdev,
>   	return err;
>   }
>   
> +/**
> + * idpf_get_ts_stats - Collect HW tstamping statistics
> + * @netdev: network interface device structure
> + * @ts_stats: HW timestamping stats structure
> + *
> + * Collect HW timestamping statistics including successfully timestamped
> + * packets, discarded due to illegal values, flushed during releasing PTP and
> + * skipped due to lack of the free index.
> + */
> +static void idpf_get_ts_stats(struct net_device *netdev,
> +			      struct ethtool_ts_stats *ts_stats)
> +{
> +	struct idpf_vport *vport;
> +	unsigned int start;
> +
> +	idpf_vport_ctrl_lock(netdev);
> +	vport = idpf_netdev_to_vport(netdev);
> +	do {
> +		start = u64_stats_fetch_begin(&vport->tstamp_stats.stats_sync);
> +		ts_stats->pkts = u64_stats_read(&vport->tstamp_stats.packets);
> +		ts_stats->lost = u64_stats_read(&vport->tstamp_stats.flushed);
> +		ts_stats->err = u64_stats_read(&vport->tstamp_stats.discarded);
> +	} while (u64_stats_fetch_retry(&vport->tstamp_stats.stats_sync, start));
> +
> +	for (u16 i = 0; i < vport->num_txq_grp; i++) {

Does the counting variable (also below) need to be fixed size, that 
means, why can’t `unsigned int` or `size_t` be used? [1]

> +		struct idpf_txq_group *txq_grp = &vport->txq_grps[i];
> +
> +		for (u16 j = 0; j < txq_grp->num_txq; j++) {
> +			struct idpf_tx_queue *txq = txq_grp->txqs[j];
> +			struct idpf_tx_queue_stats *stats;
> +			u64 ts;
> +
> +			if (!txq)
> +				continue;
> +
> +			stats = &txq->q_stats;
> +			do {
> +				start = u64_stats_fetch_begin(&txq->stats_sync);
> +
> +				ts = u64_stats_read(&stats->tstamp_skipped);
> +			} while (u64_stats_fetch_retry(&txq->stats_sync,
> +						       start));
> +
> +			ts_stats->lost += ts;
> +		}
> +	}
> +
> +	idpf_vport_ctrl_unlock(netdev);
> +}
> +
>   static const struct ethtool_ops idpf_ethtool_ops = {
>   	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
>   				     ETHTOOL_COALESCE_USE_ADAPTIVE,
> @@ -1711,6 +1761,7 @@ static const struct ethtool_ops idpf_ethtool_ops = {
>   	.set_ringparam		= idpf_set_ringparam,
>   	.get_link_ksettings	= idpf_get_link_ksettings,
>   	.get_ts_info		= idpf_get_ts_info,
> +	.get_ts_stats		= idpf_get_ts_stats,
>   };
>   
>   /**
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> index ee21f2ff0cad..142823af1f9e 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
> @@ -618,8 +618,13 @@ u64 idpf_ptp_extend_ts(struct idpf_vport *vport, u64 in_tstamp)
>   
>   	discard_time = ptp->cached_phc_jiffies + 2 * HZ;
>   
> -	if (time_is_before_jiffies(discard_time))
> +	if (time_is_before_jiffies(discard_time)) {
> +		u64_stats_update_begin(&vport->tstamp_stats.stats_sync);
> +		u64_stats_inc(&vport->tstamp_stats.discarded);
> +		u64_stats_update_end(&vport->tstamp_stats.stats_sync);
> +
>   		return 0;
> +	}
>   
>   	return idpf_ptp_tstamp_extend_32b_to_64b(ptp->cached_phc_time,
>   						 lower_32_bits(in_tstamp));
> @@ -853,10 +858,14 @@ static void idpf_ptp_release_vport_tstamp(struct idpf_vport *vport)
>   
>   	/* Remove list with latches in use */
>   	head = &vport->tx_tstamp_caps->latches_in_use;
> +	u64_stats_update_begin(&vport->tstamp_stats.stats_sync);
>   	list_for_each_entry_safe(ptp_tx_tstamp, tmp, head, list_member) {
> +		u64_stats_inc(&vport->tstamp_stats.flushed);
> +
>   		list_del(&ptp_tx_tstamp->list_member);
>   		kfree(ptp_tx_tstamp);
>   	}
> +	u64_stats_update_end(&vport->tstamp_stats.stats_sync);
>   
>   	spin_unlock_bh(&vport->tx_tstamp_caps->latches_lock);
>   
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> index 4f1fb0cefe51..8a2e0f8c5e36 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> @@ -521,6 +521,10 @@ idpf_ptp_get_tstamp_value(struct idpf_vport *vport,
>   	list_add(&ptp_tx_tstamp->list_member,
>   		 &tx_tstamp_caps->latches_free);
>   
> +	u64_stats_update_begin(&vport->tstamp_stats.stats_sync);
> +	u64_stats_inc(&vport->tstamp_stats.packets);
> +	u64_stats_update_end(&vport->tstamp_stats.stats_sync);
> +
>   	return 0;
>   }


Kind regards,

Paul


[1]: https://notabs.org/coding/smallIntsBigPenalty.htm

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH iwl-next] idpf: add HW timestamping statistics
@ 2025-08-18 11:20 Anton Nadezhdin
  2025-08-18 10:32 ` [Intel-wired-lan] " Paul Menzel
  0 siblings, 1 reply; 2+ messages in thread
From: Anton Nadezhdin @ 2025-08-18 11:20 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, anthony.l.nguyen, richardcochran, Milena Olech,
	Anton Nadezhdin, Joshua Hay, Aleksandr Loktionov

From: Milena Olech <milena.olech@intel.com>

Add HW timestamping statistics support - through implementing get_ts_stats.
Timestamp statistics include correctly timestamped packets, discarded,
skipped and flushed during PTP release.

Most of the stats are collected per vport, only requests skipped due to
lack of free latch index are collected per Tx queue.

Signed-off-by: Milena Olech <milena.olech@intel.com>
Co-developed-by: Anton Nadezhdin <anton.nadezhdin@intel.com>
Signed-off-by: Anton Nadezhdin <anton.nadezhdin@intel.com>
Reviewed-by: Joshua Hay <joshua.a.hay@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf.h        | 17 +++++++
 .../net/ethernet/intel/idpf/idpf_ethtool.c    | 51 +++++++++++++++++++
 drivers/net/ethernet/intel/idpf/idpf_ptp.c    | 11 +++-
 .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  4 ++
 4 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index f4c0eaf9bde3..5951ede8c7ea 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -256,6 +256,21 @@ enum idpf_vport_flags {
 	IDPF_VPORT_FLAGS_NBITS,
 };
 
+/**
+ * struct idpf_tstamp_stats - Tx timestamp statistics
+ * @stats_sync: See struct u64_stats_sync
+ * @packets: Number of packets successfully timestamped by the hardware
+ * @discarded: Number of Tx skbs discarded due to cached PHC
+ *	       being too old to correctly extend timestamp
+ * @flushed: Number of Tx skbs flushed due to interface closed
+ */
+struct idpf_tstamp_stats {
+	struct u64_stats_sync stats_sync;
+	u64_stats_t packets;
+	u64_stats_t discarded;
+	u64_stats_t flushed;
+};
+
 struct idpf_port_stats {
 	struct u64_stats_sync stats_sync;
 	u64_stats_t rx_hw_csum_err;
@@ -322,6 +337,7 @@ struct idpf_fsteer_fltr {
  * @tx_tstamp_caps: Capabilities negotiated for Tx timestamping
  * @tstamp_config: The Tx tstamp config
  * @tstamp_task: Tx timestamping task
+ * @tstamp_stats: Tx timestamping statistics
  */
 struct idpf_vport {
 	u16 num_txq;
@@ -372,6 +388,7 @@ struct idpf_vport {
 	struct idpf_ptp_vport_tx_tstamp_caps *tx_tstamp_caps;
 	struct kernel_hwtstamp_config tstamp_config;
 	struct work_struct tstamp_task;
+	struct idpf_tstamp_stats tstamp_stats;
 };
 
 /**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
index 0eb812ac19c2..d56a4157ad5f 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
@@ -1685,6 +1685,56 @@ static int idpf_get_ts_info(struct net_device *netdev,
 	return err;
 }
 
+/**
+ * idpf_get_ts_stats - Collect HW tstamping statistics
+ * @netdev: network interface device structure
+ * @ts_stats: HW timestamping stats structure
+ *
+ * Collect HW timestamping statistics including successfully timestamped
+ * packets, discarded due to illegal values, flushed during releasing PTP and
+ * skipped due to lack of the free index.
+ */
+static void idpf_get_ts_stats(struct net_device *netdev,
+			      struct ethtool_ts_stats *ts_stats)
+{
+	struct idpf_vport *vport;
+	unsigned int start;
+
+	idpf_vport_ctrl_lock(netdev);
+	vport = idpf_netdev_to_vport(netdev);
+	do {
+		start = u64_stats_fetch_begin(&vport->tstamp_stats.stats_sync);
+		ts_stats->pkts = u64_stats_read(&vport->tstamp_stats.packets);
+		ts_stats->lost = u64_stats_read(&vport->tstamp_stats.flushed);
+		ts_stats->err = u64_stats_read(&vport->tstamp_stats.discarded);
+	} while (u64_stats_fetch_retry(&vport->tstamp_stats.stats_sync, start));
+
+	for (u16 i = 0; i < vport->num_txq_grp; i++) {
+		struct idpf_txq_group *txq_grp = &vport->txq_grps[i];
+
+		for (u16 j = 0; j < txq_grp->num_txq; j++) {
+			struct idpf_tx_queue *txq = txq_grp->txqs[j];
+			struct idpf_tx_queue_stats *stats;
+			u64 ts;
+
+			if (!txq)
+				continue;
+
+			stats = &txq->q_stats;
+			do {
+				start = u64_stats_fetch_begin(&txq->stats_sync);
+
+				ts = u64_stats_read(&stats->tstamp_skipped);
+			} while (u64_stats_fetch_retry(&txq->stats_sync,
+						       start));
+
+			ts_stats->lost += ts;
+		}
+	}
+
+	idpf_vport_ctrl_unlock(netdev);
+}
+
 static const struct ethtool_ops idpf_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_USE_ADAPTIVE,
@@ -1711,6 +1761,7 @@ static const struct ethtool_ops idpf_ethtool_ops = {
 	.set_ringparam		= idpf_set_ringparam,
 	.get_link_ksettings	= idpf_get_link_ksettings,
 	.get_ts_info		= idpf_get_ts_info,
+	.get_ts_stats		= idpf_get_ts_stats,
 };
 
 /**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
index ee21f2ff0cad..142823af1f9e 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ptp.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ptp.c
@@ -618,8 +618,13 @@ u64 idpf_ptp_extend_ts(struct idpf_vport *vport, u64 in_tstamp)
 
 	discard_time = ptp->cached_phc_jiffies + 2 * HZ;
 
-	if (time_is_before_jiffies(discard_time))
+	if (time_is_before_jiffies(discard_time)) {
+		u64_stats_update_begin(&vport->tstamp_stats.stats_sync);
+		u64_stats_inc(&vport->tstamp_stats.discarded);
+		u64_stats_update_end(&vport->tstamp_stats.stats_sync);
+
 		return 0;
+	}
 
 	return idpf_ptp_tstamp_extend_32b_to_64b(ptp->cached_phc_time,
 						 lower_32_bits(in_tstamp));
@@ -853,10 +858,14 @@ static void idpf_ptp_release_vport_tstamp(struct idpf_vport *vport)
 
 	/* Remove list with latches in use */
 	head = &vport->tx_tstamp_caps->latches_in_use;
+	u64_stats_update_begin(&vport->tstamp_stats.stats_sync);
 	list_for_each_entry_safe(ptp_tx_tstamp, tmp, head, list_member) {
+		u64_stats_inc(&vport->tstamp_stats.flushed);
+
 		list_del(&ptp_tx_tstamp->list_member);
 		kfree(ptp_tx_tstamp);
 	}
+	u64_stats_update_end(&vport->tstamp_stats.stats_sync);
 
 	spin_unlock_bh(&vport->tx_tstamp_caps->latches_lock);
 
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
index 4f1fb0cefe51..8a2e0f8c5e36 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
@@ -521,6 +521,10 @@ idpf_ptp_get_tstamp_value(struct idpf_vport *vport,
 	list_add(&ptp_tx_tstamp->list_member,
 		 &tx_tstamp_caps->latches_free);
 
+	u64_stats_update_begin(&vport->tstamp_stats.stats_sync);
+	u64_stats_inc(&vport->tstamp_stats.packets);
+	u64_stats_update_end(&vport->tstamp_stats.stats_sync);
+
 	return 0;
 }
 

base-commit: 94f1d1440652b6145cbaa026f376ae4e7fb95843
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-08-18 10:32 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-18 11:20 [PATCH iwl-next] idpf: add HW timestamping statistics Anton Nadezhdin
2025-08-18 10:32 ` [Intel-wired-lan] " Paul Menzel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).