DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Min Hu (Connor)" <humin29@huawei.com>
To: <dev@dpdk.org>
Cc: <ferruh.yigit@intel.com>
Subject: [dpdk-dev] [PATCH 3/5] net/hns3: fix use wrong time API
Date: Wed, 28 Apr 2021 15:20:53 +0800	[thread overview]
Message-ID: <1619594455-56787-4-git-send-email-humin29@huawei.com> (raw)
In-Reply-To: <1619594455-56787-1-git-send-email-humin29@huawei.com>

From: Chengwen Feng <fengchengwen@huawei.com>

Currently, driver uses gettimeofday() API to get the time, and
then calculate the time delta, the delta will be used mainly in
judging timeout process.

But the time which gets from gettimeofday() API isn't monotonically
increasing. The process may fail if the system time is changed.

We use the following scheme to fix it:
1. Add hns3_clock_gettime() API which will get the monotonically
increasing time.
2. Add hns3_clock_calctime_ms() API which will get the milliseconds of
the monotonically increasing time.
3. Add hns3_clock_calctime_ms() API which will calc the milliseconds
of a given time.

Fixes: 2790c6464725 ("net/hns3: support device reset")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    | 46 +++++++++++++++++++++++++++++++++------
 drivers/net/hns3/hns3_ethdev.h    | 12 +++-------
 drivers/net/hns3/hns3_ethdev_vf.c | 11 +++++-----
 drivers/net/hns3/hns3_intr.c      | 34 ++++++++++++++---------------
 4 files changed, 63 insertions(+), 40 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index ad000a1..021e33c 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -6334,7 +6334,7 @@ hns3_wait_hardware_ready(struct hns3_adapter *hns)
 	if (wait_data->result == HNS3_WAIT_SUCCESS)
 		return 0;
 	else if (wait_data->result == HNS3_WAIT_TIMEOUT) {
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		hns3_warn(hw, "Reset step4 hardware not ready after reset time=%ld.%.6ld",
 			  tv.tv_sec, tv.tv_usec);
 		return -ETIME;
@@ -6344,7 +6344,7 @@ hns3_wait_hardware_ready(struct hns3_adapter *hns)
 	wait_data->hns = hns;
 	wait_data->check_completion = is_pf_reset_done;
 	wait_data->end_ms = (uint64_t)HNS3_RESET_WAIT_CNT *
-				      HNS3_RESET_WAIT_MS + get_timeofday_ms();
+				HNS3_RESET_WAIT_MS + hns3_clock_gettime_ms();
 	wait_data->interval = HNS3_RESET_WAIT_MS * USEC_PER_MSEC;
 	wait_data->count = HNS3_RESET_WAIT_CNT;
 	wait_data->result = HNS3_WAIT_REQUEST;
@@ -6383,7 +6383,7 @@ hns3_msix_process(struct hns3_adapter *hns, enum hns3_reset_level reset_level)
 	struct timeval tv;
 	uint32_t val;
 
-	gettimeofday(&tv, NULL);
+	hns3_clock_gettime(&tv);
 	if (hns3_read_dev(hw, HNS3_GLOBAL_RESET_REG) ||
 	    hns3_read_dev(hw, HNS3_FUN_RST_ING)) {
 		hns3_warn(hw, "Don't process msix during resetting time=%ld.%.6ld",
@@ -6691,12 +6691,11 @@ hns3_reset_service(void *param)
 	 */
 	reset_level = hns3_get_reset_level(hns, &hw->reset.pending);
 	if (reset_level != HNS3_NONE_RESET) {
-		gettimeofday(&tv_start, NULL);
+		hns3_clock_gettime(&tv_start);
 		ret = hns3_reset_process(hns, reset_level);
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		timersub(&tv, &tv_start, &tv_delta);
-		msec = tv_delta.tv_sec * MSEC_PER_SEC +
-		       tv_delta.tv_usec / USEC_PER_MSEC;
+		msec = hns3_clock_calctime_ms(&tv_delta);
 		if (msec > HNS3_RESET_PROCESS_MS)
 			hns3_err(hw, "%d handle long time delta %" PRIu64
 				     " ms time=%ld.%.6ld",
@@ -7196,6 +7195,39 @@ hns3_get_module_info(struct rte_eth_dev *dev,
 	return 0;
 }
 
+void
+hns3_clock_gettime(struct timeval *tv)
+{
+#ifdef CLOCK_MONOTONIC_RAW /* Defined in glibc bits/time.h */
+#define CLOCK_TYPE CLOCK_MONOTONIC_RAW
+#else
+#define CLOCK_TYPE CLOCK_MONOTONIC
+#endif
+#define NSEC_TO_USEC_DIV 1000
+
+	struct timespec spec;
+	(void)clock_gettime(CLOCK_TYPE, &spec);
+
+	tv->tv_sec = spec.tv_sec;
+	tv->tv_usec = spec.tv_nsec / NSEC_TO_USEC_DIV;
+}
+
+uint64_t
+hns3_clock_calctime_ms(struct timeval *tv)
+{
+	return (uint64_t)tv->tv_sec * MSEC_PER_SEC +
+		tv->tv_usec / USEC_PER_MSEC;
+}
+
+uint64_t
+hns3_clock_gettime_ms(void)
+{
+	struct timeval tv;
+
+	hns3_clock_gettime(&tv);
+	return hns3_clock_calctime_ms(&tv);
+}
+
 static int
 hns3_parse_io_hint_func(const char *key, const char *value, void *extra_args)
 {
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index ba50e70..b2dacb9 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -1022,15 +1022,9 @@ static inline uint32_t hns3_read_reg(void *base, uint32_t reg)
 #define MSEC_PER_SEC              1000L
 #define USEC_PER_MSEC             1000L
 
-static inline uint64_t
-get_timeofday_ms(void)
-{
-	struct timeval tv;
-
-	(void)gettimeofday(&tv, NULL);
-
-	return (uint64_t)tv.tv_sec * MSEC_PER_SEC + tv.tv_usec / USEC_PER_MSEC;
-}
+void hns3_clock_gettime(struct timeval *tv);
+uint64_t hns3_clock_calctime_ms(struct timeval *tv);
+uint64_t hns3_clock_gettime_ms(void);
 
 static inline uint64_t
 hns3_atomic_test_bit(unsigned int nr, volatile uint64_t *addr)
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index fcdf0e3..9a85e97 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2510,7 +2510,7 @@ hns3vf_wait_hardware_ready(struct hns3_adapter *hns)
 		hns3_warn(hw, "hardware is ready, delay 1 sec for PF reset complete");
 		return -EAGAIN;
 	} else if (wait_data->result == HNS3_WAIT_TIMEOUT) {
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		hns3_warn(hw, "Reset step4 hardware not ready after reset time=%ld.%.6ld",
 			  tv.tv_sec, tv.tv_usec);
 		return -ETIME;
@@ -2520,7 +2520,7 @@ hns3vf_wait_hardware_ready(struct hns3_adapter *hns)
 	wait_data->hns = hns;
 	wait_data->check_completion = is_vf_reset_done;
 	wait_data->end_ms = (uint64_t)HNS3VF_RESET_WAIT_CNT *
-				      HNS3VF_RESET_WAIT_MS + get_timeofday_ms();
+				HNS3VF_RESET_WAIT_MS + hns3_clock_gettime_ms();
 	wait_data->interval = HNS3VF_RESET_WAIT_MS * USEC_PER_MSEC;
 	wait_data->count = HNS3VF_RESET_WAIT_CNT;
 	wait_data->result = HNS3_WAIT_REQUEST;
@@ -2775,12 +2775,11 @@ hns3vf_reset_service(void *param)
 	 */
 	reset_level = hns3vf_get_reset_level(hw, &hw->reset.pending);
 	if (reset_level != HNS3_NONE_RESET) {
-		gettimeofday(&tv_start, NULL);
+		hns3_clock_gettime(&tv_start);
 		hns3_reset_process(hns, reset_level);
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		timersub(&tv, &tv_start, &tv_delta);
-		msec = tv_delta.tv_sec * MSEC_PER_SEC +
-		       tv_delta.tv_usec / USEC_PER_MSEC;
+		msec = hns3_clock_calctime_ms(&tv_delta);
 		if (msec > HNS3_RESET_PROCESS_MS)
 			hns3_err(hw, "%d handle long time delta %" PRIu64
 				 " ms time=%ld.%.6ld",
diff --git a/drivers/net/hns3/hns3_intr.c b/drivers/net/hns3/hns3_intr.c
index cc7d7c6..ba6a044 100644
--- a/drivers/net/hns3/hns3_intr.c
+++ b/drivers/net/hns3/hns3_intr.c
@@ -2465,7 +2465,7 @@ hns3_wait_callback(void *param)
 		 * Check if the current time exceeds the deadline
 		 * or a pending reset coming, or reset during close.
 		 */
-		msec = get_timeofday_ms();
+		msec = hns3_clock_gettime_ms();
 		if (msec > data->end_ms || is_reset_pending(hns) ||
 		    hw->adapter_state == HNS3_NIC_CLOSING) {
 			done = false;
@@ -2650,7 +2650,7 @@ hns3_reset_pre(struct hns3_adapter *hns)
 		__atomic_store_n(&hns->hw.reset.resetting, 1, __ATOMIC_RELAXED);
 		hw->reset.stage = RESET_STAGE_DOWN;
 		ret = hw->reset.ops->stop_service(hns);
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		if (ret) {
 			hns3_warn(hw, "Reset step1 down fail=%d time=%ld.%.6ld",
 				  ret, tv.tv_sec, tv.tv_usec);
@@ -2662,7 +2662,7 @@ hns3_reset_pre(struct hns3_adapter *hns)
 	}
 	if (hw->reset.stage == RESET_STAGE_PREWAIT) {
 		ret = hw->reset.ops->prepare_reset(hns);
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		if (ret) {
 			hns3_warn(hw,
 				  "Reset step2 prepare wait fail=%d time=%ld.%.6ld",
@@ -2700,7 +2700,7 @@ hns3_reset_post(struct hns3_adapter *hns)
 		}
 		ret = hw->reset.ops->reinit_dev(hns);
 		rte_spinlock_unlock(&hw->lock);
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		if (ret) {
 			hns3_warn(hw, "Reset step5 devinit fail=%d retries=%d",
 				  ret, hw->reset.retries);
@@ -2718,7 +2718,7 @@ hns3_reset_post(struct hns3_adapter *hns)
 		rte_spinlock_lock(&hw->lock);
 		ret = hw->reset.ops->restore_conf(hns);
 		rte_spinlock_unlock(&hw->lock);
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		if (ret) {
 			hns3_warn(hw,
 				  "Reset step6 restore fail=%d retries=%d",
@@ -2741,7 +2741,7 @@ hns3_reset_post(struct hns3_adapter *hns)
 		rte_spinlock_lock(&hw->lock);
 		hw->reset.ops->start_service(hns);
 		rte_spinlock_unlock(&hw->lock);
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		timersub(&tv, &hw->reset.start_time, &tv_delta);
 		hns3_warn(hw, "%s reset done fail_cnt:%" PRIx64
 			  " success_cnt:%" PRIx64 " global_cnt:%" PRIx64
@@ -2753,10 +2753,9 @@ hns3_reset_post(struct hns3_adapter *hns)
 			  hw->reset.stats.request_cnt, hw->reset.stats.exec_cnt,
 			  hw->reset.stats.merge_cnt);
 		hns3_warn(hw,
-			  "%s reset done delta %ld ms time=%ld.%.6ld",
+			  "%s reset done delta %" PRIu64 " ms time=%ld.%.6ld",
 			  reset_string[hw->reset.level],
-			  tv_delta.tv_sec * MSEC_PER_SEC +
-			  tv_delta.tv_usec / USEC_PER_MSEC,
+			  hns3_clock_calctime_ms(&tv_delta),
 			  tv.tv_sec, tv.tv_usec);
 		hw->reset.level = HNS3_NONE_RESET;
 	}
@@ -2796,7 +2795,7 @@ hns3_reset_process(struct hns3_adapter *hns, enum hns3_reset_level new_level)
 	if (hw->reset.level == HNS3_NONE_RESET) {
 		hw->reset.level = new_level;
 		hw->reset.stats.exec_cnt++;
-		gettimeofday(&hw->reset.start_time, NULL);
+		hns3_clock_gettime(&hw->reset.start_time);
 		hns3_warn(hw, "Start %s reset time=%ld.%.6ld",
 			  reset_string[hw->reset.level],
 			  hw->reset.start_time.tv_sec,
@@ -2804,7 +2803,7 @@ hns3_reset_process(struct hns3_adapter *hns, enum hns3_reset_level new_level)
 	}
 
 	if (is_reset_pending(hns)) {
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		hns3_warn(hw,
 			  "%s reset is aborted by high level time=%ld.%.6ld",
 			  reset_string[hw->reset.level], tv.tv_sec, tv.tv_usec);
@@ -2822,7 +2821,7 @@ hns3_reset_process(struct hns3_adapter *hns, enum hns3_reset_level new_level)
 		ret = hns3_reset_req_hw_reset(hns);
 		if (ret == -EAGAIN)
 			return ret;
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		hns3_warn(hw,
 			  "Reset step3 request IMP reset success time=%ld.%.6ld",
 			  tv.tv_sec, tv.tv_usec);
@@ -2833,7 +2832,7 @@ hns3_reset_process(struct hns3_adapter *hns, enum hns3_reset_level new_level)
 		ret = hw->reset.ops->wait_hardware_ready(hns);
 		if (ret)
 			goto retry;
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		hns3_warn(hw, "Reset step4 reset wait success time=%ld.%.6ld",
 			  tv.tv_sec, tv.tv_usec);
 		hw->reset.stage = RESET_STAGE_DEV_INIT;
@@ -2861,12 +2860,11 @@ hns3_reset_process(struct hns3_adapter *hns, enum hns3_reset_level new_level)
 		rte_spinlock_unlock(&hw->lock);
 		__atomic_store_n(&hns->hw.reset.resetting, 0, __ATOMIC_RELAXED);
 		hw->reset.stage = RESET_STAGE_NONE;
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		timersub(&tv, &hw->reset.start_time, &tv_delta);
-		hns3_warn(hw, "%s reset fail delta %ld ms time=%ld.%.6ld",
+		hns3_warn(hw, "%s reset fail delta %" PRIu64 " ms time=%ld.%.6ld",
 			  reset_string[hw->reset.level],
-			  tv_delta.tv_sec * MSEC_PER_SEC +
-			  tv_delta.tv_usec / USEC_PER_MSEC,
+			  hns3_clock_calctime_ms(&tv_delta),
 			  tv.tv_sec, tv.tv_usec);
 		hw->reset.level = HNS3_NONE_RESET;
 	}
@@ -2898,7 +2896,7 @@ hns3_reset_abort(struct hns3_adapter *hns)
 	rte_eal_alarm_cancel(hns3_wait_callback, hw->reset.wait_data);
 
 	if (hw->reset.level != HNS3_NONE_RESET) {
-		gettimeofday(&tv, NULL);
+		hns3_clock_gettime(&tv);
 		hns3_err(hw, "Failed to terminate reset: %s time=%ld.%.6ld",
 			 reset_string[hw->reset.level], tv.tv_sec, tv.tv_usec);
 	}
-- 
2.7.4


  parent reply	other threads:[~2021-04-28  7:21 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-28  7:20 [dpdk-dev] [PATCH 0/5] Features and bugfix for hns3 PMD Min Hu (Connor)
2021-04-28  7:20 ` [dpdk-dev] [PATCH 1/5] net/hns3: support preferred burst size and queues in VF Min Hu (Connor)
2021-04-28  7:20 ` [dpdk-dev] [PATCH 2/5] net/hns3: log time delta in decimal format Min Hu (Connor)
2021-04-28  7:20 ` Min Hu (Connor) [this message]
2021-04-28  7:20 ` [dpdk-dev] [PATCH 4/5] net/hns3: delete unused macro of cmd module Min Hu (Connor)
2021-04-28  7:20 ` [dpdk-dev] [PATCH 5/5] net/hns3: select Tx prepare based on Tx offload Min Hu (Connor)
2021-05-07  9:26   ` David Marchand
2021-05-07 10:23     ` Ferruh Yigit
2021-05-07 12:20       ` [dpdk-dev] [PATCH] net/hns3: fix debug build Thomas Monjalon
2021-05-07 12:25         ` David Marchand
2021-05-07 12:49           ` Thomas Monjalon
2021-05-08  0:47           ` Min Hu (Connor)
2021-04-29 16:27 ` [dpdk-dev] [PATCH 0/5] Features and bugfix for hns3 PMD Ferruh Yigit

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=1619594455-56787-4-git-send-email-humin29@huawei.com \
    --to=humin29@huawei.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.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).