[dpdk-dev] [RFC] [PATCH v1 0/3] pdump HW timestamps for mlx5

[dpdk-dev] [RFC] [PATCH v1 0/3] pdump HW timestamps for mlx5

[Patrick Keroulas]

The intention is to produce a pcap with nanosecond precision when
timestamp offloading is activated on mlx5 NIC.

The packets forwarded by testpmd hold the raw counter but a pcap
requires a time unit. Assuming that the NIC clock is already synced
with external master clock, this patchset simply integrates the
nanosecond converter that is already implemented by ibverbs. The
conversion is performed is pdump rx callback.

Patrick Keroulas (3):
  net/mlx5: add timestamp-to-ns converter from libibverbs
  ethdev: add API to convert raw timestamps to nsec
  net/pcap: dump hardware timestamps

 drivers/common/mlx5/mlx5_glue.c          | 16 ++++++++++++
 drivers/common/mlx5/mlx5_glue.h          |  4 +++
 drivers/net/mlx5/mlx5.c                  |  1 +
 drivers/net/mlx5/mlx5.h                  |  1 +
 drivers/net/mlx5/mlx5_ethdev.c           | 30 ++++++++++++++++++++++
 drivers/net/pcap/rte_eth_pcap.c          | 32 ++++++++++++++++--------
 lib/librte_ethdev/rte_ethdev.c           | 12 +++++++++
 lib/librte_ethdev/rte_ethdev.h           | 17 +++++++++++++
 lib/librte_ethdev/rte_ethdev_core.h      |  5 ++++
 lib/librte_ethdev/rte_ethdev_version.map |  2 ++
 lib/librte_mbuf/rte_mbuf_core.h          |  3 ++-
 lib/librte_pdump/rte_pdump.c             | 15 ++++++++++-
 12 files changed, 126 insertions(+), 12 deletions(-)

-- 
2.17.1

[dpdk-dev] [RFC] [PATCH v1 1/3] net/mlx5: add timestamp-to-ns converter from libibverbs

[Patrick Keroulas]

While some devices update their own clock info to provide current time,
mlx5dv part of libibverbs already handles this and also converts any raw
counter cycle to nanoseconds.

Signed-off-by: Patrick Keroulas <patrick.keroulas@radio-canada.ca>
---
 drivers/common/mlx5/mlx5_glue.c | 16 ++++++++++++++++
 drivers/common/mlx5/mlx5_glue.h |  4 ++++
 drivers/net/mlx5/mlx5.c         |  1 +
 drivers/net/mlx5/mlx5.h         |  1 +
 drivers/net/mlx5/mlx5_ethdev.c  | 30 ++++++++++++++++++++++++++++++
 5 files changed, 52 insertions(+)

diff --git a/drivers/common/mlx5/mlx5_glue.c b/drivers/common/mlx5/mlx5_glue.c
index f270f677b..756f24b8b 100644
--- a/drivers/common/mlx5/mlx5_glue.c
+++ b/drivers/common/mlx5/mlx5_glue.c
@@ -80,6 +80,20 @@ mlx5_glue_query_rt_values_ex(struct ibv_context *context,
 	return ibv_query_rt_values_ex(context, values);
 }
 
+static int
+mlx5_glue_get_clock_info(struct ibv_context *context,
+			  struct mlx5dv_clock_info *clock_info)
+{
+	return mlx5dv_get_clock_info(context, clock_info);
+}
+
+static uint64_t
+mlx5_glue_mlx5dv_ts_to_ns(struct mlx5dv_clock_info *clock_info,
+			  uint64_t device_timestamp)
+{
+	return mlx5dv_ts_to_ns(clock_info, device_timestamp);
+}
+
 static int
 mlx5_glue_query_port(struct ibv_context *context, uint8_t port_num,
 		     struct ibv_port_attr *port_attr)
