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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <75d06e1d-a21d-4365-a793-2dc593d6e751@intel.com>
Date: Tue, 25 Mar 2025 14:10:25 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Jakub Kicinski <kuba@...nel.org>, Tony Nguyen <anthony.l.nguyen@...el.com>
CC: <davem@...emloft.net>, <pabeni@...hat.com>, <edumazet@...gle.com>,
	<andrew+netdev@...n.ch>, <netdev@...r.kernel.org>, Milena Olech
	<milena.olech@...el.com>, <przemyslaw.kitszel@...el.com>,
	<karol.kolacinski@...el.com>, <richardcochran@...il.com>, Josh Hay
	<joshua.a.hay@...el.com>, Samuel Salin <Samuel.salin@...el.com>
Subject: Re: [PATCH net-next 08/10] idpf: add Tx timestamp flows



On 3/25/2025 6:00 AM, Jakub Kicinski wrote:
> 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.
> 

My guess is that other Intel drivers haven't yet gotten around to adding
this :(  It wasn't in the kernel when we submitted ice support.
Obviously not an excuse, just an observation.

>> +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.