[PATCH net-next v1 0/6] ethtool HW timestamping statistics

[PATCH net-next v1 0/6] ethtool HW timestamping statistics

[Rahul Rameshbabu]

The goal of this patch series is to introduce a common set of ethtool statistics
for hardware timestamping that a driver implementer can hook into. The
statistics counters added are based on what I believe are common
patterns/behaviors found across various hardware timestamping implementations
seen in the kernel tree today. The mlx5 family of devices is used as the PoC for
this patch series. Other vendors are more than welcome to chim in on this
series.

Changes since RFC v1:
        - Dropped the late statistics counter since that was not general enough
        - Dropped the layer selection attribute for timestamping statistics
        - Take Jakub's suggestion and converted to ETHTOOL_FLAG_STATS
        - Provided a working interface to query these new statistics from
          tools/net/ynl/ethtool.py

Changes since RFC v2:
        - Applied suggestion by Jakub for implementing an enumeration for
          ethtool header flags

Link: https://lore.kernel.org/netdev/20240309084440.299358-1-rrameshbabu@nvidia.com/
Link: https://lore.kernel.org/netdev/20240223192658.45893-1-rrameshbabu@nvidia.com/
Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
---
Rahul Rameshbabu (6):
  ethtool: add interface to read Tx hardware timestamping statistics
  net/mlx5e: Introduce lost_cqe statistic counter for PTP Tx port
    timestamping CQ
  net/mlx5e: Introduce timestamps statistic counter for Tx DMA layer
  net/mlx5e: Implement ethtool hardware timestamping statistics
  netlink: specs: ethtool: add header-flags enumeration
  tools: ynl: ethtool.py: Output timestamping statistics from tsinfo-get
    operation

 Documentation/netlink/specs/ethtool.yaml      | 22 ++++++++
 .../ethernet/mellanox/mlx5/counters.rst       | 11 ++++
 Documentation/networking/ethtool-netlink.rst  |  9 ++++
 .../net/ethernet/mellanox/mlx5/core/en/ptp.c  |  1 +
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  |  9 ++++
 .../ethernet/mellanox/mlx5/core/en_stats.c    | 48 +++++++++++++++++
 .../ethernet/mellanox/mlx5/core/en_stats.h    |  4 ++
 .../net/ethernet/mellanox/mlx5/core/en_tx.c   |  6 ++-
 include/linux/ethtool.h                       | 28 +++++++++-
 include/uapi/linux/ethtool_netlink.h          | 25 ++++++---
 net/ethtool/tsinfo.c                          | 52 ++++++++++++++++++-
 tools/net/ynl/ethtool.py                      | 11 +++-
 12 files changed, 215 insertions(+), 11 deletions(-)

-- 
2.42.0

[PATCH net-next v1 1/6] ethtool: add interface to read Tx hardware timestamping statistics

[Rahul Rameshbabu]

Multiple network devices that support hardware timestamping appear to have
common behavior with regards to timestamp handling. Implement common Tx
hardware timestamping statistics in a tx_stats struct_group. Common Rx
hardware timestamping statistics can subsequently be implemented in a
rx_stats struct_group for ethtool_ts_stats.

Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 Documentation/netlink/specs/ethtool.yaml     | 17 +++++++
 Documentation/networking/ethtool-netlink.rst |  9 ++++
 include/linux/ethtool.h                      | 28 ++++++++++-
 include/uapi/linux/ethtool_netlink.h         | 14 ++++++
 net/ethtool/tsinfo.c                         | 52 +++++++++++++++++++-
 5 files changed, 118 insertions(+), 2 deletions(-)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 197208f419dc..f5aa1e7f3383 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -559,6 +559,18 @@ attribute-sets:
       -
         name: tx-lpi-timer
         type: u32
+  -
+    name: ts-stat
+    attributes:
+      -
+        name: tx-pkts
+        type: uint
+      -
+        name: tx-lost
+        type: uint
+      -
+        name: tx-err
+        type: uint
   -
     name: tsinfo
     attributes:
@@ -581,6 +593,10 @@ attribute-sets:
       -
         name: phc-index
         type: u32
+      -
+        name: stats
+        type: nest
+        nested-attributes: ts-stat
   -
     name: cable-result
     attributes:
@@ -1388,6 +1404,7 @@ operations:
             - tx-types
             - rx-filters
             - phc-index
+            - stats
       dump: *tsinfo-get-op
     -
       name: cable-test-act
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index d583d9abf2f8..08d330b0f50f 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1237,12 +1237,21 @@ Kernel response contents:
   ``ETHTOOL_A_TSINFO_TX_TYPES``          bitset  supported Tx types
   ``ETHTOOL_A_TSINFO_RX_FILTERS``        bitset  supported Rx filters
   ``ETHTOOL_A_TSINFO_PHC_INDEX``         u32     PTP hw clock index
