Re: [dpdk-dev] [PATCH 2/2] net/pcap: add TODO for writing Tx HW timestamp

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 5/24/2020 2:38 AM, Stephen Hemminger wrote:
> On Sat, 23 May 2020 13:21:30 -0400
> Vivien Didelot <vivien.didelot@gmail.com> wrote:
> 
>> In order to write a packet with hardware timestamp enabled into a
>> PCAP file, we need to convert its device specific raw timestamp first.
>>
>> This might not be trivial since querying the raw clock value from
>> the device is still experimental, and derivating the frequency would
>> ideally require an additional alarm thread.
>>
>> As a first step, pass the mbuf to the timestamp calculation function
>> since mbuf->port and mbuf->timestamp would be needed, and add a TODO
>> note. No functional changes.
>>
>> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
>> ---
>>  drivers/net/pcap/rte_eth_pcap.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
>> index 68588c3d7..f205a28e0 100644
>> --- a/drivers/net/pcap/rte_eth_pcap.c
>> +++ b/drivers/net/pcap/rte_eth_pcap.c
>> @@ -290,10 +290,16 @@ eth_null_rx(void *queue __rte_unused,
>>  #define NSEC_PER_SEC	1e9
>>  
>>  static inline void
>> -calculate_timestamp(struct timeval *ts) {
>> +calculate_timestamp(const struct rte_mbuf *mbuf, struct timeval *ts) {
>>  	uint64_t cycles;
>>  	struct timeval cur_time;
>>  
>> +	if (mbuf->ol_flags & PKT_RX_TIMESTAMP) {
>> +		/* TODO: convert mbuf->timestamp into nanoseconds instead.
>> +                 * See rte_eth_read_clock().
>> +                 */
>> +	}
>> +
>>  	cycles = rte_get_timer_cycles() - start_cycles;
>>  	cur_time.tv_sec = cycles / hz;
>>  	cur_time.tv_usec = (cycles % hz) * NSEC_PER_SEC / hz;
>> @@ -339,7 +345,7 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>>  			caplen = sizeof(temp_data);
>>  		}
>>  
>> -		calculate_timestamp(&header.ts);
>> +		calculate_timestamp(mbuf, &header.ts);
>>  		header.len = len;
>>  		header.caplen = caplen;
>>  		/* rte_pktmbuf_read() returns a pointer to the data directly
>> -- 
> 
> NAK
> What is the point of this patch.
> Most projects do not accept speculative patches. Instead incorporate this
> patch in when you have code that uses it.
> 

+1