From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 40BC51B5C7 for ; Thu, 25 Apr 2019 20:28:56 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Apr 2019 11:28:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,394,1549958400"; d="scan'208";a="340783082" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga005.fm.intel.com with ESMTP; 25 Apr 2019 11:28:55 -0700 Received: from fmsmsx155.amr.corp.intel.com (10.18.116.71) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 25 Apr 2019 11:28:55 -0700 Received: from fmsmsx117.amr.corp.intel.com ([169.254.3.26]) by FMSMSX155.amr.corp.intel.com ([169.254.5.71]) with mapi id 14.03.0415.000; Thu, 25 Apr 2019 11:28:54 -0700 From: "Wiles, Keith" To: Tom Barbette CC: dpdk-dev , "Richardson, Bruce" , "Mcnamara, John" , Thomas Monjalon , "Yigit, Ferruh" , "Andrew Rybchenko" , Shahaf Shuler , Yongseok Koh , "olivier.matz@6wind.com" Thread-Topic: [dpdk-dev] [PATCH v3 1/3] rte_ethdev: Add API function to read dev clock Thread-Index: AQHU+sQrJitmFrYrbka1XsgUTb3geaZNqUkA Date: Thu, 25 Apr 2019 18:28:54 +0000 Message-ID: References: <20190424173424.34628-1-barbette@kth.se> <20190424173424.34628-2-barbette@kth.se> In-Reply-To: <20190424173424.34628-2-barbette@kth.se> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.188.160] Content-Type: text/plain; charset="us-ascii" Content-ID: <07606285D37C4E4AAE55783D75E1D6D2@intel.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 1/3] rte_ethdev: Add API function to read dev clock X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Apr 2019 18:28:56 -0000 > On Apr 24, 2019, at 12:34 PM, Tom Barbette wrote: >=20 > Add rte_eth_read_clock to read the raw clock of a devide. >=20 > The main use is to get the device clock conversion co-efficients to be > able to translate the raw clock of the timestamp field of the pkt mbuf > to a local synced time value. >=20 > This function was missing to allow users to convert the RX timestamp fiel= d > to real time without the complexity of the rte_timesync* facility. One ca= n > derivate the clock frequency by calling twice read_clock and then keep a > common time base. >=20 > Signed-off-by: Tom Barbette > --- > doc/guides/nics/features.rst | 1 + > lib/librte_ethdev/rte_ethdev.c | 13 +++++++ > lib/librte_ethdev/rte_ethdev.h | 45 ++++++++++++++++++++++++ > lib/librte_ethdev/rte_ethdev_core.h | 6 ++++ > lib/librte_ethdev/rte_ethdev_version.map | 2 ++ > lib/librte_mbuf/rte_mbuf.h | 2 ++ > 6 files changed, 69 insertions(+) >=20 > diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst > index c5bf32222..025b7f812 100644 > --- a/doc/guides/nics/features.rst > +++ b/doc/guides/nics/features.rst > @@ -602,6 +602,7 @@ Supports Timestamp. > * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_TIMESTAMP``. > * **[provides] mbuf**: ``mbuf.timestamp``. > * **[provides] rte_eth_dev_info**: ``rx_offload_capa,rx_queue_offload_cap= a: DEV_RX_OFFLOAD_TIMESTAMP``. > +* **[related] eth_dev_ops**: ``read_clock``. >=20 > .. _nic_features_macsec_offload: >=20 > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethde= v.c > index d7cfa3d53..22e35d839 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -4170,6 +4170,19 @@ rte_eth_timesync_write_time(uint16_t port_id, cons= t struct timespec *timestamp) > timestamp)); > } >=20 > +int > +rte_eth_read_clock(uint16_t port_id, uint64_t *timestamp) > +{ > + struct rte_eth_dev *dev; > + > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > + dev =3D &rte_eth_devices[port_id]; > + > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->read_clock, -ENOTSUP); > + return eth_err(port_id, (*dev->dev_ops->read_clock)(dev, > + timestamp)); > +} > + > int > rte_eth_dev_get_reg_info(uint16_t port_id, struct rte_dev_reg_info *info) > { > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethde= v.h > index b8d19c69f..dfaf16a56 100644 > --- a/lib/librte_ethdev/rte_ethdev.h > +++ b/lib/librte_ethdev/rte_ethdev.h > @@ -3793,6 +3793,51 @@ int rte_eth_timesync_read_time(uint16_t port_id, s= truct timespec *time); > */ > int rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *= time); >=20 > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Read the current clock counter of an Ethernet device > + * > + * This returns the current raw clock value of an Ethernet device. > + * The value returned here is from the same clock than the one > + * filling timestamp field of RX packets. Therefore it can be used > + * to compute a precise conversion of the device clock to the real time. > + * > + * E.g, a simple heuristic to derivate the frequency would be: > + * uint64_t start, end; > + * rte_eth_read_clock(port, start); > + * rte_delay_ms(100); > + * rte_eth_read_clock(port, end); > + * double freq =3D (end - start) * 10; > + * > + * Compute a common reference with: > + * uint64_t base_time_sec =3D current_time(); > + * uint64_t base_clock; > + * rte_eth_read_clock(port, base_clock); > + * > + * Then, convert the raw mbuf timestamp with: > + * base_time_sec + (double)(mbuf->timestamp - base_clock) / freq; > + * > + * This simple example will not provide a very good accuracy. One must > + * at least measure multiple times the frequency and do a regression. > + * To avoid deviation from the system time, the common reference can > + * be repeated from time to time. The integer division can also be > + * converted by a multiplication and a shift for better performance. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param time > + * Pointer to the uint64_t that holds the raw clock value. What is a raw clock value? It took me a bit to find that it is in nano-seco= nds need to document that point. > + * > + * @return > + * - 0: Success. > + * - -ENODEV: The port ID is invalid. > + * - -ENOTSUP: The function is not supported by the Ethernet driver. > + */ > +int __rte_experimental > +rte_eth_read_clock(uint16_t port_id, uint64_t *timestamp); Using timestamp variable is not very descriptive and some other name needs = to be used. Also in the driver it is called *clock. Another question is why does this routine not perform the looping/delaying = and calling read_clock and then return frequency instead. If you want a tim= estamp reading function then this one is not being described that way and I= would think it should be done someplace else or should be. > + > /** > * Config l2 tunnel ether type of an Ethernet device for filtering specif= ic > * tunnel packets by ether type. > diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_= ethdev_core.h > index 8f03f83f6..86806b3eb 100644 > --- a/lib/librte_ethdev/rte_ethdev_core.h > +++ b/lib/librte_ethdev/rte_ethdev_core.h > @@ -322,6 +322,10 @@ typedef int (*eth_timesync_write_time)(struct rte_et= h_dev *dev, > const struct timespec *timestamp); > /**< @internal Function used to get time from the device clock */ >=20 > +typedef int (*eth_read_clock)(struct rte_eth_dev *dev, > + uint64_t *timestamp); > +/**< @internal Function used to get the current value of the device cloc= k. */ > + > typedef int (*eth_get_reg_t)(struct rte_eth_dev *dev, > struct rte_dev_reg_info *info); > /**< @internal Retrieve registers */ > @@ -496,6 +500,8 @@ 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. */ >=20 > + eth_read_clock read_clock; > + > eth_xstats_get_by_id_t xstats_get_by_id; > /**< Get extended device statistic values by ID. */ > eth_xstats_get_names_by_id_t xstats_get_names_by_id; > diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev= /rte_ethdev_version.map > index afcd25599..670e4fad5 100644 > --- a/lib/librte_ethdev/rte_ethdev_version.map > +++ b/lib/librte_ethdev/rte_ethdev_version.map > @@ -253,10 +253,12 @@ EXPERIMENTAL { > rte_eth_dev_rx_intr_ctl_q_get_fd; > rte_eth_find_next_of; > rte_eth_find_next_sibling; > + rte_eth_read_clock; > rte_eth_switch_domain_alloc; > rte_eth_switch_domain_free; > rte_flow_conv; > rte_flow_expand_rss; > + rte_eth_read_clock; > rte_mtr_capabilities_get; > rte_mtr_create; > rte_mtr_destroy; > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index 68415af02..e530a96c5 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -668,6 +668,8 @@ struct rte_mbuf { >=20 > /** 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. The message here is not a good place for this detail, I would put it in the= function call doc above. > */ > uint64_t timestamp; >=20 > --=20 > 2.17.1 >=20 Regards, Keith From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 1AE6FA05D3 for ; Thu, 25 Apr 2019 20:28:58 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C541A1B5D1; Thu, 25 Apr 2019 20:28:57 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 40BC51B5C7 for ; Thu, 25 Apr 2019 20:28:56 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Apr 2019 11:28:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,394,1549958400"; d="scan'208";a="340783082" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga005.fm.intel.com with ESMTP; 25 Apr 2019 11:28:55 -0700 Received: from fmsmsx155.amr.corp.intel.com (10.18.116.71) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 25 Apr 2019 11:28:55 -0700 Received: from fmsmsx117.amr.corp.intel.com ([169.254.3.26]) by FMSMSX155.amr.corp.intel.com ([169.254.5.71]) with mapi id 14.03.0415.000; Thu, 25 Apr 2019 11:28:54 -0700 From: "Wiles, Keith" To: Tom Barbette CC: dpdk-dev , "Richardson, Bruce" , "Mcnamara, John" , Thomas Monjalon , "Yigit, Ferruh" , "Andrew Rybchenko" , Shahaf Shuler , Yongseok Koh , "olivier.matz@6wind.com" Thread-Topic: [dpdk-dev] [PATCH v3 1/3] rte_ethdev: Add API function to read dev clock Thread-Index: AQHU+sQrJitmFrYrbka1XsgUTb3geaZNqUkA Date: Thu, 25 Apr 2019 18:28:54 +0000 Message-ID: References: <20190424173424.34628-1-barbette@kth.se> <20190424173424.34628-2-barbette@kth.se> In-Reply-To: <20190424173424.34628-2-barbette@kth.se> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.188.160] Content-Type: text/plain; charset="UTF-8" Content-ID: <07606285D37C4E4AAE55783D75E1D6D2@intel.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 1/3] rte_ethdev: Add API function to read dev clock X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190425182854.IqT33fMXHLz7RjMcfSbxiDHxaPWZDOnjC-j8yfwmDZk@z> > On Apr 24, 2019, at 12:34 PM, Tom Barbette wrote: >=20 > Add rte_eth_read_clock to read the raw clock of a devide. >=20 > The main use is to get the device clock conversion co-efficients to be > able to translate the raw clock of the timestamp field of the pkt mbuf > to a local synced time value. >=20 > This function was missing to allow users to convert the RX timestamp fiel= d > to real time without the complexity of the rte_timesync* facility. One ca= n > derivate the clock frequency by calling twice read_clock and then keep a > common time base. >=20 > Signed-off-by: Tom Barbette > --- > doc/guides/nics/features.rst | 1 + > lib/librte_ethdev/rte_ethdev.c | 13 +++++++ > lib/librte_ethdev/rte_ethdev.h | 45 ++++++++++++++++++++++++ > lib/librte_ethdev/rte_ethdev_core.h | 6 ++++ > lib/librte_ethdev/rte_ethdev_version.map | 2 ++ > lib/librte_mbuf/rte_mbuf.h | 2 ++ > 6 files changed, 69 insertions(+) >=20 > diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst > index c5bf32222..025b7f812 100644 > --- a/doc/guides/nics/features.rst > +++ b/doc/guides/nics/features.rst > @@ -602,6 +602,7 @@ Supports Timestamp. > * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_TIMESTAMP``. > * **[provides] mbuf**: ``mbuf.timestamp``. > * **[provides] rte_eth_dev_info**: ``rx_offload_capa,rx_queue_offload_cap= a: DEV_RX_OFFLOAD_TIMESTAMP``. > +* **[related] eth_dev_ops**: ``read_clock``. >=20 > .. _nic_features_macsec_offload: >=20 > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethde= v.c > index d7cfa3d53..22e35d839 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -4170,6 +4170,19 @@ rte_eth_timesync_write_time(uint16_t port_id, cons= t struct timespec *timestamp) > timestamp)); > } >=20 > +int > +rte_eth_read_clock(uint16_t port_id, uint64_t *timestamp) > +{ > + struct rte_eth_dev *dev; > + > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > + dev =3D &rte_eth_devices[port_id]; > + > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->read_clock, -ENOTSUP); > + return eth_err(port_id, (*dev->dev_ops->read_clock)(dev, > + timestamp)); > +} > + > int > rte_eth_dev_get_reg_info(uint16_t port_id, struct rte_dev_reg_info *info) > { > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethde= v.h > index b8d19c69f..dfaf16a56 100644 > --- a/lib/librte_ethdev/rte_ethdev.h > +++ b/lib/librte_ethdev/rte_ethdev.h > @@ -3793,6 +3793,51 @@ int rte_eth_timesync_read_time(uint16_t port_id, s= truct timespec *time); > */ > int rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *= time); >=20 > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Read the current clock counter of an Ethernet device > + * > + * This returns the current raw clock value of an Ethernet device. > + * The value returned here is from the same clock than the one > + * filling timestamp field of RX packets. Therefore it can be used > + * to compute a precise conversion of the device clock to the real time. > + * > + * E.g, a simple heuristic to derivate the frequency would be: > + * uint64_t start, end; > + * rte_eth_read_clock(port, start); > + * rte_delay_ms(100); > + * rte_eth_read_clock(port, end); > + * double freq =3D (end - start) * 10; > + * > + * Compute a common reference with: > + * uint64_t base_time_sec =3D current_time(); > + * uint64_t base_clock; > + * rte_eth_read_clock(port, base_clock); > + * > + * Then, convert the raw mbuf timestamp with: > + * base_time_sec + (double)(mbuf->timestamp - base_clock) / freq; > + * > + * This simple example will not provide a very good accuracy. One must > + * at least measure multiple times the frequency and do a regression. > + * To avoid deviation from the system time, the common reference can > + * be repeated from time to time. The integer division can also be > + * converted by a multiplication and a shift for better performance. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param time > + * Pointer to the uint64_t that holds the raw clock value. What is a raw clock value? It took me a bit to find that it is in nano-seco= nds need to document that point. > + * > + * @return > + * - 0: Success. > + * - -ENODEV: The port ID is invalid. > + * - -ENOTSUP: The function is not supported by the Ethernet driver. > + */ > +int __rte_experimental > +rte_eth_read_clock(uint16_t port_id, uint64_t *timestamp); Using timestamp variable is not very descriptive and some other name needs = to be used. Also in the driver it is called *clock. Another question is why does this routine not perform the looping/delaying = and calling read_clock and then return frequency instead. If you want a tim= estamp reading function then this one is not being described that way and I= would think it should be done someplace else or should be. > + > /** > * Config l2 tunnel ether type of an Ethernet device for filtering specif= ic > * tunnel packets by ether type. > diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_= ethdev_core.h > index 8f03f83f6..86806b3eb 100644 > --- a/lib/librte_ethdev/rte_ethdev_core.h > +++ b/lib/librte_ethdev/rte_ethdev_core.h > @@ -322,6 +322,10 @@ typedef int (*eth_timesync_write_time)(struct rte_et= h_dev *dev, > const struct timespec *timestamp); > /**< @internal Function used to get time from the device clock */ >=20 > +typedef int (*eth_read_clock)(struct rte_eth_dev *dev, > + uint64_t *timestamp); > +/**< @internal Function used to get the current value of the device cloc= k. */ > + > typedef int (*eth_get_reg_t)(struct rte_eth_dev *dev, > struct rte_dev_reg_info *info); > /**< @internal Retrieve registers */ > @@ -496,6 +500,8 @@ 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. */ >=20 > + eth_read_clock read_clock; > + > eth_xstats_get_by_id_t xstats_get_by_id; > /**< Get extended device statistic values by ID. */ > eth_xstats_get_names_by_id_t xstats_get_names_by_id; > diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev= /rte_ethdev_version.map > index afcd25599..670e4fad5 100644 > --- a/lib/librte_ethdev/rte_ethdev_version.map > +++ b/lib/librte_ethdev/rte_ethdev_version.map > @@ -253,10 +253,12 @@ EXPERIMENTAL { > rte_eth_dev_rx_intr_ctl_q_get_fd; > rte_eth_find_next_of; > rte_eth_find_next_sibling; > + rte_eth_read_clock; > rte_eth_switch_domain_alloc; > rte_eth_switch_domain_free; > rte_flow_conv; > rte_flow_expand_rss; > + rte_eth_read_clock; > rte_mtr_capabilities_get; > rte_mtr_create; > rte_mtr_destroy; > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index 68415af02..e530a96c5 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -668,6 +668,8 @@ struct rte_mbuf { >=20 > /** 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. The message here is not a good place for this detail, I would put it in the= function call doc above. > */ > uint64_t timestamp; >=20 > --=20 > 2.17.1 >=20 Regards, Keith