DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v1] net/ice: fix incorrect reading of PHY timestamp
@ 2024-10-25 10:26 Soumyadeep Hore
  2024-10-29 10:37 ` Singh, Aman Deep
  2024-10-30  2:16 ` [PATCH v2] " Soumyadeep Hore
  0 siblings, 2 replies; 6+ messages in thread
From: Soumyadeep Hore @ 2024-10-25 10:26 UTC (permalink / raw)
  To: bruce.richardson, aman.deep.singh; +Cc: dev, shaiq.wani, stable

In ICE PMD, previously the ready bitmap checking before reading
PHY timestamp was not present. This caused incorrect Tx
timestamping.

The ready bitmap checking is enabled and PHY timestamp is read once
the ready bitmap gives positive value.

Fixes: 881169950d80 ("net/ice/base: implement initial PTP support for E830")
Cc: stable@dpdk.org

Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
---
 drivers/net/ice/ice_ethdev.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 70298ac330..efc6db4346 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -6597,10 +6597,26 @@ ice_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
 	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ice_adapter *ad =
 			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-	uint64_t ts_ns, tstamp;
+	uint64_t ts_ns, tstamp, tstamp_ready = 0;
+	uint64_t start_time, curr_time;
 	const uint64_t mask = 0xFFFFFFFF;
 	int ret;
 