@@ -1195,6 +1209,8 @@ const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue) {
 	.query_device = mlx5_glue_query_device,
 	.query_device_ex = mlx5_glue_query_device_ex,
 	.query_rt_values_ex = mlx5_glue_query_rt_values_ex,
+	.get_clock_info = mlx5_glue_get_clock_info,
+	.convert_ts_to_ns = mlx5_glue_mlx5dv_ts_to_ns,
 	.query_port = mlx5_glue_query_port,
 	.create_comp_channel = mlx5_glue_create_comp_channel,
 	.destroy_comp_channel = mlx5_glue_destroy_comp_channel,
diff --git a/drivers/common/mlx5/mlx5_glue.h b/drivers/common/mlx5/mlx5_glue.h
index 81d6a2267..d4b3bd793 100644
--- a/drivers/common/mlx5/mlx5_glue.h
+++ b/drivers/common/mlx5/mlx5_glue.h
@@ -123,6 +123,10 @@ struct mlx5_glue {
 			  struct ibv_port_attr *port_attr);
 	struct ibv_comp_channel *(*create_comp_channel)
 		(struct ibv_context *context);
+	int (*get_clock_info)(struct ibv_context *context,
+			       struct mlx5dv_clock_info *clock_info);
+	uint64_t (*convert_ts_to_ns)(struct mlx5dv_clock_info *clock_info,
+			  uint64_t device_timestamp);
 	int (*destroy_comp_channel)(struct ibv_comp_channel *channel);
 	struct ibv_cq *(*create_cq)(struct ibv_context *context, int cqe,
 				    void *cq_context,
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 5589772eb..7b201844c 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -1569,6 +1569,7 @@ const struct eth_dev_ops mlx5_dev_ops = {
 	.xstats_get_names = mlx5_xstats_get_names,
 	.fw_version_get = mlx5_fw_version_get,
 	.dev_infos_get = mlx5_dev_infos_get,
+	.convert_ts_to_ns = mlx5_convert_ts_to_ns,
 	.read_clock = mlx5_read_clock,
 	.dev_supported_ptypes_get = mlx5_dev_supported_ptypes_get,
 	.vlan_filter_set = mlx5_vlan_filter_set,
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index d9f5d816f..905e24487 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -656,6 +656,7 @@ int mlx5_set_flags(struct rte_eth_dev *dev, unsigned int keep,
 		   unsigned int flags);
 int mlx5_dev_configure(struct rte_eth_dev *dev);
 int mlx5_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info);
+int mlx5_convert_ts_to_ns(struct rte_eth_dev *dev, uint64_t *timestamp);
 int mlx5_read_clock(struct rte_eth_dev *dev, uint64_t *clock);
 int mlx5_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t fw_size);
 const uint32_t *mlx5_dev_supported_ptypes_get(struct rte_eth_dev *dev);
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 47f11b963..1d0411f6b 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -690,6 +690,36 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
 	return 0;
 }
 
+/**
+ * Convert raw clock counter to nanoseconds
+ *
+ * @param dev
+ *   Pointer to Ethernet device structure.
+ * @param[in&out] timestamp
+ *   Pointer to the timestamp to be converted.
+ *
+ * @return
+ *   0 if the clock has correctly been read
+ *   The value of errno in case of error
+ */
+int
+mlx5_convert_ts_to_ns(struct rte_eth_dev *dev, uint64_t *timestamp)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	struct ibv_context *ctx = priv->sh->ctx;
+	struct mlx5dv_clock_info clock_info;
+
+	int err = mlx5_glue->get_clock_info(ctx, &clock_info);
+	if (err != 0) {
+		DRV_LOG(WARNING, "Could not get the clock info!");
+		return err;
+	}
+
+	*timestamp = mlx5_glue->convert_ts_to_ns(&clock_info, *timestamp);
+
+	return err;
+}
+
 /**
  * Get device current raw clock counter
  *
-- 
2.17.1

[dpdk-dev] [RFC] [PATCH v1 2/3] ethdev: add API to convert raw timestamps to nsec

[Patrick Keroulas]

Existing ethdev functions can read/write time from/to device but
they're all related to timesync and none of them can translate a raw
counter in real time unit  which is usefull in a pdump application.

A new API is required because the conversion is derived from dev clock
info.

Signed-off-by: Patrick Keroulas <patrick.keroulas@radio-canada.ca>
---
 lib/librte_ethdev/rte_ethdev.c           | 12 ++++++++++++
 lib/librte_ethdev/rte_ethdev.h           | 17 +++++++++++++++++
 lib/librte_ethdev/rte_ethdev_core.h      |  5 +++++
 lib/librte_ethdev/rte_ethdev_version.map |  2 ++
 lib/librte_mbuf/rte_mbuf_core.h          |  3 ++-
 lib/librte_pdump/rte_pdump.c             | 15 ++++++++++++++-
 6 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 8e10a6fc3..822fa6d5a 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -4810,6 +4810,18 @@ rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *timestamp)
 								timestamp));
 }
 
+int
+rte_eth_convert_ts_to_ns(uint16_t port_id, uint64_t *timestamp)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->convert_ts_to_ns, -ENOTSUP);
+	return eth_err(port_id, (*dev->dev_ops->convert_ts_to_ns)(dev, timestamp));
+}
+
 int
 rte_eth_read_clock(uint16_t port_id, uint64_t *clock)
 {
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index a49242bcd..2d4d0bc7d 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4103,6 +4103,23 @@ int rte_eth_timesync_read_time(uint16_t port_id, struct timespec *time);
  */
 int rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *time);
 