+  ``ETHTOOL_A_TSINFO_STATS``             nested  HW timestamping statistics
   =====================================  ======  ==========================
 
 ``ETHTOOL_A_TSINFO_PHC_INDEX`` is absent if there is no associated PHC (there
 is no special value for this case). The bitset attributes are omitted if they
 would be empty (no bit set).
 
+Additional hardware timestamping statistics response contents:
+
+  =====================================  ======  ===================================
+  ``ETHTOOL_A_TS_STAT_TX_PKTS``          u64     Packets with Tx HW timestamps
+  ``ETHTOOL_A_TS_STAT_TX_LOST``          u64     Tx HW timestamp not arrived count
+  ``ETHTOOL_A_TS_STAT_TX_ERR``           u64     HW error request Tx timestamp count
+  =====================================  ======  ===================================
+
 CABLE_TEST
 ==========
 
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 9901e563f706..4b5ca7628700 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -480,6 +480,27 @@ struct ethtool_rmon_stats {
 	);
 };
 
+/**
+ * struct ethtool_ts_stats - HW timestamping statistics
+ * @tx_stats: struct group for TX HW timestamping
+ *	@pkts: Number of packets successfully timestamped by the hardware.
+ *	@lost: Number of hardware timestamping requests where the timestamping
+ *		information from the hardware never arrived for submission with
+ *		the skb.
+ *	@err: Number of arbitrary timestamp generation error events that the
+ *		hardware encountered, exclusive of @lost statistics. Cases such
+ *		as resource exhaustion, unavailability, firmware errors, and
+ *		detected illogical timestamp values not submitted with the skb
+ *		are inclusive to this counter.
+ */
+struct ethtool_ts_stats {
+	struct_group(tx_stats,
+		u64 pkts;
+		u64 lost;
+		u64 err;
+	);
+};
+
 #define ETH_MODULE_EEPROM_PAGE_LEN	128
 #define ETH_MODULE_MAX_I2C_ADDRESS	0x7f
 
