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

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

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@meta.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);
>>   
>