Re: [PATCH net-next 08/10] idpf: add Tx timestamp flows

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 18 Mar 2025 09:13:23 -0700 Tony Nguyen wrote:
> +/**
> + * struct idpf_tx_tstamp_stats - Tx timestamp statistics
> + * @tx_hwtstamp_lock: Lock to protect Tx tstamp stats
> + * @tx_hwtstamp_discarded: Number of Tx skbs discarded due to cached PHC time
> + *			   being too old to correctly extend timestamp
> + * @tx_hwtstamp_flushed: Number of Tx skbs flushed due to interface closed
> + */
> +struct idpf_tx_tstamp_stats {
> +	struct mutex tx_hwtstamp_lock;
> +	u32 tx_hwtstamp_discarded;
> +	u32 tx_hwtstamp_flushed;
> +};

>   * idpf_get_rxnfc - command to get RX flow classification rules
> @@ -479,6 +480,9 @@ static const struct idpf_stats idpf_gstrings_port_stats[] = {
>  	IDPF_PORT_STAT("tx-unicast_pkts", port_stats.vport_stats.tx_unicast),
>  	IDPF_PORT_STAT("tx-multicast_pkts", port_stats.vport_stats.tx_multicast),
>  	IDPF_PORT_STAT("tx-broadcast_pkts", port_stats.vport_stats.tx_broadcast),
> +	IDPF_PORT_STAT("tx-hwtstamp_skipped", port_stats.tx_hwtstamp_skipped),
> +	IDPF_PORT_STAT("tx-hwtstamp_flushed", tstamp_stats.tx_hwtstamp_flushed),
> +	IDPF_PORT_STAT("tx-hwtstamp_discarded", tstamp_stats.tx_hwtstamp_discarded),

I don't see you implementing .get_ts_stats ? If there is a reason
please explain in the commit msg. We require that standard stats
are reported if you want to report custom, more granular ones.

> +static int idpf_get_ts_info(struct net_device *netdev,
> +			    struct kernel_ethtool_ts_info *info)
> +{
> +	struct idpf_netdev_priv *np = netdev_priv(netdev);
> +	struct idpf_vport *vport;
> +	int err = 0;
> +
> +	if (!mutex_trylock(&np->adapter->vport_ctrl_lock))

Why trylock? This also needs a solid and well documented justification
to pass.
-- 
pw-bot: cr