+/**
+ * Convert a raw clock counter to nanoseconds from device clock
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param[in&out] timestamp
+ *   Pointer to the timestamp to be converted.
+ *
+ * @return
+ *   - 0: Success.
+ *   - -ENODEV: The port ID is invalid.
+ *   - -ENOTSUP: The function is not supported by the Ethernet driver.
+ */
+__rte_experimental
+int
+rte_eth_convert_ts_to_ns(uint16_t port_id, uint64_t *timestamp);
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
index 32407dd41..255b41b67 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -464,6 +464,10 @@ typedef int (*eth_timesync_write_time)(struct rte_eth_dev *dev,
 				       const struct timespec *timestamp);
 /**< @internal Function used to get time from the device clock */
 
+typedef int (*eth_convert_ts_to_ns)(struct rte_eth_dev *dev,
+				      uint64_t *timestamp);
+/**< @internal Function used to convert timestamp from device clock */
+
 typedef int (*eth_read_clock)(struct rte_eth_dev *dev,
 				      uint64_t *timestamp);
 /**< @internal Function used to get the current value of the device clock. */
@@ -730,6 +734,7 @@ struct eth_dev_ops {
 	eth_timesync_read_time     timesync_read_time; /** Get the device clock time. */
 	eth_timesync_write_time    timesync_write_time; /** Set the device clock time. */
 
+	eth_convert_ts_to_ns       convert_ts_to_ns;
 	eth_read_clock             read_clock;
 
 	eth_xstats_get_by_id_t     xstats_get_by_id;
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 715505604..754c05630 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -241,4 +241,6 @@ EXPERIMENTAL {
 	__rte_ethdev_trace_rx_burst;
 	__rte_ethdev_trace_tx_burst;
 	rte_flow_get_aged_flows;
+
+	rte_eth_convert_ts_to_ns;
 };
diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h
index b9a59c879..7f51f9157 100644
--- a/lib/librte_mbuf/rte_mbuf_core.h
+++ b/lib/librte_mbuf/rte_mbuf_core.h
@@ -592,7 +592,8 @@ struct rte_mbuf {
 	/** Valid if PKT_RX_TIMESTAMP is set. The unit and time reference
 	 * are not normalized but are always the same for a given port.
 	 * Some devices allow to query rte_eth_read_clock that will return the
-	 * current device timestamp.
+	 * current device timestamp or rte_eth_ts_to_ns that will convert raw
+	 * counter to nanoseconds.
 	 */
 	uint64_t timestamp;
 
diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c
index f96709f95..bc80ed0c6 100644
--- a/lib/librte_pdump/rte_pdump.c
+++ b/lib/librte_pdump/rte_pdump.c
@@ -100,12 +100,25 @@ pdump_copy(struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params)
 	}
 }
 
+static inline void
+pdump_ts_to_ns(uint16_t port_id, struct rte_mbuf **pkts, uint16_t nb_pkts)
+{
+	unsigned i;
+
+	for (i=0; i < nb_pkts; i++) {
+		if (pkts[i]->ol_flags & PKT_RX_TIMESTAMP) {
+			rte_eth_convert_ts_to_ns(port_id, &pkts[i]->timestamp);
+		}
+	}
+}
+
 static uint16_t
