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

[Stephen Hemminger <stephen@networkplumber.org>]

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.