netdev - Re: [PATCH net-next v3 1/5] eth: fbnic: add software TX timestamping support

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: <ac195016-a05a-4757-9876-94d076937af7@linux.dev>
Date: Mon, 7 Oct 2024 10:56:43 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Jacob Keller <jacob.e.keller@...el.com>, Vadim Fedorenko
 <vadfed@...a.com>, Jakub Kicinski <kuba@...nel.org>,
 David Ahern <dsahern@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 "David S. Miller" <davem@...emloft.net>,
 Alexander Duyck <alexanderduyck@...com>
Cc: netdev@...r.kernel.org, Richard Cochran <richardcochran@...il.com>
Subject: Re: [PATCH net-next v3 1/5] eth: fbnic: add software TX timestamping
 support

On 04/10/2024 23:55, Jacob Keller wrote:
> 
> 
> On 10/3/2024 5:39 AM, Vadim Fedorenko wrote:
>> Add software TX timestamping support. RX software timestamping is
>> implemented in the core and there is no need to provide special flag
>> in the driver anymore.
>>
>> Signed-off-by: Vadim Fedorenko <vadfed@...a.com>
>> ---
>>   drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c | 11 +++++++++++
>>   drivers/net/ethernet/meta/fbnic/fbnic_txrx.c    |  3 +++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>> index 5d980e178941..ffc773014e0f 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>> @@ -6,6 +6,16 @@
>>   #include "fbnic_netdev.h"
>>   #include "fbnic_tlv.h"
>>   
>> +static int
>> +fbnic_get_ts_info(struct net_device *netdev,
>> +		  struct kernel_ethtool_ts_info *tsinfo)
>> +{
>> +	tsinfo->so_timestamping =
>> +		SOF_TIMESTAMPING_TX_SOFTWARE;
>> +
>> +	return 0;
>> +}
>> +
> 
> You could use ethtool_op_get_ts_info(), but I imagine future patches
> will update this for hardware timestamping, so I don't think thats a big
> deal.
> 
> I think you *do* still want to report SOF_TIMESTAMPING_RX_SOFTWARE and
> SOF_TIMESTAMPING_SOFTWARE to get the API correct... Perhaps that could
> be improved in the core stack though.... Or did that already get changed
> recently?

Yeah, as you found in the next mail, software RX timestamping was moved
to the core recently.

> You should also set phc_index to -1 until you have a PTP clock device.

That's definitely missing, thanks! I'll add it to the next version.

> 
>>   static void
>>   fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
>>   {
>> @@ -66,6 +76,7 @@ fbnic_get_eth_mac_stats(struct net_device *netdev,
>>   
>>   static const struct ethtool_ops fbnic_ethtool_ops = {
>>   	.get_drvinfo		= fbnic_get_drvinfo,
>> +	.get_ts_info		= fbnic_get_ts_info,
>>   	.get_eth_mac_stats	= fbnic_get_eth_mac_stats,
>>   };
>>   
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
>> index 6a6d7e22f1a7..8337d49bad0b 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
>> @@ -205,6 +205,9 @@ fbnic_tx_map(struct fbnic_ring *ring, struct sk_buff *skb, __le64 *meta)
>>   
>>   	ring->tail = tail;
>>   
>> +	/* Record SW timestamp */
>> +	skb_tx_timestamp(skb);
>> +
>>   	/* Verify there is room for another packet */
>>   	fbnic_maybe_stop_tx(skb->dev, ring, FBNIC_MAX_SKB_DESC);
>>   
>