+	start_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
+
+	while (!(tstamp_ready & BIT_ULL(0))) {
+		ret = ice_get_phy_tx_tstamp_ready(hw, ad->ptp_tx_block, &tstamp_ready);
+		if (ret) {
+			PMD_DRV_LOG(ERR, "Failed to get phy ready for timestamp");
+			return -1;
+		}
+		curr_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
+		if (curr_time - start_time > 1000) {
+			PMD_DRV_LOG(ERR, "Timeout to get phy ready for timestamp");
+			return -1;
+		}
+	}
+
 	ret = ice_read_phy_tstamp(hw, ad->ptp_tx_block, ad->ptp_tx_index, &tstamp);
 	if (ret || tstamp == 0) {
 		PMD_DRV_LOG(ERR, "Failed to read phy timestamp");
-- 
2.43.0


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

* Re: [PATCH v1] net/ice: fix incorrect reading of PHY timestamp
  2024-10-25 10:26 [PATCH v1] net/ice: fix incorrect reading of PHY timestamp Soumyadeep Hore
@ 2024-10-29 10:37 ` Singh, Aman Deep
  2024-10-29 17:09   ` Bruce Richardson
  2024-10-30  2:16 ` [PATCH v2] " Soumyadeep Hore
  1 sibling, 1 reply; 6+ messages in thread
From: Singh, Aman Deep @ 2024-10-29 10:37 UTC (permalink / raw)
  To: Soumyadeep Hore, bruce.richardson; +Cc: dev, shaiq.wani, stable

[-- Attachment #1: Type: text/plain, Size: 1907 bytes --]


On 25-10-2024 15:56, Soumyadeep Hore wrote:
> In ICE PMD, previously the ready bitmap checking before reading
> PHY timestamp was not present. This caused incorrect Tx
> timestamping.
>
> The ready bitmap checking is enabled and PHY timestamp is read once
> the ready bitmap gives positive value.
>
> Fixes: 881169950d80 ("net/ice/base: implement initial PTP support for E830")
> Cc:stable@dpdk.org
>
> Signed-off-by: Soumyadeep Hore<soumyadeep.hore@intel.com>

Acked-by: Aman Singh<aman.deep.singh@intel.com>

> ---
>   drivers/net/ice/ice_ethdev.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index 70298ac330..efc6db4346 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -6597,10 +6597,26 @@ ice_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
>   	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>   	struct ice_adapter *ad =
>   			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> -	uint64_t ts_ns, tstamp;
> +	uint64_t ts_ns, tstamp, tstamp_ready = 0;
> +	uint64_t start_time, curr_time;
>   	const uint64_t mask = 0xFFFFFFFF;
>   	int ret;
>   
> +	start_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
> +
> +	while (!(tstamp_ready & BIT_ULL(0))) {
> +		ret = ice_get_phy_tx_tstamp_ready(hw, ad->ptp_tx_block, &tstamp_ready);
> +		if (ret) {
> +			PMD_DRV_LOG(ERR, "Failed to get phy ready for timestamp");
> +			return -1;
> +		}
> +		curr_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
> +		if (curr_time - start_time > 1000) {
> +			PMD_DRV_LOG(ERR, "Timeout to get phy ready for timestamp");
> +			return -1;
> +		}
> +	}
> +
>   	ret = ice_read_phy_tstamp(hw, ad->ptp_tx_block, ad->ptp_tx_index, &tstamp);
>   	if (ret || tstamp == 0) {
>   		PMD_DRV_LOG(ERR, "Failed to read phy timestamp");

[-- Attachment #2: Type: text/html, Size: 2659 bytes --]

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

* Re: [PATCH v1] net/ice: fix incorrect reading of PHY timestamp
  2024-10-29 10:37 ` Singh, Aman Deep
@ 2024-10-29 17:09   ` Bruce Richardson
  0 siblings, 0 replies; 6+ messages in thread
From: Bruce Richardson @ 2024-10-29 17:09 UTC (permalink / raw)
  To: Singh, Aman Deep; +Cc: Soumyadeep Hore, dev, shaiq.wani, stable

On Tue, Oct 29, 2024 at 04:07:45PM +0530, Singh, Aman Deep wrote:
>    On 25-10-2024 15:56, Soumyadeep Hore wrote:
> 
> In ICE PMD, previously the ready bitmap checking before reading
> PHY timestamp was not present. This caused incorrect Tx
> timestamping.
> 
> The ready bitmap checking is enabled and PHY timestamp is read once
> the ready bitmap gives positive value.
> 
> Fixes: 881169950d80 ("net/ice/base: implement initial PTP support for E830")
> Cc: [1]stable@dpdk.org
> 
> Signed-off-by: Soumyadeep Hore [2]<soumyadeep.hore@intel.com>
> 
> Acked-by: Aman Singh [3]<aman.deep.singh@intel.com>
> 
Is this a v5 of this patch, or a resubmission of v1? There are 3 different
v1's of this patch, plus a v2, v3 and v4 listed in patchwork, see [1].
I had feedback on v3 of this patch, which I believe is still relevant to
this version.

Please keep all revisions of a patch in the one thread and with increasing
version number, saves us reviewers getting confused! :-) 
Also, with each new version, please include a summary below the cutline of
what has changed from previous, so we know what to look out for when
reviewing.

/Bruce

[1] https://patches.dpdk.org/project/dpdk/list/?series=&submitter=&state=*&q=fix+incorrect+reading+of+PHY&archive=&delegate=

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

* [PATCH v2] net/ice: fix incorrect reading of PHY timestamp
  2024-10-25 10:26 [PATCH v1] net/ice: fix incorrect reading of PHY timestamp Soumyadeep Hore
  2024-10-29 10:37 ` Singh, Aman Deep
@ 2024-10-30  2:16 ` Soumyadeep Hore
  2024-10-30 15:31   ` Bruce Richardson
  1 sibling, 1 reply; 6+ messages in thread
From: Soumyadeep Hore @ 2024-10-30  2:16 UTC (permalink / raw)
  To: bruce.richardson, aman.deep.singh; +Cc: dev, shaiq.wani, stable

In ICE PMD, previously the ready bitmap checking before reading
PHY timestamp was not present. This caused incorrect Tx
timestamping.

The ready bitmap checking is enabled and PHY timestamp is read once
the ready bitmap gives positive value.

Fixes: 881169950d80 ("net/ice/base: implement initial PTP support for E830")
Cc: stable@dpdk.org

Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
---
v2:
- Addressed Bruce's comments
---
 drivers/net/ice/ice_ethdev.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 70298ac330..6d0d37b3a0 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -6597,10 +6597,26 @@ ice_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
 	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ice_adapter *ad =
 			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-	uint64_t ts_ns, tstamp;
+	uint64_t ts_ns, tstamp, tstamp_ready = 0;
+	uint64_t end_time;
 	const uint64_t mask = 0xFFFFFFFF;
 	int ret;
 
+	end_time = rte_get_timer_cycles() + rte_get_timer_hz();
+
+	while (!(tstamp_ready & BIT_ULL(0))) {
+		ret = ice_get_phy_tx_tstamp_ready(hw, ad->ptp_tx_block, &tstamp_ready);
+		if (ret) {
+			PMD_DRV_LOG(ERR, "Failed to get phy ready for timestamp");
+			return -1;
+		}
+
+		if (rte_get_timer_cycles() > end_time) {
+			PMD_DRV_LOG(ERR, "Timeout to get phy ready for timestamp");
+			return -1;
+		}
+	}
+
 	ret = ice_read_phy_tstamp(hw, ad->ptp_tx_block, ad->ptp_tx_index, &tstamp);
 	if (ret || tstamp == 0) {
 		PMD_DRV_LOG(ERR, "Failed to read phy timestamp");
-- 
2.43.0


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

* Re: [PATCH v2] net/ice: fix incorrect reading of PHY timestamp
  2024-10-30  2:16 ` [PATCH v2] " Soumyadeep Hore
@ 2024-10-30 15:31   ` Bruce Richardson
  0 siblings, 0 replies; 6+ messages in thread
From: Bruce Richardson @ 2024-10-30 15:31 UTC (permalink / raw)
  To: Soumyadeep Hore; +Cc: aman.deep.singh, dev, shaiq.wani, stable

On Wed, Oct 30, 2024 at 02:16:11AM +0000, Soumyadeep Hore wrote:
> In ICE PMD, previously the ready bitmap checking before reading
> PHY timestamp was not present. This caused incorrect Tx
> timestamping.
> 
> The ready bitmap checking is enabled and PHY timestamp is read once
> the ready bitmap gives positive value.
> 
> Fixes: 881169950d80 ("net/ice/base: implement initial PTP support for E830")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
> ---
> v2:
> - Addressed Bruce's comments
> ---
>  drivers/net/ice/ice_ethdev.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index 70298ac330..6d0d37b3a0 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -6597,10 +6597,26 @@ ice_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
>  	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	struct ice_adapter *ad =
>  			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> -	uint64_t ts_ns, tstamp;
> +	uint64_t ts_ns, tstamp, tstamp_ready = 0;
> +	uint64_t end_time;
>  	const uint64_t mask = 0xFFFFFFFF;
>  	int ret;
>  
> +	end_time = rte_get_timer_cycles() + rte_get_timer_hz();
> +

Patch code itself looks fine, but don't you think that 1sec is an
excessively long time to wait? Maybe do a V3 with a lower timeout
threshold.

/Bruce

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

* [PATCH v2] net/ice: fix incorrect reading of PHY timestamp
  2024-10-11 13:00 [PATCH v1] " Soumyadeep Hore
@ 2024-10-25  5:14 ` Soumyadeep Hore
  0 siblings, 0 replies; 6+ messages in thread
From: Soumyadeep Hore @ 2024-10-25  5:14 UTC (permalink / raw)
  To: bruce.richardson, aman.deep.singh; +Cc: dev, shaiq.wani, stable

In ICE PMD, previously the ready bitmap checking before reading
PHY timestamp was not present. This caused incorrect Tx
timestamping.

The ready bitmap checking is enabled and PHY timestamp is read once
the ready bitmap gives positive value.

Fixes: 881169950d80 ("net/ice/base: implement initial PTP support for E830")
Cc: stable@dpdk.org

Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
---
 drivers/net/ice/ice_ethdev.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 7b1bd163a2..e0db47cf28 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -6517,12 +6517,28 @@ ice_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
 	struct ice_adapter *ad =
 			ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	uint8_t lport;
-	uint64_t ts_ns, ns, tstamp;
+	uint64_t ts_ns, ns, tstamp, tstamp_ready = 0;
+	uint64_t start_time, curr_time;
 	const uint64_t mask = 0xFFFFFFFF;
 	int ret;
 
 	lport = hw->port_info->lport;
 
+	start_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
+
+	while (!(tstamp_ready & BIT_ULL(0))) {
+		ret = ice_get_phy_tx_tstamp_ready(hw, lport, &tstamp_ready);
+		if (ret) {
+			PMD_DRV_LOG(ERR, "Failed to get phy ready for timestamp");
+			return -1;
+		}
+		curr_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
+		if (curr_time - start_time > 1000) {
+			PMD_DRV_LOG(ERR, "Timeout to get phy ready for timestamp");
+			return -1;
+		}
+	}
+
 	ret = ice_read_phy_tstamp(hw, lport, 0, &tstamp);
 	if (ret) {
 		PMD_DRV_LOG(ERR, "Failed to read phy timestamp");
-- 
2.43.0


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

end of thread, other threads:[~2024-10-30 15:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-25 10:26 [PATCH v1] net/ice: fix incorrect reading of PHY timestamp Soumyadeep Hore
2024-10-29 10:37 ` Singh, Aman Deep
2024-10-29 17:09   ` Bruce Richardson
2024-10-30  2:16 ` [PATCH v2] " Soumyadeep Hore
2024-10-30 15:31   ` Bruce Richardson
  -- strict thread matches above, loose matches on Subject: below --
2024-10-11 13:00 [PATCH v1] " Soumyadeep Hore
2024-10-25  5:14 ` [PATCH v2] " Soumyadeep Hore

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