[dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp

[dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp

[Vivien Didelot]

When capturing packets into a PCAP file, DPDK currently uses
microseconds for the timestamp. But libpcap supports interpreting
tv_usec as nanoseconds depending on the file timestamp precision.

To support this, use PCAP_TSTAMP_PRECISION_NANO when creating the
empty PCAP file as specified by PCAP_OPEN_DEAD(3PCAP) and implement
nanosecond timeval addition. This also ensures that the precision
reported by capinfos is nanoseconds (9).

Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index b4c79d174..68588c3d7 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -287,6 +287,8 @@ eth_null_rx(void *queue __rte_unused,
 	return 0;
 }
 
+#define NSEC_PER_SEC	1e9
+
 static inline void
 calculate_timestamp(struct timeval *ts) {
 	uint64_t cycles;
@@ -294,8 +296,14 @@ calculate_timestamp(struct timeval *ts) {
 
 	cycles = rte_get_timer_cycles() - start_cycles;
 	cur_time.tv_sec = cycles / hz;
-	cur_time.tv_usec = (cycles % hz) * 1e6 / hz;
-	timeradd(&start_time, &cur_time, ts);
+	cur_time.tv_usec = (cycles % hz) * NSEC_PER_SEC / hz;
+
+	ts->tv_sec = start_time.tv_sec + cur_time.tv_sec;
+	ts->tv_usec = start_time.tv_usec + cur_time.tv_usec;
+	if (ts->tv_usec > NSEC_PER_SEC) {
+		ts->tv_usec -= NSEC_PER_SEC;
+		ts->tv_sec += 1;
+	}
 }
 
 /*
@@ -475,7 +483,8 @@ open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper)
 	 * with pcap_dump_open(). We create big enough an Ethernet
 	 * pcap holder.
 	 */
-	tx_pcap = pcap_open_dead(DLT_EN10MB, RTE_ETH_PCAP_SNAPSHOT_LEN);
+	tx_pcap = pcap_open_dead_with_tstamp_precision(DLT_EN10MB,
+			RTE_ETH_PCAP_SNAPSHOT_LEN, PCAP_TSTAMP_PRECISION_NANO);
 	if (tx_pcap == NULL) {
 		PMD_LOG(ERR, "Couldn't create dead pcap");
 		return -1;
-- 
2.26.2

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

[Vivien Didelot]

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
-- 
2.26.2

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

[Stephen Hemminger]

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.

Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp

[Stephen Hemminger]

On Sat, 23 May 2020 13:21:29 -0400
Vivien Didelot <vivien.didelot@gmail.com> wrote:

> When capturing packets into a PCAP file, DPDK currently uses
> microseconds for the timestamp. But libpcap supports interpreting
> tv_usec as nanoseconds depending on the file timestamp precision.
> 
> To support this, use PCAP_TSTAMP_PRECISION_NANO when creating the
> empty PCAP file as specified by PCAP_OPEN_DEAD(3PCAP) and implement
> nanosecond timeval addition. This also ensures that the precision
> reported by capinfos is nanoseconds (9).
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
> ---
>  drivers/net/pcap/rte_eth_pcap.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index b4c79d174..68588c3d7 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -287,6 +287,8 @@ eth_null_rx(void *queue __rte_unused,
>  	return 0;
>  }
>  
> +#define NSEC_PER_SEC	1e9
> +
>  static inline void
>  calculate_timestamp(struct timeval *ts) {
>  	uint64_t cycles;
> @@ -294,8 +296,14 @@ calculate_timestamp(struct timeval *ts) {
>  
>  	cycles = rte_get_timer_cycles() - start_cycles;
>  	cur_time.tv_sec = cycles / hz;
> -	cur_time.tv_usec = (cycles % hz) * 1e6 / hz;
> -	timeradd(&start_time, &cur_time, ts);
> +	cur_time.tv_usec = (cycles % hz) * NSEC_PER_SEC / hz;
> +
> +	ts->tv_sec = start_time.tv_sec + cur_time.tv_sec;
> +	ts->tv_usec = start_time.tv_usec + cur_time.tv_usec;
> +	if (ts->tv_usec > NSEC_PER_SEC) {
> +		ts->tv_usec -= NSEC_PER_SEC;
> +		ts->tv_sec += 1;
> +	}

Please no floating point math in the fast path of capturing packets!

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

[Ferruh Yigit]

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

Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp

[Ferruh Yigit]

On 5/23/2020 6:21 PM, Vivien Didelot wrote:
> When capturing packets into a PCAP file, DPDK currently uses
> microseconds for the timestamp. But libpcap supports interpreting
> tv_usec as nanoseconds depending on the file timestamp precision.
> 
> To support this, use PCAP_TSTAMP_PRECISION_NANO when creating the
> empty PCAP file as specified by PCAP_OPEN_DEAD(3PCAP) and implement
> nanosecond timeval addition. This also ensures that the precision
> reported by capinfos is nanoseconds (9).

Overall good idea and patch looks good.

Only concern is change in the libpcap dependency. Do you know which libpcap
version supports 'PCAP_TSTAMP_PRECISION_NANO'?
If a user of pcap PMD updates to latest DPDK and the environment doesn't have
new version of the libpcap, this change will require an environment update and
this may or may not be easy to do. That is why not sure if the updates require
dependency change should wait for the LTS or not.

Another things is the backward compatibility, as far as I understand the pcap
file recorded with nanosecond precision can be read and parsed without problem
by old application that doesn't know the nanosecond precision, but can you
please confirm this?

Thanks,
ferruh

> 
> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
> ---
>  drivers/net/pcap/rte_eth_pcap.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index b4c79d174..68588c3d7 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -287,6 +287,8 @@ eth_null_rx(void *queue __rte_unused,
>  	return 0;
>  }
>  
> +#define NSEC_PER_SEC	1e9
> +
>  static inline void
>  calculate_timestamp(struct timeval *ts) {
>  	uint64_t cycles;
> @@ -294,8 +296,14 @@ calculate_timestamp(struct timeval *ts) {
>  
>  	cycles = rte_get_timer_cycles() - start_cycles;
>  	cur_time.tv_sec = cycles / hz;
> -	cur_time.tv_usec = (cycles % hz) * 1e6 / hz;
> -	timeradd(&start_time, &cur_time, ts);
> +	cur_time.tv_usec = (cycles % hz) * NSEC_PER_SEC / hz;
> +
> +	ts->tv_sec = start_time.tv_sec + cur_time.tv_sec;
> +	ts->tv_usec = start_time.tv_usec + cur_time.tv_usec;
> +	if (ts->tv_usec > NSEC_PER_SEC) {
> +		ts->tv_usec -= NSEC_PER_SEC;
> +		ts->tv_sec += 1;
> +	}
>  }
>  
>  /*
> @@ -475,7 +483,8 @@ open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper)
>  	 * with pcap_dump_open(). We create big enough an Ethernet
>  	 * pcap holder.
>  	 */
> -	tx_pcap = pcap_open_dead(DLT_EN10MB, RTE_ETH_PCAP_SNAPSHOT_LEN);
> +	tx_pcap = pcap_open_dead_with_tstamp_precision(DLT_EN10MB,
> +			RTE_ETH_PCAP_SNAPSHOT_LEN, PCAP_TSTAMP_PRECISION_NANO);
>  	if (tx_pcap == NULL) {
>  		PMD_LOG(ERR, "Couldn't create dead pcap");
>  		return -1;
> 

Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp

[Stephen Hemminger]

On Wed, 3 Jun 2020 18:50:51 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 5/23/2020 6:21 PM, Vivien Didelot wrote:
> > When capturing packets into a PCAP file, DPDK currently uses
> > microseconds for the timestamp. But libpcap supports interpreting
> > tv_usec as nanoseconds depending on the file timestamp precision.
> > 
> > To support this, use PCAP_TSTAMP_PRECISION_NANO when creating the
> > empty PCAP file as specified by PCAP_OPEN_DEAD(3PCAP) and implement
> > nanosecond timeval addition. This also ensures that the precision
> > reported by capinfos is nanoseconds (9).  
> 
> Overall good idea and patch looks good.
> 
> Only concern is change in the libpcap dependency. Do you know which libpcap
> version supports 'PCAP_TSTAMP_PRECISION_NANO'?
> If a user of pcap PMD updates to latest DPDK and the environment doesn't have
> new version of the libpcap, this change will require an environment update and
> this may or may not be easy to do. That is why not sure if the updates require
> dependency change should wait for the LTS or not.
> 
> Another things is the backward compatibility, as far as I understand the pcap
> file recorded with nanosecond precision can be read and parsed without problem
> by old application that doesn't know the nanosecond precision, but can you
> please confirm this?

We should do pcapng instead of doing the pcap with nano timestamp.
My impression is that libpcap is a dormant project at this point, and the
finer grain timestamp is a hack that is only in some verisions.
That was one of the reasons pcapng started.

Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp

[Stephen Hemminger]

On Wed, 3 Jun 2020 16:26:52 -0400
Vivien Didelot <vivien.didelot@gmail.com> wrote:

> On Wed, 3 Jun 2020 13:11:39 -0700, Stephen Hemminger <stephen@networkplumber.org> wrote:
> > On Wed, 3 Jun 2020 18:50:51 +0100
> > Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >   
> > > On 5/23/2020 6:21 PM, Vivien Didelot wrote:  
> > > > When capturing packets into a PCAP file, DPDK currently uses
> > > > microseconds for the timestamp. But libpcap supports interpreting
> > > > tv_usec as nanoseconds depending on the file timestamp precision.
> > > > 
> > > > To support this, use PCAP_TSTAMP_PRECISION_NANO when creating the
> > > > empty PCAP file as specified by PCAP_OPEN_DEAD(3PCAP) and implement
> > > > nanosecond timeval addition. This also ensures that the precision
> > > > reported by capinfos is nanoseconds (9).    
> > > 
> > > Overall good idea and patch looks good.
> > > 
> > > Only concern is change in the libpcap dependency. Do you know which libpcap
> > > version supports 'PCAP_TSTAMP_PRECISION_NANO'?
> > > If a user of pcap PMD updates to latest DPDK and the environment doesn't have
> > > new version of the libpcap, this change will require an environment update and
> > > this may or may not be easy to do. That is why not sure if the updates require
> > > dependency change should wait for the LTS or not.
> > > 
> > > Another things is the backward compatibility, as far as I understand the pcap
> > > file recorded with nanosecond precision can be read and parsed without problem
> > > by old application that doesn't know the nanosecond precision, but can you
> > > please confirm this?  
> > 
> > We should do pcapng instead of doing the pcap with nano timestamp.
> > My impression is that libpcap is a dormant project at this point, and the
> > finer grain timestamp is a hack that is only in some verisions.
> > That was one of the reasons pcapng started.  
> 
> Reading tv_usec as nanoseconds has been supported for years in libpcap.
> 
> For instance PCAP_TSTAMP_PRECISION_NANO was introduced in ba89e4a18e8b
> ("Make timestamps precision configurable") on May 17, 2013.
> 
> BTW, I have no clue what you mean by "floating point math".

It was a concern that 1e9 constant in C is interpreted as floating point.
But probably a non-issue. Have gotten burned in the past by floating point
creeping into DPDK (in Qos code).




 

Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp

[Ferruh Yigit]

On 6/3/2020 9:26 PM, Vivien Didelot wrote:
> On Wed, 3 Jun 2020 13:11:39 -0700, Stephen Hemminger <stephen@networkplumber.org> wrote:
>> On Wed, 3 Jun 2020 18:50:51 +0100
>> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>>> On 5/23/2020 6:21 PM, Vivien Didelot wrote:
>>>> When capturing packets into a PCAP file, DPDK currently uses
>>>> microseconds for the timestamp. But libpcap supports interpreting
>>>> tv_usec as nanoseconds depending on the file timestamp precision.
>>>>
>>>> To support this, use PCAP_TSTAMP_PRECISION_NANO when creating the
>>>> empty PCAP file as specified by PCAP_OPEN_DEAD(3PCAP) and implement
>>>> nanosecond timeval addition. This also ensures that the precision
>>>> reported by capinfos is nanoseconds (9).  
>>>
>>> Overall good idea and patch looks good.
>>>
>>> Only concern is change in the libpcap dependency. Do you know which libpcap
>>> version supports 'PCAP_TSTAMP_PRECISION_NANO'?
>>> If a user of pcap PMD updates to latest DPDK and the environment doesn't have
>>> new version of the libpcap, this change will require an environment update and
>>> this may or may not be easy to do. That is why not sure if the updates require
>>> dependency change should wait for the LTS or not.
>>>
>>> Another things is the backward compatibility, as far as I understand the pcap
>>> file recorded with nanosecond precision can be read and parsed without problem
>>> by old application that doesn't know the nanosecond precision, but can you
>>> please confirm this?
>>
>> We should do pcapng instead of doing the pcap with nano timestamp.
>> My impression is that libpcap is a dormant project at this point, and the
>> finer grain timestamp is a hack that is only in some verisions.
>> That was one of the reasons pcapng started.
> 
> Reading tv_usec as nanoseconds has been supported for years in libpcap.
> 
> For instance PCAP_TSTAMP_PRECISION_NANO was introduced in ba89e4a18e8b
> ("Make timestamps precision configurable") on May 17, 2013.

Thanks for the commit info, it looks like this support exists since
libpcap-1.5.0, and even CentOS 7 has libpcap-1.5.3-xx so looks safe to proceed.

Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp

[Ferruh Yigit]

On 6/3/2020 9:11 PM, Stephen Hemminger wrote:
> On Wed, 3 Jun 2020 18:50:51 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> On 5/23/2020 6:21 PM, Vivien Didelot wrote:
>>> When capturing packets into a PCAP file, DPDK currently uses
>>> microseconds for the timestamp. But libpcap supports interpreting
>>> tv_usec as nanoseconds depending on the file timestamp precision.
>>>
>>> To support this, use PCAP_TSTAMP_PRECISION_NANO when creating the
>>> empty PCAP file as specified by PCAP_OPEN_DEAD(3PCAP) and implement
>>> nanosecond timeval addition. This also ensures that the precision
>>> reported by capinfos is nanoseconds (9).  
>>
>> Overall good idea and patch looks good.
>>
>> Only concern is change in the libpcap dependency. Do you know which libpcap
>> version supports 'PCAP_TSTAMP_PRECISION_NANO'?
>> If a user of pcap PMD updates to latest DPDK and the environment doesn't have
>> new version of the libpcap, this change will require an environment update and
>> this may or may not be easy to do. That is why not sure if the updates require
>> dependency change should wait for the LTS or not.
>>
>> Another things is the backward compatibility, as far as I understand the pcap
>> file recorded with nanosecond precision can be read and parsed without problem
>> by old application that doesn't know the nanosecond precision, but can you
>> please confirm this?
> 
> We should do pcapng instead of doing the pcap with nano timestamp.
> My impression is that libpcap is a dormant project at this point, and the
> finer grain timestamp is a hack that is only in some verisions.
> That was one of the reasons pcapng started.
> 

When there is a way to get better precision timestamp from pcap, why not benefit
from it. We can discuss switching to pcapng separately I think.

Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp

[Ferruh Yigit]

On 6/4/2020 11:52 AM, Ferruh Yigit wrote:
> On 6/3/2020 9:26 PM, Vivien Didelot wrote:
>> On Wed, 3 Jun 2020 13:11:39 -0700, Stephen Hemminger <stephen@networkplumber.org> wrote:
>>> On Wed, 3 Jun 2020 18:50:51 +0100
>>> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>
>>>> On 5/23/2020 6:21 PM, Vivien Didelot wrote:
>>>>> When capturing packets into a PCAP file, DPDK currently uses
>>>>> microseconds for the timestamp. But libpcap supports interpreting
>>>>> tv_usec as nanoseconds depending on the file timestamp precision.
>>>>>
>>>>> To support this, use PCAP_TSTAMP_PRECISION_NANO when creating the
>>>>> empty PCAP file as specified by PCAP_OPEN_DEAD(3PCAP) and implement
>>>>> nanosecond timeval addition. This also ensures that the precision
>>>>> reported by capinfos is nanoseconds (9).  
>>>>
>>>> Overall good idea and patch looks good.
>>>>
>>>> Only concern is change in the libpcap dependency. Do you know which libpcap
>>>> version supports 'PCAP_TSTAMP_PRECISION_NANO'?
>>>> If a user of pcap PMD updates to latest DPDK and the environment doesn't have
>>>> new version of the libpcap, this change will require an environment update and
>>>> this may or may not be easy to do. That is why not sure if the updates require
>>>> dependency change should wait for the LTS or not.
>>>>
>>>> Another things is the backward compatibility, as far as I understand the pcap
>>>> file recorded with nanosecond precision can be read and parsed without problem
>>>> by old application that doesn't know the nanosecond precision, but can you
>>>> please confirm this?
>>>
>>> We should do pcapng instead of doing the pcap with nano timestamp.
>>> My impression is that libpcap is a dormant project at this point, and the
>>> finer grain timestamp is a hack that is only in some verisions.
>>> That was one of the reasons pcapng started.
>>
>> Reading tv_usec as nanoseconds has been supported for years in libpcap.
>>
>> For instance PCAP_TSTAMP_PRECISION_NANO was introduced in ba89e4a18e8b
>> ("Make timestamps precision configurable") on May 17, 2013.
> 
> Thanks for the commit info, it looks like this support exists since
> libpcap-1.5.0, and even CentOS 7 has libpcap-1.5.3-xx so looks safe to proceed.
> 

But it may be good to mention from the change in the release notes, can you
please update the release notes (doc/guides/rel_notes/release_20_08.rst) in next
version of the patch.