@@ -755,7 +776,10 @@ struct ethtool_rxfh_param {
  * @get_ts_info: Get the time stamping and PTP hardware clock capabilities.
  *	It may be called with RCU, or rtnl or reference on the device.
  *	Drivers supporting transmit time stamps in software should set this to
- *	ethtool_op_get_ts_info().
+ *	ethtool_op_get_ts_info(). Drivers must not zero statistics which they
+ *	don't report. The stats	structure is initialized to ETHTOOL_STAT_NOT_SET
+ *	indicating driver does not report statistics.
+ * @get_ts_stats: Query the device hardware timestamping statistics.
  * @get_module_info: Get the size and type of the eeprom contained within
  *	a plug-in module.
  * @get_module_eeprom: Get the eeprom information from the plug-in module
@@ -898,6 +922,8 @@ struct ethtool_ops {
 				 struct ethtool_dump *, void *);
 	int	(*set_dump)(struct net_device *, struct ethtool_dump *);
 	int	(*get_ts_info)(struct net_device *, struct ethtool_ts_info *);
+	void	(*get_ts_stats)(struct net_device *dev,
+				struct ethtool_ts_stats *ts_stats);
 	int     (*get_module_info)(struct net_device *,
 				   struct ethtool_modinfo *);
 	int     (*get_module_eeprom)(struct net_device *,
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 3f89074aa06c..8dfd714c2308 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -478,12 +478,26 @@ enum {
 	ETHTOOL_A_TSINFO_TX_TYPES,			/* bitset */
 	ETHTOOL_A_TSINFO_RX_FILTERS,			/* bitset */
 	ETHTOOL_A_TSINFO_PHC_INDEX,			/* u32 */
+	ETHTOOL_A_TSINFO_STATS,				/* nest - _A_TSINFO_STAT */
 
 	/* add new constants above here */
 	__ETHTOOL_A_TSINFO_CNT,
 	ETHTOOL_A_TSINFO_MAX = (__ETHTOOL_A_TSINFO_CNT - 1)
 };
 
+enum {
+	ETHTOOL_A_TS_STAT_UNSPEC,
+
+	ETHTOOL_A_TS_STAT_TX_PKTS,			/* u64 */
+	ETHTOOL_A_TS_STAT_TX_LOST,			/* u64 */
+	ETHTOOL_A_TS_STAT_TX_ERR,			/* u64 */
+
+	/* add new constants above here */
+	__ETHTOOL_A_TS_STAT_CNT,
+	ETHTOOL_A_TS_STAT_MAX = (__ETHTOOL_A_TS_STAT_CNT - 1)
+
+};
+
 /* PHC VCLOCKS */
 
 enum {
diff --git a/net/ethtool/tsinfo.c b/net/ethtool/tsinfo.c
index 9daed0aab162..be2755c8d8fd 100644
--- a/net/ethtool/tsinfo.c
+++ b/net/ethtool/tsinfo.c
@@ -13,14 +13,18 @@ struct tsinfo_req_info {
 struct tsinfo_reply_data {
 	struct ethnl_reply_data		base;
 	struct ethtool_ts_info		ts_info;
+	struct ethtool_ts_stats		stats;
 };
 
 #define TSINFO_REPDATA(__reply_base) \
 	container_of(__reply_base, struct tsinfo_reply_data, base)
 
+#define ETHTOOL_TS_STAT_CNT \
+	(__ETHTOOL_A_TS_STAT_CNT - (ETHTOOL_A_TS_STAT_UNSPEC + 1))
+
 const struct nla_policy ethnl_tsinfo_get_policy[] = {
 	[ETHTOOL_A_TSINFO_HEADER]		=
-		NLA_POLICY_NESTED(ethnl_header_policy),
+		NLA_POLICY_NESTED(ethnl_header_policy_stats),
 };
 
 static int tsinfo_prepare_data(const struct ethnl_req_info *req_base,
@@ -34,6 +38,12 @@ static int tsinfo_prepare_data(const struct ethnl_req_info *req_base,
 	ret = ethnl_ops_begin(dev);
 	if (ret < 0)
 		return ret;
+	if (req_base->flags & ETHTOOL_FLAG_STATS &&
+	    dev->ethtool_ops->get_ts_stats) {
+		ethtool_stats_init((u64 *)&data->stats,
+				   sizeof(data->stats) / sizeof(u64));
+		dev->ethtool_ops->get_ts_stats(dev, &data->stats);
+	}
 	ret = __ethtool_get_ts_info(dev, &data->ts_info);
 	ethnl_ops_complete(dev);
 
@@ -79,10 +89,47 @@ static int tsinfo_reply_size(const struct ethnl_req_info *req_base,
 	}
 	if (ts_info->phc_index >= 0)
 		len += nla_total_size(sizeof(u32));	/* _TSINFO_PHC_INDEX */
+	if (req_base->flags & ETHTOOL_FLAG_STATS)
+		len += nla_total_size(0) + /* _TSINFO_STATS */
+		       nla_total_size_64bit(sizeof(u64)) * ETHTOOL_TS_STAT_CNT;
 
 	return len;
 }
 
+static int tsinfo_put_stat(struct sk_buff *skb, u64 val, u16 attrtype)
+{
+	if (val == ETHTOOL_STAT_NOT_SET)
+		return 0;
+	if (nla_put_uint(skb, attrtype, val))
+		return -EMSGSIZE;
+	return 0;
+}
+
+static int tsinfo_put_stats(struct sk_buff *skb,
+			    const struct ethtool_ts_stats *stats)
+{
+	struct nlattr *nest;
+
+	nest = nla_nest_start(skb, ETHTOOL_A_TSINFO_STATS);
+	if (!nest)
+		return -EMSGSIZE;
+
+	if (tsinfo_put_stat(skb, stats->tx_stats.pkts,
+			    ETHTOOL_A_TS_STAT_TX_PKTS) ||
+	    tsinfo_put_stat(skb, stats->tx_stats.lost,
+			    ETHTOOL_A_TS_STAT_TX_LOST) ||
+	    tsinfo_put_stat(skb, stats->tx_stats.err,
+			    ETHTOOL_A_TS_STAT_TX_ERR))
+		goto err_cancel;
+
+	nla_nest_end(skb, nest);
+	return 0;
+
+err_cancel:
+	nla_nest_cancel(skb, nest);
+	return -EMSGSIZE;
+}
+
 static int tsinfo_fill_reply(struct sk_buff *skb,
 			     const struct ethnl_req_info *req_base,
 			     const struct ethnl_reply_data *reply_base)
@@ -119,6 +166,9 @@ static int tsinfo_fill_reply(struct sk_buff *skb,
 	if (ts_info->phc_index >= 0 &&
 	    nla_put_u32(skb, ETHTOOL_A_TSINFO_PHC_INDEX, ts_info->phc_index))
 		return -EMSGSIZE;
+	if (req_base->flags & ETHTOOL_FLAG_STATS &&
+	    tsinfo_put_stats(skb, &data->stats))
+		return -EMSGSIZE;
 
 	return 0;
 }
-- 
2.42.0

[PATCH net-next v1 2/6] net/mlx5e: Introduce lost_cqe statistic counter for PTP Tx port timestamping CQ

[Rahul Rameshbabu]

Track the number of times a CQE was expected to not be delivered on PTP Tx
port timestamping CQ. A CQE is expected to not be delivered if a certain
amount of time passes since the corresponding CQE containing the DMA
timestamp information has arrived. Increment the late_cqe counter when such
a CQE does manage to be delivered to the CQ.

Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 .../device_drivers/ethernet/mellanox/mlx5/counters.rst      | 6 ++++++
 drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c            | 1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.c          | 1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.h          | 1 +
 4 files changed, 9 insertions(+)

diff --git a/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst b/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst
index f69ee1ebee01..5464cd9e2694 100644
--- a/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst
+++ b/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst
@@ -702,6 +702,12 @@ the software port.
        the device typically ensures not posting the CQE.
      - Error
 
+   * - `ptp_cq[i]_lost_cqe`
+     - Number of times a CQE is expected to not be delivered on the PTP
+       timestamping CQE by the device due to a time delta elapsing. If such a
+       CQE is somehow delivered, `ptp_cq[i]_late_cqe` is incremented.
+     - Error
+
 .. [#ring_global] The corresponding ring and global counters do not share the
                   same name (i.e. do not follow the common naming scheme).
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
index d0af7271da34..afd654583b6b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
@@ -169,6 +169,7 @@ static void mlx5e_ptpsq_mark_ts_cqes_undelivered(struct mlx5e_ptpsq *ptpsq,
 		WARN_ON_ONCE(!pos->inuse);
 		pos->inuse = false;
 		list_del(&pos->entry);
+		ptpsq->cq_stats->lost_cqe++;
 	}
 	spin_unlock_bh(&cqe_list->tracker_list_lock);
 }
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index f3d0898bdbc6..64a435f914ff 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -2175,6 +2175,7 @@ static const struct counter_desc ptp_cq_stats_desc[] = {
 	{ MLX5E_DECLARE_PTP_CQ_STAT(struct mlx5e_ptp_cq_stats, abort) },
 	{ MLX5E_DECLARE_PTP_CQ_STAT(struct mlx5e_ptp_cq_stats, abort_abs_diff_ns) },
 	{ MLX5E_DECLARE_PTP_CQ_STAT(struct mlx5e_ptp_cq_stats, late_cqe) },
+	{ MLX5E_DECLARE_PTP_CQ_STAT(struct mlx5e_ptp_cq_stats, lost_cqe) },
 };
 
 static const struct counter_desc ptp_rq_stats_desc[] = {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
index 12b3607afecd..03f6265d3ed5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
@@ -461,6 +461,7 @@ struct mlx5e_ptp_cq_stats {
 	u64 abort;
 	u64 abort_abs_diff_ns;
 	u64 late_cqe;
+	u64 lost_cqe;
 };
 
 struct mlx5e_rep_stats {
-- 
2.42.0

[PATCH net-next v1 3/6] net/mlx5e: Introduce timestamps statistic counter for Tx DMA layer

[Rahul Rameshbabu]

Count number of transmitted packets that were hardware timestamped at the
device DMA layer.

Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 .../device_drivers/ethernet/mellanox/mlx5/counters.rst      | 5 +++++
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.c          | 2 ++
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.h          | 1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c             | 6 ++++--
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst b/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst
index 5464cd9e2694..fed821ef9b09 100644
--- a/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst
+++ b/Documentation/networking/device_drivers/ethernet/mellanox/mlx5/counters.rst
@@ -300,6 +300,11 @@ the software port.
        in the beginning of the queue. This is a normal condition.
      - Informative
 
+   * - `tx[i]_timestamps`
+     - Transmitted packets that were hardware timestamped at the device's DMA
+       layer.
+     - Informative
+
    * - `tx[i]_added_vlan_packets`
      - The number of packets sent where vlan tag insertion was offloaded to the
        hardware.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index 64a435f914ff..a06c6c80e36f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -2063,6 +2063,7 @@ static const struct counter_desc sq_stats_desc[] = {
 	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, csum_partial_inner) },
 	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, added_vlan_packets) },
 	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, nop) },
+	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, timestamps) },
 	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, mpwqe_blks) },
 	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, mpwqe_pkts) },
 #ifdef CONFIG_MLX5_EN_TLS
