DPDK patches and discussions
 help / color / mirror / Atom feed
From: Shahaf Shuler <shahafs@mellanox.com>
To: Tom Barbette <barbette@kth.se>, "dev@dpdk.org" <dev@dpdk.org>,
	Alex Rosenbaum <alexr@mellanox.com>
Cc: Yongseok Koh <yskoh@mellanox.com>,
	"john.mcnamara@intel.com" <john.mcnamara@intel.com>,
	"marko.kovacevic@intel.com" <marko.kovacevic@intel.com>
Subject: Re: [dpdk-dev] MLX5 should define the timestamp field in the doc
Date: Thu, 6 Sep 2018 09:07:38 +0000	[thread overview]
Message-ID: <DB7PR05MB44262E749DE1BAF56611895FC3010@DB7PR05MB4426.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <1536138003083.99523@kth.se>

Wednesday, September 5, 2018 12:00 PM, Tom Barbette:
>Actually I managed this patch to implement support for rte_eth_timesync_read_time.

I am not fully familiar w/ this API, but it looks like the timespec returned from this call is expected to be in real time values (i.e. seconds and nano seconds),
at least this is what I see on the ptpclient example on the DPDK tree.

>
>Please tell me potential modifications, and if I shall submit it again as a "normal" patch to dev ?
>
>---

[...]

