On Sat, 9 Mar 2024 00:44:35 -0800 Rahul Rameshbabu wrote:
> 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.
> Documentation/netlink/specs/ethtool.yaml | 20 +++++++++
> include/linux/ethtool.h | 21 ++++++++++
> include/uapi/linux/ethtool_netlink.h | 15 +++++++
> net/ethtool/tsinfo.c | 52 +++++++++++++++++++++++-
> 4 files changed, 107 insertions(+), 1 deletion(-)
Feels like we should mention the new stats somehow in
Documentation/networking/ethtool-netlink.rst
> diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
> index 197208f419dc..f99b003c78c0 100644
> --- a/Documentation/netlink/specs/ethtool.yaml
> +++ b/Documentation/netlink/specs/ethtool.yaml
> @@ -559,6 +559,21 @@ attribute-sets:
> -
> name: tx-lpi-timer
> type: u32
> + -
> + name: ts-stat
> + attributes:
> + -
> + name: pad
> + type: pad
You can remove the pad entry, and...
> + -
> + name: tx-pkts
> + type: u64
...use the uint type for the stats
> + -
> + name: tx-lost
> + type: u64
> + -
> + name: tx-err
> + type: u64
> -
> name: tsinfo
> attributes:
> +/**
> + * struct ethtool_ts_stats - HW timestamping statistics
> + * @tx_stats: struct group for TX HW timestamping
> + * @pkts: Number of packets successfully timestamped by the queried
> + * layer.
> + * @lost: Number of packet timestamps that failed to get applied on a
> + * packet by the queried layer.
> + * @err: Number of timestamping errors that occurred on the queried
> + * layer.
the kdocs for @lost and @err are not very clear
> + * @get_ts_stats: Query the device hardware timestamping statistics.
Let's copy & paste the "Drivers must not zero" text in here?
People seem to miss that requirement anyway, but at least we'll
have something to point at in review.
> +enum {
> + ETHTOOL_A_TS_STAT_UNSPEC,
> + ETHTOOL_A_TS_STAT_PAD,
> +
> + ETHTOOL_A_TS_STAT_TX_PKT, /* array, u64 */
> + ETHTOOL_A_TS_STAT_TX_LOST, /* array, u64 */
> + ETHTOOL_A_TS_STAT_TX_ERR, /* array, u64 */
I don't think these are arrays.
> +
> + /* add new constants above here */
> + __ETHTOOL_A_TS_STAT_CNT,
> + ETHTOOL_A_TS_STAT_MAX = (__ETHTOOL_A_TS_STAT_CNT - 1)
> +
> +};
