DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp
@ 2020-05-23 17:21 Vivien Didelot
  2020-05-23 17:21 ` [dpdk-dev] [PATCH 2/2] net/pcap: add TODO for writing Tx HW timestamp Vivien Didelot
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Vivien Didelot @ 2020-05-23 17:21 UTC (permalink / raw)
  To: dev; +Cc: patrick.keroulas, thomas, 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH 2/2] net/pcap: add TODO for writing Tx HW timestamp
  2020-05-23 17:21 [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp Vivien Didelot
@ 2020-05-23 17:21 ` Vivien Didelot
  2020-05-24  1:38   ` Stephen Hemminger
  2020-05-24  1:39 ` [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp Stephen Hemminger
  2020-06-03 17:50 ` Ferruh Yigit
  2 siblings, 1 reply; 11+ messages in thread
From: Vivien Didelot @ 2020-05-23 17:21 UTC (permalink / raw)
  To: dev; +Cc: patrick.keroulas, thomas, 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] net/pcap: add TODO for writing Tx HW timestamp
  2020-05-23 17:21 ` [dpdk-dev] [PATCH 2/2] net/pcap: add TODO for writing Tx HW timestamp Vivien Didelot
@ 2020-05-24  1:38   ` Stephen Hemminger
  2020-06-03 17:31     ` Ferruh Yigit
  0 siblings, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2020-05-24  1:38 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: dev, patrick.keroulas, thomas

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp
  2020-05-23 17:21 [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp Vivien Didelot
  2020-05-23 17:21 ` [dpdk-dev] [PATCH 2/2] net/pcap: add TODO for writing Tx HW timestamp Vivien Didelot
@ 2020-05-24  1:39 ` Stephen Hemminger
  2020-06-03 17:50 ` Ferruh Yigit
  2 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2020-05-24  1:39 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: dev, patrick.keroulas, thomas

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!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] net/pcap: add TODO for writing Tx HW timestamp
  2020-05-24  1:38   ` Stephen Hemminger
@ 2020-06-03 17:31     ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2020-06-03 17:31 UTC (permalink / raw)
  To: Stephen Hemminger, Vivien Didelot; +Cc: dev, patrick.keroulas, thomas

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp
  2020-05-23 17:21 [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp Vivien Didelot
  2020-05-23 17:21 ` [dpdk-dev] [PATCH 2/2] net/pcap: add TODO for writing Tx HW timestamp Vivien Didelot
  2020-05-24  1:39 ` [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp Stephen Hemminger
@ 2020-06-03 17:50 ` Ferruh Yigit
  2020-06-03 20:11   ` Stephen Hemminger
  2 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2020-06-03 17:50 UTC (permalink / raw)
  To: Vivien Didelot, dev; +Cc: patrick.keroulas, thomas

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp
  2020-06-03 17:50 ` Ferruh Yigit
@ 2020-06-03 20:11   ` Stephen Hemminger
       [not found]     ` <20200603162652.GB13716@t480s.localdomain>
  2020-06-04 10:55     ` Ferruh Yigit
  0 siblings, 2 replies; 11+ messages in thread
From: Stephen Hemminger @ 2020-06-03 20:11 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Vivien Didelot, dev, patrick.keroulas, thomas

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp
       [not found]     ` <20200603162652.GB13716@t480s.localdomain>
@ 2020-06-03 21:59       ` Stephen Hemminger
  2020-06-04 10:52       ` Ferruh Yigit
  1 sibling, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2020-06-03 21:59 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: Ferruh Yigit, dev, patrick.keroulas, thomas

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).




 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp
       [not found]     ` <20200603162652.GB13716@t480s.localdomain>
  2020-06-03 21:59       ` Stephen Hemminger
@ 2020-06-04 10:52       ` Ferruh Yigit
  2020-06-04 11:16         ` Ferruh Yigit
  1 sibling, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2020-06-04 10:52 UTC (permalink / raw)
  To: Vivien Didelot, Stephen Hemminger; +Cc: dev, patrick.keroulas, thomas

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp
  2020-06-03 20:11   ` Stephen Hemminger
       [not found]     ` <20200603162652.GB13716@t480s.localdomain>
@ 2020-06-04 10:55     ` Ferruh Yigit
  1 sibling, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2020-06-04 10:55 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Vivien Didelot, dev, patrick.keroulas, thomas

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp
  2020-06-04 10:52       ` Ferruh Yigit
@ 2020-06-04 11:16         ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2020-06-04 11:16 UTC (permalink / raw)
  To: Vivien Didelot, Stephen Hemminger; +Cc: dev, patrick.keroulas, thomas

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-06-04 11:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-23 17:21 [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp Vivien Didelot
2020-05-23 17:21 ` [dpdk-dev] [PATCH 2/2] net/pcap: add TODO for writing Tx HW timestamp Vivien Didelot
2020-05-24  1:38   ` Stephen Hemminger
2020-06-03 17:31     ` Ferruh Yigit
2020-05-24  1:39 ` [dpdk-dev] [PATCH 1/2] net/pcap: support software Tx nanosecond timestamp Stephen Hemminger
2020-06-03 17:50 ` Ferruh Yigit
2020-06-03 20:11   ` Stephen Hemminger
     [not found]     ` <20200603162652.GB13716@t480s.localdomain>
2020-06-03 21:59       ` Stephen Hemminger
2020-06-04 10:52       ` Ferruh Yigit
2020-06-04 11:16         ` Ferruh Yigit
2020-06-04 10:55     ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).