-pdump_rx(uint16_t port __rte_unused, uint16_t qidx __rte_unused,
+pdump_rx(uint16_t port_id, uint16_t qidx __rte_unused,
 	struct rte_mbuf **pkts, uint16_t nb_pkts,
 	uint16_t max_pkts __rte_unused,
 	void *user_params)
 {
+	pdump_ts_to_ns(port_id, pkts, nb_pkts);
 	pdump_copy(pkts, nb_pkts, user_params);
 	return nb_pkts;
 }
-- 
2.17.1

[dpdk-dev] [RFC] [PATCH v1 3/3] net/pcap: dump hardware timestamps

[Patrick Keroulas]

When hardware timestamping is activated, system time should no longer be
used to timestamp dumped the packets. Instead, use value held by
forwarded and assume they were converted to nanoseconds.

Signed-off-by: Patrick Keroulas <patrick.keroulas@radio-canada.ca>
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index b4c79d174..1303e3338 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -45,6 +45,8 @@
 
 #define RTE_PMD_PCAP_MAX_QUEUES 16
 
+#define NSEC_PER_SEC	1000000000L
+
 static char errbuf[PCAP_ERRBUF_SIZE];
 static struct timeval start_time;
 static uint64_t start_cycles;
@@ -288,14 +290,24 @@ eth_null_rx(void *queue __rte_unused,
 }
 
 static inline void
-calculate_timestamp(struct timeval *ts) {
-	uint64_t cycles;
-	struct timeval cur_time;
-
-	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);
+calculate_timestamp(const struct rte_mbuf *mbuf, struct timeval *ts) {
+	if (mbuf->ol_flags & PKT_RX_TIMESTAMP) {
+		/* timestamp unit is nanoseconds but must fit in timeval */
+		ts->tv_sec = mbuf->timestamp / NSEC_PER_SEC;
+		ts->tv_usec = mbuf->timestamp % NSEC_PER_SEC;
+	}
+	else {
+		uint64_t cycles = rte_get_timer_cycles() - start_cycles;
+		struct timeval cur_time;
+		cur_time.tv_sec = cycles / hz;
+		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;
+		}
+	}
 }
 
 /*
@@ -331,7 +343,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
@@ -475,7 +487,7 @@ 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.17.1

Re: [dpdk-dev] [RFC] [PATCH v1 3/3] net/pcap: dump hardware timestamps

[Ferruh Yigit]

On 6/9/2020 3:17 PM, Patrick Keroulas wrote:
> When hardware timestamping is activated, system time should no longer be
> used to timestamp dumped the packets. Instead, use value held by
> forwarded and assume they were converted to nanoseconds.
> 
> Signed-off-by: Patrick Keroulas <patrick.keroulas@radio-canada.ca>
> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>

I think better to get the Vivien's patch that enables 'nanosecond timestamp',
with requested change first and later this patch can be on top of it to add
PKT_RX_TIMESTAMP support.

Or you can split the patch into two, as mentioned above, both are OK but I don't
want pcap nanosecond timestamp patch blocked from this hw timestamp patchset
discussions.

Re: [dpdk-dev] [RFC] [PATCH v1 3/3] net/pcap: dump hardware timestamps

[Ferruh Yigit]

On 6/9/2020 3:41 PM, Ferruh Yigit wrote:
> On 6/9/2020 3:17 PM, Patrick Keroulas wrote:
>> When hardware timestamping is activated, system time should no longer be
>> used to timestamp dumped the packets. Instead, use value held by
>> forwarded and assume they were converted to nanoseconds.
>>
>> Signed-off-by: Patrick Keroulas <patrick.keroulas@radio-canada.ca>
>> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
> 
> I think better to get the Vivien's patch that enables 'nanosecond timestamp',
> with requested change first and later this patch can be on top of it to add
> PKT_RX_TIMESTAMP support.
> 
> Or you can split the patch into two, as mentioned above, both are OK but I don't
> want pcap nanosecond timestamp patch blocked from this hw timestamp patchset
> discussions.
> 

Vivien's patch merged [1], can you please rebase this patchset on top of next-net?

[1] https://patches.dpdk.org/patch/71130/

Re: [dpdk-dev] [RFC] [PATCH v1 0/3] pdump HW timestamps for mlx5

[Slava Ovsiienko]

Hi, Patrick.

Not all rdma-core versions provide the 
- mlx5dv_get_clock_info(context, clock_info);
- mlx5dv_ts_to_ns(clock_info, device_timestamp);

With these versions we would get compilation/linkage error.
There should be conditional compilation like it is done with other mlx5dv routines.

With best regards, Slava

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Patrick Keroulas
> Sent: Tuesday, June 9, 2020 17:17
> To: dev@dpdk.org
> Cc: Patrick Keroulas <patrick.keroulas@radio-canada.ca>
> Subject: [dpdk-dev] [RFC] [PATCH v1 0/3] pdump HW timestamps for mlx5
> 
> The intention is to produce a pcap with nanosecond precision when
> timestamp offloading is activated on mlx5 NIC.
> 
> The packets forwarded by testpmd hold the raw counter but a pcap requires
> a time unit. Assuming that the NIC clock is already synced with external
> master clock, this patchset simply integrates the nanosecond converter that
> is already implemented by ibverbs. The conversion is performed is pdump rx
> callback.
> 
> Patrick Keroulas (3):
>   net/mlx5: add timestamp-to-ns converter from libibverbs
>   ethdev: add API to convert raw timestamps to nsec
>   net/pcap: dump hardware timestamps
> 
>  drivers/common/mlx5/mlx5_glue.c          | 16 ++++++++++++
>  drivers/common/mlx5/mlx5_glue.h          |  4 +++
>  drivers/net/mlx5/mlx5.c                  |  1 +
>  drivers/net/mlx5/mlx5.h                  |  1 +
>  drivers/net/mlx5/mlx5_ethdev.c           | 30 ++++++++++++++++++++++
>  drivers/net/pcap/rte_eth_pcap.c          | 32 ++++++++++++++++--------
>  lib/librte_ethdev/rte_ethdev.c           | 12 +++++++++
>  lib/librte_ethdev/rte_ethdev.h           | 17 +++++++++++++
>  lib/librte_ethdev/rte_ethdev_core.h      |  5 ++++
>  lib/librte_ethdev/rte_ethdev_version.map |  2 ++
>  lib/librte_mbuf/rte_mbuf_core.h          |  3 ++-
>  lib/librte_pdump/rte_pdump.c             | 15 ++++++++++-
>  12 files changed, 126 insertions(+), 12 deletions(-)
> 
> --
> 2.17.1