@@ -2215,6 +2216,7 @@ static const struct counter_desc qos_sq_stats_desc[] = {
 	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, csum_partial_inner) },
 	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, added_vlan_packets) },
 	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, nop) },
+	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, timestamps) },
 	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, mpwqe_blks) },
 	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, mpwqe_pkts) },
 #ifdef CONFIG_MLX5_EN_TLS
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
index 03f6265d3ed5..3c634c5fd420 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
@@ -429,6 +429,7 @@ struct mlx5e_sq_stats {
 	u64 stopped;
 	u64 dropped;
 	u64 recover;
+	u64 timestamps;
 	/* dirtied @completion */
 	u64 cqes ____cacheline_aligned_in_smp;
 	u64 wake;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index 2fa076b23fbe..09a592909101 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -750,11 +750,13 @@ static void mlx5e_consume_skb(struct mlx5e_txqsq *sq, struct sk_buff *skb,
 		u64 ts = get_cqe_ts(cqe);
 
 		hwts.hwtstamp = mlx5e_cqe_ts_to_ns(sq->ptp_cyc2time, sq->clock, ts);
-		if (sq->ptpsq)
+		if (sq->ptpsq) {
 			mlx5e_skb_cb_hwtstamp_handler(skb, MLX5E_SKB_CB_CQE_HWTSTAMP,
 						      hwts.hwtstamp, sq->ptpsq->cq_stats);
-		else
+		} else {
 			skb_tstamp_tx(skb, &hwts);
+			sq->stats->timestamps++;
+		}
 	}
 
 	napi_consume_skb(skb, napi_budget);
-- 
2.42.0

[PATCH net-next v1 4/6] net/mlx5e: Implement ethtool hardware timestamping statistics

[Rahul Rameshbabu]

Feed driver statistics counters related to hardware timestamping to
standardized ethtool hardware timestamping statistics group.

Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  |  9 ++++
 .../ethernet/mellanox/mlx5/core/en_stats.c    | 45 +++++++++++++++++++
 .../ethernet/mellanox/mlx5/core/en_stats.h    |  2 +
 3 files changed, 56 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index cc51ce16df14..d3b77054c30a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -2381,6 +2381,14 @@ static void mlx5e_get_rmon_stats(struct net_device *netdev,
 	mlx5e_stats_rmon_get(priv, rmon_stats, ranges);
 }
 
+static void mlx5e_get_ts_stats(struct net_device *netdev,
+			       struct ethtool_ts_stats *ts_stats)
+{
+	struct mlx5e_priv *priv = netdev_priv(netdev);
+
+	mlx5e_stats_ts_get(priv, ts_stats);
+}
+
 const struct ethtool_ops mlx5e_ethtool_ops = {
 	.cap_rss_ctx_supported	= true,
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
@@ -2430,5 +2438,6 @@ const struct ethtool_ops mlx5e_ethtool_ops = {
 	.get_eth_mac_stats = mlx5e_get_eth_mac_stats,
 	.get_eth_ctrl_stats = mlx5e_get_eth_ctrl_stats,
 	.get_rmon_stats    = mlx5e_get_rmon_stats,
+	.get_ts_stats      = mlx5e_get_ts_stats,
 	.get_link_ext_stats = mlx5e_get_link_ext_stats
 };
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index a06c6c80e36f..d8cf1fe99fb0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -1172,6 +1172,51 @@ void mlx5e_stats_rmon_get(struct mlx5e_priv *priv,
 	*ranges = mlx5e_rmon_ranges;
 }
 
+void mlx5e_stats_ts_get(struct mlx5e_priv *priv,
+			struct ethtool_ts_stats *ts_stats)
+{
+	int i, j;
+
+	mutex_lock(&priv->state_lock);
+
+	if (priv->tx_ptp_opened) {
+		struct mlx5e_ptp *ptp = priv->channels.ptp;
+
+		ts_stats->pkts = 0;
+		ts_stats->err = 0;
+		ts_stats->lost = 0;
+
+		/* Aggregate stats across all TCs */
+		for (i = 0; i < ptp->num_tc; i++) {
+			struct mlx5e_ptp_cq_stats *stats =
+				ptp->ptpsq[i].cq_stats;
+
+			ts_stats->pkts += stats->cqe;
+			ts_stats->err += stats->abort + stats->err_cqe +
+				stats->late_cqe;
+			ts_stats->lost += stats->lost_cqe;
+		}
+	} else {
+		/* DMA layer will always successfully timestamp packets. Other
+		 * counters do not make sense for this layer.
+		 */
+		ts_stats->pkts = 0;
+
+		/* Aggregate stats across all SQs */
+		for (j = 0; j < priv->channels.num; j++) {
+			struct mlx5e_channel *c = priv->channels.c[j];
+
+			for (i = 0; i < c->num_tc; i++) {
+				struct mlx5e_sq_stats *stats = c->sq[i].stats;
+
+				ts_stats->pkts += stats->timestamps;
+			}
+		}
+	}
+
+	mutex_unlock(&priv->state_lock);
+}
+
 #define PPORT_PHY_STATISTICAL_OFF(c) \
 	MLX5_BYTE_OFF(ppcnt_reg, \
 		      counter_set.phys_layer_statistical_cntrs.c##_high)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
index 3c634c5fd420..7b3e6cf1229a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
@@ -126,6 +126,8 @@ void mlx5e_stats_eth_ctrl_get(struct mlx5e_priv *priv,
 void mlx5e_stats_rmon_get(struct mlx5e_priv *priv,
 			  struct ethtool_rmon_stats *rmon,
 			  const struct ethtool_rmon_hist_range **ranges);
+void mlx5e_stats_ts_get(struct mlx5e_priv *priv,
+			struct ethtool_ts_stats *ts_stats);
 void mlx5e_get_link_ext_stats(struct net_device *dev,
 			      struct ethtool_link_ext_stats *stats);
 
-- 
2.42.0

[PATCH net-next v1 5/6] netlink: specs: ethtool: add header-flags enumeration

[Rahul Rameshbabu]

Enable specifying header flags using enumerated entries in the spec file
rather than raw flag values.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 Documentation/netlink/specs/ethtool.yaml |  5 +++++
 include/uapi/linux/ethtool_netlink.h     | 11 +++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index f5aa1e7f3383..bae3bb013787 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -16,6 +16,10 @@ definitions:
     name: stringset
     type: enum
     entries: []
+  -
+    name: header-flags
+    type: flags
+    entries: [ compact-bitset, omit-reply, stats ]
 
 attribute-sets:
   -
@@ -30,6 +34,7 @@ attribute-sets:
       -
         name: flags
         type: u32
+        enum: header-flags
 
   -
     name: bitset-bit
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 8dfd714c2308..34a4a64262b2 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -117,12 +117,11 @@ enum {
 
 /* request header */
 
-/* use compact bitsets in reply */
-#define ETHTOOL_FLAG_COMPACT_BITSETS	(1 << 0)
-/* provide optional reply for SET or ACT requests */
-#define ETHTOOL_FLAG_OMIT_REPLY	(1 << 1)
-/* request statistics, if supported by the driver */
-#define ETHTOOL_FLAG_STATS		(1 << 2)
+enum ethtool_header_flags {
+	ETHTOOL_FLAG_COMPACT_BITSETS	= 1 << 0,	/* use compact bitsets in reply */
+	ETHTOOL_FLAG_OMIT_REPLY		= 1 << 1,	/* provide optional reply for SET or ACT requests */
+	ETHTOOL_FLAG_STATS		= 1 << 2,	/* request statistics, if supported by the driver */
+};
 
 #define ETHTOOL_FLAG_ALL (ETHTOOL_FLAG_COMPACT_BITSETS | \
 			  ETHTOOL_FLAG_OMIT_REPLY | \
-- 
2.42.0

[PATCH net-next v1 6/6] tools: ynl: ethtool.py: Output timestamping statistics from tsinfo-get operation

[Rahul Rameshbabu]

Print the nested stats attribute containing timestamping statistics when
the --show-time-stamping flag is used.

  [root@binary-eater-vm-01 linux-ethtool-ts]# ./tools/net/ynl/ethtool.py --show-time-stamping mlx5_1
  Time stamping parameters for mlx5_1:
  Capabilities:
    hardware-transmit
    hardware-receive
    hardware-raw-clock
  PTP Hardware Clock: 0
  Hardware Transmit Timestamp Modes:
    off
    on
  Hardware Receive Filter Modes:
    none
    all
  Statistics:
    tx-pkts: 8
    tx-lost: 0
    tx-err: 0

Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 tools/net/ynl/ethtool.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/ethtool.py b/tools/net/ynl/ethtool.py
index 6c9f7e31250c..47264ae20036 100755
--- a/tools/net/ynl/ethtool.py
+++ b/tools/net/ynl/ethtool.py
@@ -320,7 +320,13 @@ def main():
         return
 
     if args.show_time_stamping:
-        tsinfo = dumpit(ynl, args, 'tsinfo-get')
+        req = {
+          'header': {
+            'flags': 'stats',
+          },
+        }
+
+        tsinfo = dumpit(ynl, args, 'tsinfo-get', req)
 
         print(f'Time stamping parameters for {args.device}:')
 
@@ -334,6 +340,9 @@ def main():
 
         print('Hardware Receive Filter Modes:')
         [print(f'\t{v}') for v in bits_to_dict(tsinfo['rx-filters'])]
+
+        print('Statistics:')
+        [print(f'\t{k}: {v}') for k, v in tsinfo['stats'].items()]
         return
 
     print(f'Settings for {args.device}:')
-- 
2.42.0

Re: [PATCH net-next v1 1/6] ethtool: add interface to read Tx hardware timestamping statistics

[Jakub Kicinski]

On Tue,  2 Apr 2024 13:52:01 -0700 Rahul Rameshbabu wrote:
> +/**
> + * struct ethtool_ts_stats - HW timestamping statistics
> + * @tx_stats: struct group for TX HW timestamping
> + *	@pkts: Number of packets successfully timestamped by the hardware.
> + *	@lost: Number of hardware timestamping requests where the timestamping
> + *		information from the hardware never arrived for submission with
> + *		the skb.
> + *	@err: Number of arbitrary timestamp generation error events that the
> + *		hardware encountered, exclusive of @lost statistics. Cases such
> + *		as resource exhaustion, unavailability, firmware errors, and
> + *		detected illogical timestamp values not submitted with the skb
> + *		are inclusive to this counter.
> + */
> +struct ethtool_ts_stats {
> +	struct_group(tx_stats,

Doesn't seem like the group should be documented:

include/linux/ethtool.h:503: warning: Excess struct member 'tx_stats' description in 'ethtool_ts_stats'
-- 
pw-bot: cr

Re: [PATCH net-next v1 1/6] ethtool: add interface to read Tx hardware timestamping statistics

[Rahul Rameshbabu]

On Tue, 02 Apr, 2024 19:18:42 -0700 Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue,  2 Apr 2024 13:52:01 -0700 Rahul Rameshbabu wrote:
>> +/**
>> + * struct ethtool_ts_stats - HW timestamping statistics
>> + * @tx_stats: struct group for TX HW timestamping
>> + *	@pkts: Number of packets successfully timestamped by the hardware.
>> + *	@lost: Number of hardware timestamping requests where the timestamping
>> + *		information from the hardware never arrived for submission with
>> + *		the skb.
>> + *	@err: Number of arbitrary timestamp generation error events that the
>> + *		hardware encountered, exclusive of @lost statistics. Cases such
>> + *		as resource exhaustion, unavailability, firmware errors, and
>> + *		detected illogical timestamp values not submitted with the skb
>> + *		are inclusive to this counter.
>> + */
>> +struct ethtool_ts_stats {
>> +	struct_group(tx_stats,
>
> Doesn't seem like the group should be documented:
>
> include/linux/ethtool.h:503: warning: Excess struct member 'tx_stats' description in 'ethtool_ts_stats'

Was looking into why our internal verification did not catch this. We
run W=1 with clang, but looks like the warning does not get triggered
unless explicitly run with scripts/kernel-doc.

  https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-format-kernel-doc-comments

I have debugged using strace that the way the kernel doc checking works
when W=1 is set is that the matching source file that is being compiled
is passed to scripts/kernel-doc, so include files are missed from the
doc check. I think this is worth adding to the kernel documentation.

Anyway, I will send out a v2 with this fixed but wait for potentially
more feedback on v1.

--
Thanks,

Rahul Rameshbabu

RE: [PATCH net-next v1 1/6] ethtool: add interface to read Tx hardware timestamping statistics

[Keller, Jacob E]

> -----Original Message-----
> From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> Sent: Tuesday, April 2, 2024 10:15 PM
> To: Jakub Kicinski <kuba@kernel.org>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; Zaki, Ahmed <ahmed.zaki@intel.com>; Lobakin, Aleksander
> <aleksander.lobakin@intel.com>; alexandre.torgue@foss.st.com;
> andrew@lunn.ch; cjubran@nvidia.com; corbet@lwn.net; davem@davemloft.net;
> dtatulea@nvidia.com; edumazet@google.com; gal@nvidia.com;
> hkallweit1@gmail.com; Keller, Jacob E <jacob.e.keller@intel.com>;
> jiri@resnulli.us; joabreu@synopsys.com; justinstitt@google.com;
> kory.maincent@bootlin.com; leon@kernel.org; liuhangbin@gmail.com;
> maxime.chevallier@bootlin.com; pabeni@redhat.com; Greenwalt, Paul
> <paul.greenwalt@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>;
> rdunlap@infradead.org; richardcochran@gmail.com; saeed@kernel.org;
> tariqt@nvidia.com; vadim.fedorenko@linux.dev; vladimir.oltean@nxp.com;
> Drewek, Wojciech <wojciech.drewek@intel.com>
> Subject: Re: [PATCH net-next v1 1/6] ethtool: add interface to read Tx hardware
> timestamping statistics
> 
> On Tue, 02 Apr, 2024 19:18:42 -0700 Jakub Kicinski <kuba@kernel.org> wrote:
> > On Tue,  2 Apr 2024 13:52:01 -0700 Rahul Rameshbabu wrote:
> >> +/**
> >> + * struct ethtool_ts_stats - HW timestamping statistics
> >> + * @tx_stats: struct group for TX HW timestamping
> >> + *	@pkts: Number of packets successfully timestamped by the hardware.
> >> + *	@lost: Number of hardware timestamping requests where the
> timestamping
> >> + *		information from the hardware never arrived for submission with
> >> + *		the skb.
> >> + *	@err: Number of arbitrary timestamp generation error events that the
> >> + *		hardware encountered, exclusive of @lost statistics. Cases such
> >> + *		as resource exhaustion, unavailability, firmware errors, and
> >> + *		detected illogical timestamp values not submitted with the skb
> >> + *		are inclusive to this counter.
> >> + */
> >> +struct ethtool_ts_stats {
> >> +	struct_group(tx_stats,
> >
> > Doesn't seem like the group should be documented:
> >
> > include/linux/ethtool.h:503: warning: Excess struct member 'tx_stats'
> description in 'ethtool_ts_stats'
> 
> Was looking into why our internal verification did not catch this. We
> run W=1 with clang, but looks like the warning does not get triggered
> unless explicitly run with scripts/kernel-doc.
> 
>   https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-
> format-kernel-doc-comments
> 
> I have debugged using strace that the way the kernel doc checking works
> when W=1 is set is that the matching source file that is being compiled
> is passed to scripts/kernel-doc, so include files are missed from the
> doc check. I think this is worth adding to the kernel documentation.
> 

It would be great if the W=1 setup could figure out the include files and send those to kernel-doc too, but I'm not sure if this is possible and if so how difficult it would be to implement it. A lot of headers produce warnings because a lot fewer people manually run kernel-doc on the entire source.

> Anyway, I will send out a v2 with this fixed but wait for potentially
> more feedback on v1.
> 
> --
> Thanks,
> 
> Rahul Rameshbabu

Re: [PATCH net-next v1 1/6] ethtool: add interface to read Tx hardware timestamping statistics

[Rahul Rameshbabu]

On Wed, 03 Apr, 2024 18:44:52 +0000 "Keller, Jacob E" <jacob.e.keller@intel.com> wrote:
>> On Tue, 02 Apr, 2024 19:18:42 -0700 Jakub Kicinski <kuba@kernel.org> wrote:
>> > On Tue,  2 Apr 2024 13:52:01 -0700 Rahul Rameshbabu wrote:
>> >> +/**
>> >> + * struct ethtool_ts_stats - HW timestamping statistics
>> >> + * @tx_stats: struct group for TX HW timestamping
>> >> + *	@pkts: Number of packets successfully timestamped by the hardware.
>> >> + *	@lost: Number of hardware timestamping requests where the
>> timestamping
>> >> + *		information from the hardware never arrived for submission with
>> >> + *		the skb.
>> >> + *	@err: Number of arbitrary timestamp generation error events that the
>> >> + *		hardware encountered, exclusive of @lost statistics. Cases such
>> >> + *		as resource exhaustion, unavailability, firmware errors, and
>> >> + *		detected illogical timestamp values not submitted with the skb
>> >> + *		are inclusive to this counter.
>> >> + */
>> >> +struct ethtool_ts_stats {
>> >> +	struct_group(tx_stats,
>> >
>> > Doesn't seem like the group should be documented:
>> >
>> > include/linux/ethtool.h:503: warning: Excess struct member 'tx_stats'
>> description in 'ethtool_ts_stats'
>> 
>> Was looking into why our internal verification did not catch this. We
>> run W=1 with clang, but looks like the warning does not get triggered
>> unless explicitly run with scripts/kernel-doc.
>> 
>>   https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#how-to-
>> format-kernel-doc-comments
>> 
>> I have debugged using strace that the way the kernel doc checking works
>> when W=1 is set is that the matching source file that is being compiled
>> is passed to scripts/kernel-doc, so include files are missed from the
>> doc check. I think this is worth adding to the kernel documentation.
>> 
>
> It would be great if the W=1 setup could figure out the include files and send
> those to kernel-doc too, but I'm not sure if this is possible and if so how
> difficult it would be to implement it. A lot of headers produce warnings because
> a lot fewer people manually run kernel-doc on the entire source.
>

I took a look into this, and the one naive solution I had in mind was a
checkdocs target for the kernel where you use gcc -MM to deduce all the
includes, create a unique list, and then run scripts/kernel-doc against
the list of include files. That said, I do think this is excessive
compared to having a checkpatch logic that runs scripts/kernel-doc on
the parts that change in your commit before and after the patch is
applied.

Kudos to the netdev CI for having this.

  https://github.com/linux-netdev/nipa/blob/main/tests/patch/kdoc/kdoc.sh