> }
>
> /**
>+ * Get device current time
>+ *
>+ * @param dev
>+ *   Pointer to Ethernet device structure.
>+ *
>+ * @param[out] time
>+ *   Time output value.
>+ *
>+ * @return
>+ *   0 if the time has correctly been set
>+ */
>+int
>+mlx5_timesync_read_time(struct rte_eth_dev *dev, struct timespec *time)
>+{
>+    struct priv *priv = dev->data->dev_private;
>+    struct ibv_values_ex values;
>+    int err = 0;
>+
>+    values.comp_mask = IBV_VALUES_MASK_RAW_CLOCK;
>+    if ((err = mlx5_glue->query_rt_values_ex(priv->ctx, &values)) != 0) {

The use of this function will not bring you the outcome the API defines.
see the man page of ibv_query_rt_values_ex:
struct ibv_values_ex {
        uint32_t             comp_mask;    /* Compatibility mask that defines the query/queried fields [in/out]

        struct timespec      raw_clock;    /* HW raw clock */
};

enum ibv_values_mask {
        IBV_VALUES_MASK_RAW_CLOCK = 1 << 0, /* HW raw clock */
};

The output is the HW raw clock (just like you have in the mbuf).

In order it to work the application needs to understand the PTP coefficients for the raw->real time conversion. this can be done, just need some more work.
do you have a ptp daemon implemented to calc the coefficients?

>+ DRV_LOG(WARNING, "Could not query time !");
>+        return err;
>+    }
>+
>+    *time = values.raw_clock;
>+    return 0;
>+}
>+
>+
>+/**
>  * Get supported packet types.
>  *
>  * @param dev
>diff --git a/drivers/net/mlx5/mlx5_glue.c b/drivers/net/mlx5/mlx5_glue.c
>index c7965e5..3c72f5b 100644
>--- a/drivers/net/mlx5/mlx5_glue.c
>+++ b/drivers/net/mlx5/mlx5_glue.c
>@@ -84,6 +84,13 @@ mlx5_glue_query_device_ex(struct ibv_context *context,
> }
>
> static int
>+mlx5_glue_query_rt_values_ex(struct ibv_context *context,
>+   struct ibv_values_ex* values)
>+{
>+ return ibv_query_rt_values_ex(context, values);
>+}
>+
>+static int
> mlx5_glue_query_port(struct ibv_context *context, uint8_t port_num,
>       struct ibv_port_attr *port_attr)
> {
>@@ -354,6 +361,7 @@ const struct mlx5_glue *mlx5_glue = &(const struct mlx5_glue){
>  .close_device = mlx5_glue_close_device,
>  .query_device = mlx5_glue_query_device,
>  .query_device_ex = mlx5_glue_query_device_ex,
>+ .query_rt_values_ex = mlx5_glue_query_rt_values_ex,
>  .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/net/mlx5/mlx5_glue.h b/drivers/net/mlx5/mlx5_glue.h
>index e584d36..0582e95 100644
>--- a/drivers/net/mlx5/mlx5_glue.h
>+++ b/drivers/net/mlx5/mlx5_glue.h
>@@ -54,6 +54,8 @@ struct mlx5_glue {
>  int (*query_device_ex)(struct ibv_context *context,
>         const struct ibv_query_device_ex_input *input,
>         struct ibv_device_attr_ex *attr);
>+ int (*query_rt_values_ex)(struct ibv_context *context,
>+        struct ibv_values_ex *values);
>  int (*query_port)(struct ibv_context *context, uint8_t port_num,
>    struct ibv_port_attr *port_attr);
>  struct ibv_comp_channel *(*create_comp_channel)
>--
>2.7.4
>
>
>________________________________________
>De : Shahaf Shuler <shahafs@mellanox.com>
>Envoyé : mercredi 5 septembre 2018 10:18
>À : Tom Barbette; dev@dpdk.org; Alex Rosenbaum
>Cc : Yongseok Koh; john.mcnamara@intel.com; marko.kovacevic@intel.com
>Objet : RE: MLX5 should define the timestamp field in the doc
>
>Thanks for the details.
>
>The use case is clear. We will take it internally to see when we can support it.
>AFAIK we cannot read the internal time from userspace.
>
>Adding also AlexR to comment
>
>From: Tom Barbette <barbette@kth.se>
>Sent: Wednesday, September 5, 2018 10:11 AM
>To: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
>Cc: Yongseok Koh <yskoh@mellanox.com>; john.mcnamara@intel.com; marko.kovacevic@intel.com
>Subject: RE: MLX5 should define the timestamp field in the doc
>
>Thanks for your answer Shahaf !
>
>We're trying to measure the latency of packets going through various service chains inside individual "server".  Eg. we can see that on Server 1, the latency for the service chain handling HTTP packets is ~800ns (+ max and mins, tail latency, etc). What we do now is to timestamp packets right after they are received, and compute the difference with the timestamp just before they are sent. Over a cluster this shows us where the latency is happening.
>
>We would like this "box" latency to include the time spent in queues, and for that the hardware timestamp seems fit-for-purpose as it would timestamp the packets before the software queues. Moreover, as we use batching, we lose a lot of precision as we timestamp a whole batch at once.
>
>I'm pretty sure this use case is of interest for many others. Tail latency is of the essence nowadays, and finding where packets get delayed precisely is important.
>
>Instead of converting the timestamp to real time, in this very use case it seems the Mellanox card could actually be our unique source of time, we just need to be able to convert ticks to seconds.
>
>Any chance we can run an equivalent of mlx5_read_internal_timer (https://elixir.bootlin.com/linux/v4.18.5/source/drivers/net/ethernet/mellanox/mlx5/core/main.c#L623) from userspace ? Are these registers also mapped, or can be done so with a few changes? With only that we can actually derive the frequency and the offset easily.
>
>Tom



  reply	other threads:[~2018-09-06  9:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-04 14:26 Tom Barbette
2018-09-05  5:29 ` Shahaf Shuler
2018-09-05  7:11   ` Tom Barbette
2018-09-05  8:18     ` Shahaf Shuler
2018-09-05  9:00       ` Tom Barbette
2018-09-06  9:07         ` Shahaf Shuler [this message]
2018-09-06  9:33           ` Tom Barbette
2018-09-06 14:21             ` Shahaf Shuler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DB7PR05MB44262E749DE1BAF56611895FC3010@DB7PR05MB4426.eurprd05.prod.outlook.com \
    --to=shahafs@mellanox.com \
    --cc=alexr@mellanox.com \
    --cc=barbette@kth.se \
    --cc=dev@dpdk.org \
    --cc=john.mcnamara@intel.com \
    --cc=marko.kovacevic@intel.com \
    --cc=yskoh@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).