DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/iavf: add devargs to enable watchdog
@ 2023-04-18  5:42 Zhichao Zeng
  2023-04-24  2:33 ` Zhang, Qi Z
  2023-05-05  2:27 ` [PATCH v2] net/iavf: add devargs to control watchdog Zhichao Zeng
  0 siblings, 2 replies; 8+ messages in thread
From: Zhichao Zeng @ 2023-04-18  5:42 UTC (permalink / raw)
  To: dev; +Cc: qi.z.zhang, Zhichao Zeng, Jingjing Wu, Beilei Xing

This patch adds devargs to enable reset watchdog for iavf, use
'-a {pci:xxxx:xx:xx:x},watchdog_period={microseconds}' to enable watchdog.

If the watchdog period is configured through the IAVF_DEV_WATCHDOG_PERIOD
and devargs at the same time, the IAVF_DEV_WATCHDOG_PERIOD will prevail.

Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
---
 drivers/net/iavf/iavf.h        |  1 +
 drivers/net/iavf/iavf_ethdev.c | 54 ++++++++++++++++++++++++++++++++--
 drivers/net/iavf/iavf_vchnl.c  |  9 ++++--
 3 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index aa18650ffa..fc0ce529ce 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -304,6 +304,7 @@ struct iavf_devargs {
 	uint8_t proto_xtr_dflt;
 	uint8_t proto_xtr[IAVF_MAX_QUEUE_NUM];
 	uint16_t quanta_size;
+	uint32_t watchdog_period; /* microseconds */
 };
 
 struct iavf_security_ctx;
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index f6d68403ce..f7c7ee3348 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -36,6 +36,7 @@
 /* devargs */
 #define IAVF_PROTO_XTR_ARG         "proto_xtr"
 #define IAVF_QUANTA_SIZE_ARG       "quanta_size"
+#define IAVF_RESET_WATCHDOG_ARG    "watchdog_period"
 
 uint64_t iavf_timestamp_dynflag;
 int iavf_timestamp_dynfield_offset = -1;
@@ -43,6 +44,7 @@ int iavf_timestamp_dynfield_offset = -1;
 static const char * const iavf_valid_args[] = {
 	IAVF_PROTO_XTR_ARG,
 	IAVF_QUANTA_SIZE_ARG,
+	IAVF_RESET_WATCHDOG_ARG,
 	NULL
 };
 
@@ -301,15 +303,23 @@ iavf_dev_watchdog(void *cb_arg)
 
 			/* enter reset state with VFLR event */
 			adapter->vf.vf_reset = true;
+			adapter->vf.link_up = false;
 
 			rte_eth_dev_callback_process(adapter->vf.eth_dev,
 				RTE_ETH_EVENT_INTR_RESET, NULL);
 		}
 	}
 
+#if (IAVF_DEV_WATCHDOG_PERIOD > 0)
 	/* re-alarm watchdog */
 	rc = rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
 			&iavf_dev_watchdog, cb_arg);
+#else
+	/* re-alarm watchdog by devargs */
+	if (adapter->devargs.watchdog_period)
+		rc = rte_eal_alarm_set(adapter->devargs.watchdog_period,
+				&iavf_dev_watchdog, cb_arg);
+#endif
 
 	if (rc)
 		PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
@@ -317,23 +327,37 @@ iavf_dev_watchdog(void *cb_arg)
 }
 
 static void
-iavf_dev_watchdog_enable(struct iavf_adapter *adapter __rte_unused)
+iavf_dev_watchdog_enable(struct iavf_adapter *adapter)
 {
 #if (IAVF_DEV_WATCHDOG_PERIOD > 0)
-	PMD_DRV_LOG(INFO, "Enabling device watchdog");
+	PMD_DRV_LOG(INFO, "Enabling device watchdog, macro: %dμs", IAVF_DEV_WATCHDOG_PERIOD);
 	adapter->vf.watchdog_enabled = true;
 	if (rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
 			&iavf_dev_watchdog, (void *)adapter))
 		PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
+#else
+	if (adapter->devargs.watchdog_period) {
+		PMD_DRV_LOG(INFO, "Enabling device watchdog, devargs: %dμs",
+				adapter->devargs.watchdog_period);
+		adapter->vf.watchdog_enabled = true;
+		if (rte_eal_alarm_set(adapter->devargs.watchdog_period,
+				&iavf_dev_watchdog, (void *)adapter))
+			PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
+	}
 #endif
 }
 
 static void
-iavf_dev_watchdog_disable(struct iavf_adapter *adapter __rte_unused)
+iavf_dev_watchdog_disable(struct iavf_adapter *adapter)
 {
 #if (IAVF_DEV_WATCHDOG_PERIOD > 0)
 	PMD_DRV_LOG(INFO, "Disabling device watchdog");
 	adapter->vf.watchdog_enabled = false;
+#else
+	if (adapter->devargs.watchdog_period) {
+		PMD_DRV_LOG(INFO, "Disabling device watchdog");
+		adapter->vf.watchdog_enabled = false;
+	}
 #endif
 }
 
@@ -2201,6 +2225,25 @@ parse_u16(__rte_unused const char *key, const char *value, void *args)
 	return 0;
 }
 
+static int
+iavf_parse_watchdog_period(__rte_unused const char *key, const char *value, void *args)
+{
+	u32 *num = (u32 *)args;
+	u32 tmp;
+
+	errno = 0;
+	tmp = strtoul(value, NULL, 10);
+	if (errno || !tmp) {
+		PMD_DRV_LOG(WARNING, "%s: \"%s\" is not a valid u32",
+				key, value);
+		return -1;
+	}
+
+	*num = tmp;
+
+	return 0;
+}
+
 static int iavf_parse_devargs(struct rte_eth_dev *dev)
 {
 	struct iavf_adapter *ad =
@@ -2232,6 +2275,11 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	if (ret)
 		goto bail;
 
+	ret = rte_kvargs_process(kvlist, IAVF_RESET_WATCHDOG_ARG,
+				 &iavf_parse_watchdog_period, &ad->devargs.watchdog_period);
+	if (ret)
+		goto bail;
+
 	if (ad->devargs.quanta_size != 0 &&
 	    (ad->devargs.quanta_size < 256 || ad->devargs.quanta_size > 4096 ||
 	     ad->devargs.quanta_size & 0x40)) {
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 9adaadb173..402261ba9c 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -433,9 +433,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 	switch (pf_msg->event) {
 	case VIRTCHNL_EVENT_RESET_IMPENDING:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
-		vf->vf_reset = true;
-		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
-					      NULL, 0);
+		vf->link_up = false;
+		if (!vf->vf_reset) {
+			vf->vf_reset = true;
+			iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
+				NULL, 0);
+		}
 		break;
 	case VIRTCHNL_EVENT_LINK_CHANGE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
-- 
2.25.1


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

* RE: [PATCH] net/iavf: add devargs to enable watchdog
  2023-04-18  5:42 [PATCH] net/iavf: add devargs to enable watchdog Zhichao Zeng
@ 2023-04-24  2:33 ` Zhang, Qi Z
  2023-05-05  2:27 ` [PATCH v2] net/iavf: add devargs to control watchdog Zhichao Zeng
  1 sibling, 0 replies; 8+ messages in thread
From: Zhang, Qi Z @ 2023-04-24  2:33 UTC (permalink / raw)
  To: Zeng, ZhichaoX, dev; +Cc: Wu, Jingjing, Xing, Beilei



> -----Original Message-----
> From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> Sent: Tuesday, April 18, 2023 1:42 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zeng, ZhichaoX
> <zhichaox.zeng@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>
> Subject: [PATCH] net/iavf: add devargs to enable watchdog
> 
> This patch adds devargs to enable reset watchdog for iavf, use '-a
> {pci:xxxx:xx:xx:x},watchdog_period={microseconds}' to enable watchdog.
> 
> If the watchdog period is configured through the
> IAVF_DEV_WATCHDOG_PERIOD and devargs at the same time, the
> IAVF_DEV_WATCHDOG_PERIOD will prevail.

Is devargs prevail is more reasonable for user?

> 
> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> ---
>  drivers/net/iavf/iavf.h        |  1 +
>  drivers/net/iavf/iavf_ethdev.c | 54 ++++++++++++++++++++++++++++++++--
>  drivers/net/iavf/iavf_vchnl.c  |  9 ++++--
>  3 files changed, 58 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
> aa18650ffa..fc0ce529ce 100644
> --- a/drivers/net/iavf/iavf.h
> +++ b/drivers/net/iavf/iavf.h
> @@ -304,6 +304,7 @@ struct iavf_devargs {
>  	uint8_t proto_xtr_dflt;
>  	uint8_t proto_xtr[IAVF_MAX_QUEUE_NUM];
>  	uint16_t quanta_size;
> +	uint32_t watchdog_period; /* microseconds */
>  };
> 
>  struct iavf_security_ctx;
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index f6d68403ce..f7c7ee3348 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -36,6 +36,7 @@
>  /* devargs */
>  #define IAVF_PROTO_XTR_ARG         "proto_xtr"
>  #define IAVF_QUANTA_SIZE_ARG       "quanta_size"
> +#define IAVF_RESET_WATCHDOG_ARG    "watchdog_period"
> 
>  uint64_t iavf_timestamp_dynflag;
>  int iavf_timestamp_dynfield_offset = -1; @@ -43,6 +44,7 @@ int
> iavf_timestamp_dynfield_offset = -1;  static const char * const
> iavf_valid_args[] = {
>  	IAVF_PROTO_XTR_ARG,
>  	IAVF_QUANTA_SIZE_ARG,
> +	IAVF_RESET_WATCHDOG_ARG,
>  	NULL
>  };
> 
> @@ -301,15 +303,23 @@ iavf_dev_watchdog(void *cb_arg)
> 
>  			/* enter reset state with VFLR event */
>  			adapter->vf.vf_reset = true;
> +			adapter->vf.link_up = false;
> 
>  			rte_eth_dev_callback_process(adapter->vf.eth_dev,
>  				RTE_ETH_EVENT_INTR_RESET, NULL);
>  		}
>  	}
> 
> +#if (IAVF_DEV_WATCHDOG_PERIOD > 0)
>  	/* re-alarm watchdog */
>  	rc = rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
>  			&iavf_dev_watchdog, cb_arg);
> +#else
> +	/* re-alarm watchdog by devargs */
> +	if (adapter->devargs.watchdog_period)
> +		rc = rte_eal_alarm_set(adapter->devargs.watchdog_period,
> +				&iavf_dev_watchdog, cb_arg);
> +#endif
> 
>  	if (rc)
>  		PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog
> alarm", @@ -317,23 +327,37 @@ iavf_dev_watchdog(void *cb_arg)  }
> 
>  static void
> -iavf_dev_watchdog_enable(struct iavf_adapter *adapter __rte_unused)
> +iavf_dev_watchdog_enable(struct iavf_adapter *adapter)
>  {
>  #if (IAVF_DEV_WATCHDOG_PERIOD > 0)
> -	PMD_DRV_LOG(INFO, "Enabling device watchdog");
> +	PMD_DRV_LOG(INFO, "Enabling device watchdog, macro: %dμs",
> +IAVF_DEV_WATCHDOG_PERIOD);
>  	adapter->vf.watchdog_enabled = true;
>  	if (rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
>  			&iavf_dev_watchdog, (void *)adapter))
>  		PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
> +#else
> +	if (adapter->devargs.watchdog_period) {
> +		PMD_DRV_LOG(INFO, "Enabling device watchdog,
> devargs: %dμs",
> +				adapter->devargs.watchdog_period);
> +		adapter->vf.watchdog_enabled = true;
> +		if (rte_eal_alarm_set(adapter->devargs.watchdog_period,
> +				&iavf_dev_watchdog, (void *)adapter))
> +			PMD_DRV_LOG(ERR, "Failed to enabled device
> watchdog");
> +	}
>  #endif
>  }
> 
>  static void
> -iavf_dev_watchdog_disable(struct iavf_adapter *adapter __rte_unused)
> +iavf_dev_watchdog_disable(struct iavf_adapter *adapter)
>  {
>  #if (IAVF_DEV_WATCHDOG_PERIOD > 0)
>  	PMD_DRV_LOG(INFO, "Disabling device watchdog");
>  	adapter->vf.watchdog_enabled = false;
> +#else
> +	if (adapter->devargs.watchdog_period) {
> +		PMD_DRV_LOG(INFO, "Disabling device watchdog");
> +		adapter->vf.watchdog_enabled = false;
> +	}
>  #endif
>  }
> 
> @@ -2201,6 +2225,25 @@ parse_u16(__rte_unused const char *key, const
> char *value, void *args)
>  	return 0;
>  }
> 
> +static int
> +iavf_parse_watchdog_period(__rte_unused const char *key, const char
> +*value, void *args) {
> +	u32 *num = (u32 *)args;
> +	u32 tmp;
> +
> +	errno = 0;
> +	tmp = strtoul(value, NULL, 10);
> +	if (errno || !tmp) {
> +		PMD_DRV_LOG(WARNING, "%s: \"%s\" is not a valid u32",
> +				key, value);
> +		return -1;
> +	}
> +
> +	*num = tmp;
> +
> +	return 0;
> +}
> +
>  static int iavf_parse_devargs(struct rte_eth_dev *dev)  {
>  	struct iavf_adapter *ad =
> @@ -2232,6 +2275,11 @@ static int iavf_parse_devargs(struct rte_eth_dev
> *dev)
>  	if (ret)
>  		goto bail;
> 
> +	ret = rte_kvargs_process(kvlist, IAVF_RESET_WATCHDOG_ARG,
> +				 &iavf_parse_watchdog_period, &ad-
> >devargs.watchdog_period);
> +	if (ret)
> +		goto bail;
> +
>  	if (ad->devargs.quanta_size != 0 &&
>  	    (ad->devargs.quanta_size < 256 || ad->devargs.quanta_size > 4096
> ||
>  	     ad->devargs.quanta_size & 0x40)) { diff --git
> a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index
> 9adaadb173..402261ba9c 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -433,9 +433,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev,
> uint8_t *msg,
>  	switch (pf_msg->event) {
>  	case VIRTCHNL_EVENT_RESET_IMPENDING:
>  		PMD_DRV_LOG(DEBUG,
> "VIRTCHNL_EVENT_RESET_IMPENDING event");
> -		vf->vf_reset = true;
> -		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
> -					      NULL, 0);
> +		vf->link_up = false;
> +		if (!vf->vf_reset) {
> +			vf->vf_reset = true;
> +			iavf_dev_event_post(dev,
> RTE_ETH_EVENT_INTR_RESET,
> +				NULL, 0);
> +		}
>  		break;
>  	case VIRTCHNL_EVENT_LINK_CHANGE:
>  		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE
> event");
> --
> 2.25.1


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

* [PATCH v2] net/iavf: add devargs to control watchdog
  2023-04-18  5:42 [PATCH] net/iavf: add devargs to enable watchdog Zhichao Zeng
  2023-04-24  2:33 ` Zhang, Qi Z
@ 2023-05-05  2:27 ` Zhichao Zeng
  2023-05-15  2:02   ` Zhang, Qi Z
  2023-05-15  5:47   ` [PATCH v3] " Zhichao Zeng
  1 sibling, 2 replies; 8+ messages in thread
From: Zhichao Zeng @ 2023-05-05  2:27 UTC (permalink / raw)
  To: dev; +Cc: qi.z.zhang, Zhichao Zeng, Jingjing Wu, Beilei Xing

This patch enables the watchdog to detect VF FLR when the link state
changes to down, and the default period is 2000us as defined by
IAVF_DEV_WATCHDOG_PERIOD.

In addition, the 'watchdog_period' devargs was added to adjust the watchdog
period(microseconds), or set to 0 to disable the watchdog.

Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>

---
v2: enables watchdog when link status changes to down
---
 drivers/net/iavf/iavf.h        |  5 ++-
 drivers/net/iavf/iavf_ethdev.c | 81 +++++++++++++++++++++++++---------
 drivers/net/iavf/iavf_vchnl.c  | 21 +++++++--
 3 files changed, 81 insertions(+), 26 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index aa18650ffa..98861e4242 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -31,7 +31,7 @@
 
 #define IAVF_NUM_MACADDR_MAX      64
 
-#define IAVF_DEV_WATCHDOG_PERIOD     0
+#define IAVF_DEV_WATCHDOG_PERIOD     2000 /* microseconds, set 0 to disable*/
 
 #define IAVF_DEFAULT_RX_PTHRESH      8
 #define IAVF_DEFAULT_RX_HTHRESH      8
@@ -304,6 +304,7 @@ struct iavf_devargs {
 	uint8_t proto_xtr_dflt;
 	uint8_t proto_xtr[IAVF_MAX_QUEUE_NUM];
 	uint16_t quanta_size;
+	uint32_t watchdog_period;
 };
 
 struct iavf_security_ctx;
@@ -498,4 +499,6 @@ int iavf_flow_unsub(struct iavf_adapter *adapter,
 		    struct iavf_fsub_conf *filter);
 int iavf_flow_sub_check(struct iavf_adapter *adapter,
 			struct iavf_fsub_conf *filter);
+void iavf_dev_watchdog_enable(struct iavf_adapter *adapter);
+void iavf_dev_watchdog_disable(struct iavf_adapter *adapter);
 #endif /* _IAVF_ETHDEV_H_ */
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index f6d68403ce..4cf4eb20cd 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -36,6 +36,7 @@
 /* devargs */
 #define IAVF_PROTO_XTR_ARG         "proto_xtr"
 #define IAVF_QUANTA_SIZE_ARG       "quanta_size"
+#define IAVF_RESET_WATCHDOG_ARG    "watchdog_period"
 
 uint64_t iavf_timestamp_dynflag;
 int iavf_timestamp_dynfield_offset = -1;
@@ -43,6 +44,7 @@ int iavf_timestamp_dynfield_offset = -1;
 static const char * const iavf_valid_args[] = {
 	IAVF_PROTO_XTR_ARG,
 	IAVF_QUANTA_SIZE_ARG,
+	IAVF_RESET_WATCHDOG_ARG,
 	NULL
 };
 
@@ -301,40 +303,46 @@ iavf_dev_watchdog(void *cb_arg)
 
 			/* enter reset state with VFLR event */
 			adapter->vf.vf_reset = true;
+			adapter->vf.link_up = false;
 
 			rte_eth_dev_callback_process(adapter->vf.eth_dev,
 				RTE_ETH_EVENT_INTR_RESET, NULL);
 		}
 	}
 
-	/* re-alarm watchdog */
-	rc = rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
-			&iavf_dev_watchdog, cb_arg);
+	if (adapter->devargs.watchdog_period) {
+		/* re-alarm watchdog */
+		rc = rte_eal_alarm_set(adapter->devargs.watchdog_period,
+					&iavf_dev_watchdog, cb_arg);
 
-	if (rc)
-		PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
-			adapter->vf.eth_dev->data->name);
+		if (rc)
+			PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
+				adapter->vf.eth_dev->data->name);
+	}
 }
 
-static void
-iavf_dev_watchdog_enable(struct iavf_adapter *adapter __rte_unused)
-{
-#if (IAVF_DEV_WATCHDOG_PERIOD > 0)
-	PMD_DRV_LOG(INFO, "Enabling device watchdog");
-	adapter->vf.watchdog_enabled = true;
-	if (rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
-			&iavf_dev_watchdog, (void *)adapter))
-		PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
-#endif
+void
+iavf_dev_watchdog_enable(struct iavf_adapter *adapter)
+{
+	if (adapter->devargs.watchdog_period && !adapter->vf.watchdog_enabled) {
+		PMD_DRV_LOG(INFO, "Enabling device watchdog, period is %dμs",
+					adapter->devargs.watchdog_period);
+		adapter->vf.watchdog_enabled = true;
+		if (rte_eal_alarm_set(adapter->devargs.watchdog_period,
+					&iavf_dev_watchdog, (void *)adapter))
+			PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
+	} else {
+		PMD_DRV_LOG(INFO, "Device watchdog is disabled");
+	}
 }
 
-static void
-iavf_dev_watchdog_disable(struct iavf_adapter *adapter __rte_unused)
+void
+iavf_dev_watchdog_disable(struct iavf_adapter *adapter)
 {
-#if (IAVF_DEV_WATCHDOG_PERIOD > 0)
-	PMD_DRV_LOG(INFO, "Disabling device watchdog");
-	adapter->vf.watchdog_enabled = false;
-#endif
+	if (adapter->devargs.watchdog_period && adapter->vf.watchdog_enabled) {
+		PMD_DRV_LOG(INFO, "Disabling device watchdog");
+		adapter->vf.watchdog_enabled = false;
+	}
 }
 
 static int
@@ -2201,6 +2209,25 @@ parse_u16(__rte_unused const char *key, const char *value, void *args)
 	return 0;
 }
 
+static int
+iavf_parse_watchdog_period(__rte_unused const char *key, const char *value, void *args)
+{
+	int *num = (int *)args;
+	int tmp;
+
+	errno = 0;
+	tmp = atoi(value);
+	if (tmp < 0) {
+		PMD_DRV_LOG(WARNING, "%s: \"%s\" is not greater than or equal to zero",
+				key, value);
+		return -1;
+	}
+
+	*num = tmp;
+
+	return 0;
+}
+
 static int iavf_parse_devargs(struct rte_eth_dev *dev)
 {
 	struct iavf_adapter *ad =
@@ -2208,6 +2235,7 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	struct rte_devargs *devargs = dev->device->devargs;
 	struct rte_kvargs *kvlist;
 	int ret;
+	int watchdog_period = -1;
 
 	if (!devargs)
 		return 0;
@@ -2232,6 +2260,15 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	if (ret)
 		goto bail;
 
+	ret = rte_kvargs_process(kvlist, IAVF_RESET_WATCHDOG_ARG,
+				 &iavf_parse_watchdog_period, &watchdog_period);
+	if (ret)
+		goto bail;
+	if (watchdog_period == -1)
+		ad->devargs.watchdog_period = IAVF_DEV_WATCHDOG_PERIOD;
+	else
+		ad->devargs.watchdog_period = watchdog_period;
+
 	if (ad->devargs.quanta_size != 0 &&
 	    (ad->devargs.quanta_size < 256 || ad->devargs.quanta_size > 4096 ||
 	     ad->devargs.quanta_size & 0x40)) {
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 9adaadb173..f2d7331f55 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -256,6 +256,12 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 				vf->link_speed = iavf_convert_link_speed(speed);
 			}
 			iavf_dev_link_update(vf->eth_dev, 0);
+			if (vf->link_up && !vf->vf_reset) {
+				iavf_dev_watchdog_disable(adapter);
+			} else {
+				if (!vf->link_up)
+					iavf_dev_watchdog_enable(adapter);
+			}
 			PMD_DRV_LOG(INFO, "Link status update:%s",
 					vf->link_up ? "up" : "down");
 			break;
@@ -433,9 +439,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 	switch (pf_msg->event) {
 	case VIRTCHNL_EVENT_RESET_IMPENDING:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
-		vf->vf_reset = true;
-		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
-					      NULL, 0);
+		vf->link_up = false;
+		if (!vf->vf_reset) {
+			vf->vf_reset = true;
+			iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
+				NULL, 0);
+		}
 		break;
 	case VIRTCHNL_EVENT_LINK_CHANGE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
@@ -449,6 +458,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 			vf->link_speed = iavf_convert_link_speed(speed);
 		}
 		iavf_dev_link_update(dev, 0);
+		if (vf->link_up && !vf->vf_reset) {
+			iavf_dev_watchdog_disable(adapter);
+		} else {
+			if (!vf->link_up)
+				iavf_dev_watchdog_enable(adapter);
+		}
 		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL, 0);
 		break;
 	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
-- 
2.25.1


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

* RE: [PATCH v2] net/iavf: add devargs to control watchdog
  2023-05-05  2:27 ` [PATCH v2] net/iavf: add devargs to control watchdog Zhichao Zeng
@ 2023-05-15  2:02   ` Zhang, Qi Z
  2023-05-15  5:47   ` [PATCH v3] " Zhichao Zeng
  1 sibling, 0 replies; 8+ messages in thread
From: Zhang, Qi Z @ 2023-05-15  2:02 UTC (permalink / raw)
  To: Zeng, ZhichaoX, dev; +Cc: Wu, Jingjing, Xing, Beilei



> -----Original Message-----
> From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> Sent: Friday, May 5, 2023 10:27 AM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zeng, ZhichaoX
> <zhichaox.zeng@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing,
> Beilei <beilei.xing@intel.com>
> Subject: [PATCH v2] net/iavf: add devargs to control watchdog
> 
> This patch enables the watchdog to detect VF FLR when the link state
> changes to down, and the default period is 2000us as defined by
> IAVF_DEV_WATCHDOG_PERIOD.
> 
> In addition, the 'watchdog_period' devargs was added to adjust the
> watchdog period(microseconds), or set to 0 to disable the watchdog.

Please also update ice.rst by adding the usage of the new devarg.

> 
> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> 
> ---
> v2: enables watchdog when link status changes to down
> ---
>  drivers/net/iavf/iavf.h        |  5 ++-
>  drivers/net/iavf/iavf_ethdev.c | 81 +++++++++++++++++++++++++---------
>  drivers/net/iavf/iavf_vchnl.c  | 21 +++++++--
>  3 files changed, 81 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
> aa18650ffa..98861e4242 100644
> --- a/drivers/net/iavf/iavf.h
> +++ b/drivers/net/iavf/iavf.h
> @@ -31,7 +31,7 @@
> 
>  #define IAVF_NUM_MACADDR_MAX      64
> 
> -#define IAVF_DEV_WATCHDOG_PERIOD     0
> +#define IAVF_DEV_WATCHDOG_PERIOD     2000 /* microseconds, set 0 to
> disable*/
> 
>  #define IAVF_DEFAULT_RX_PTHRESH      8
>  #define IAVF_DEFAULT_RX_HTHRESH      8
> @@ -304,6 +304,7 @@ struct iavf_devargs {
>  	uint8_t proto_xtr_dflt;
>  	uint8_t proto_xtr[IAVF_MAX_QUEUE_NUM];
>  	uint16_t quanta_size;
> +	uint32_t watchdog_period;
>  };
> 
>  struct iavf_security_ctx;
> @@ -498,4 +499,6 @@ int iavf_flow_unsub(struct iavf_adapter *adapter,
>  		    struct iavf_fsub_conf *filter);
>  int iavf_flow_sub_check(struct iavf_adapter *adapter,
>  			struct iavf_fsub_conf *filter);
> +void iavf_dev_watchdog_enable(struct iavf_adapter *adapter); void
> +iavf_dev_watchdog_disable(struct iavf_adapter *adapter);
>  #endif /* _IAVF_ETHDEV_H_ */
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index f6d68403ce..4cf4eb20cd 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -36,6 +36,7 @@
>  /* devargs */
>  #define IAVF_PROTO_XTR_ARG         "proto_xtr"
>  #define IAVF_QUANTA_SIZE_ARG       "quanta_size"
> +#define IAVF_RESET_WATCHDOG_ARG    "watchdog_period"
> 
>  uint64_t iavf_timestamp_dynflag;
>  int iavf_timestamp_dynfield_offset = -1; @@ -43,6 +44,7 @@ int
> iavf_timestamp_dynfield_offset = -1;  static const char * const
> iavf_valid_args[] = {
>  	IAVF_PROTO_XTR_ARG,
>  	IAVF_QUANTA_SIZE_ARG,
> +	IAVF_RESET_WATCHDOG_ARG,
>  	NULL
>  };
> 
> @@ -301,40 +303,46 @@ iavf_dev_watchdog(void *cb_arg)
> 
>  			/* enter reset state with VFLR event */
>  			adapter->vf.vf_reset = true;
> +			adapter->vf.link_up = false;
> 
>  			rte_eth_dev_callback_process(adapter->vf.eth_dev,
>  				RTE_ETH_EVENT_INTR_RESET, NULL);
>  		}
>  	}
> 
> -	/* re-alarm watchdog */
> -	rc = rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
> -			&iavf_dev_watchdog, cb_arg);
> +	if (adapter->devargs.watchdog_period) {
> +		/* re-alarm watchdog */
> +		rc = rte_eal_alarm_set(adapter->devargs.watchdog_period,
> +					&iavf_dev_watchdog, cb_arg);
> 
> -	if (rc)
> -		PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog
> alarm",
> -			adapter->vf.eth_dev->data->name);
> +		if (rc)
> +			PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device
> watchdog alarm",
> +				adapter->vf.eth_dev->data->name);
> +	}
>  }
> 
> -static void
> -iavf_dev_watchdog_enable(struct iavf_adapter *adapter __rte_unused) -{ -
> #if (IAVF_DEV_WATCHDOG_PERIOD > 0)
> -	PMD_DRV_LOG(INFO, "Enabling device watchdog");
> -	adapter->vf.watchdog_enabled = true;
> -	if (rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
> -			&iavf_dev_watchdog, (void *)adapter))
> -		PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
> -#endif
> +void
> +iavf_dev_watchdog_enable(struct iavf_adapter *adapter) {
> +	if (adapter->devargs.watchdog_period && !adapter-
> >vf.watchdog_enabled) {
> +		PMD_DRV_LOG(INFO, "Enabling device watchdog, period
> is %dμs",
> +					adapter->devargs.watchdog_period);
> +		adapter->vf.watchdog_enabled = true;
> +		if (rte_eal_alarm_set(adapter->devargs.watchdog_period,
> +					&iavf_dev_watchdog, (void
> *)adapter))
> +			PMD_DRV_LOG(ERR, "Failed to enabled device
> watchdog");
> +	} else {
> +		PMD_DRV_LOG(INFO, "Device watchdog is disabled");
> +	}
>  }
> 
> -static void
> -iavf_dev_watchdog_disable(struct iavf_adapter *adapter __rte_unused)
> +void
> +iavf_dev_watchdog_disable(struct iavf_adapter *adapter)
>  {
> -#if (IAVF_DEV_WATCHDOG_PERIOD > 0)
> -	PMD_DRV_LOG(INFO, "Disabling device watchdog");
> -	adapter->vf.watchdog_enabled = false;
> -#endif
> +	if (adapter->devargs.watchdog_period && adapter-
> >vf.watchdog_enabled) {
> +		PMD_DRV_LOG(INFO, "Disabling device watchdog");
> +		adapter->vf.watchdog_enabled = false;
> +	}
>  }
> 
>  static int
> @@ -2201,6 +2209,25 @@ parse_u16(__rte_unused const char *key, const
> char *value, void *args)
>  	return 0;
>  }
> 
> +static int
> +iavf_parse_watchdog_period(__rte_unused const char *key, const char
> +*value, void *args) {
> +	int *num = (int *)args;
> +	int tmp;
> +
> +	errno = 0;
> +	tmp = atoi(value);
> +	if (tmp < 0) {
> +		PMD_DRV_LOG(WARNING, "%s: \"%s\" is not greater than or
> equal to zero",
> +				key, value);
> +		return -1;
> +	}
> +
> +	*num = tmp;
> +
> +	return 0;
> +}
> +
>  static int iavf_parse_devargs(struct rte_eth_dev *dev)  {
>  	struct iavf_adapter *ad =
> @@ -2208,6 +2235,7 @@ static int iavf_parse_devargs(struct rte_eth_dev
> *dev)
>  	struct rte_devargs *devargs = dev->device->devargs;
>  	struct rte_kvargs *kvlist;
>  	int ret;
> +	int watchdog_period = -1;
> 
>  	if (!devargs)
>  		return 0;
> @@ -2232,6 +2260,15 @@ static int iavf_parse_devargs(struct rte_eth_dev
> *dev)
>  	if (ret)
>  		goto bail;
> 
> +	ret = rte_kvargs_process(kvlist, IAVF_RESET_WATCHDOG_ARG,
> +				 &iavf_parse_watchdog_period,
> &watchdog_period);
> +	if (ret)
> +		goto bail;
> +	if (watchdog_period == -1)
> +		ad->devargs.watchdog_period =
> IAVF_DEV_WATCHDOG_PERIOD;
> +	else
> +		ad->devargs.watchdog_period = watchdog_period;
> +
>  	if (ad->devargs.quanta_size != 0 &&
>  	    (ad->devargs.quanta_size < 256 || ad->devargs.quanta_size > 4096
> ||
>  	     ad->devargs.quanta_size & 0x40)) { diff --git
> a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index
> 9adaadb173..f2d7331f55 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -256,6 +256,12 @@ iavf_read_msg_from_pf(struct iavf_adapter
> *adapter, uint16_t buf_len,
>  				vf->link_speed =
> iavf_convert_link_speed(speed);
>  			}
>  			iavf_dev_link_update(vf->eth_dev, 0);
> +			if (vf->link_up && !vf->vf_reset) {
> +				iavf_dev_watchdog_disable(adapter);
> +			} else {
> +				if (!vf->link_up)
> +					iavf_dev_watchdog_enable(adapter);
> +			}
>  			PMD_DRV_LOG(INFO, "Link status update:%s",
>  					vf->link_up ? "up" : "down");
>  			break;
> @@ -433,9 +439,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev,
> uint8_t *msg,
>  	switch (pf_msg->event) {
>  	case VIRTCHNL_EVENT_RESET_IMPENDING:
>  		PMD_DRV_LOG(DEBUG,
> "VIRTCHNL_EVENT_RESET_IMPENDING event");
> -		vf->vf_reset = true;
> -		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
> -					      NULL, 0);
> +		vf->link_up = false;
> +		if (!vf->vf_reset) {
> +			vf->vf_reset = true;
> +			iavf_dev_event_post(dev,
> RTE_ETH_EVENT_INTR_RESET,
> +				NULL, 0);
> +		}
>  		break;
>  	case VIRTCHNL_EVENT_LINK_CHANGE:
>  		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE
> event"); @@ -449,6 +458,12 @@ iavf_handle_pf_event_msg(struct
> rte_eth_dev *dev, uint8_t *msg,
>  			vf->link_speed = iavf_convert_link_speed(speed);
>  		}
>  		iavf_dev_link_update(dev, 0);
> +		if (vf->link_up && !vf->vf_reset) {
> +			iavf_dev_watchdog_disable(adapter);
> +		} else {
> +			if (!vf->link_up)
> +				iavf_dev_watchdog_enable(adapter);
> +		}
>  		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL,
> 0);
>  		break;
>  	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
> --
> 2.25.1


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

* [PATCH v3] net/iavf: add devargs to control watchdog
  2023-05-05  2:27 ` [PATCH v2] net/iavf: add devargs to control watchdog Zhichao Zeng
  2023-05-15  2:02   ` Zhang, Qi Z
@ 2023-05-15  5:47   ` Zhichao Zeng
  2023-05-15  6:19     ` Zhang, Qi Z
  2023-05-15  6:54     ` [PATCH v4] " Zhichao Zeng
  1 sibling, 2 replies; 8+ messages in thread
From: Zhichao Zeng @ 2023-05-15  5:47 UTC (permalink / raw)
  To: dev; +Cc: qi.z.zhang, Zhichao Zeng, Qiming Yang, Jingjing Wu, Beilei Xing

This patch enables the watchdog to detect VF FLR when the link state
changes to down, and the default period is 2000us as defined by
IAVF_DEV_WATCHDOG_PERIOD.

In addition, the 'watchdog_period' devargs was added to adjust the watchdog
period(microseconds), or set to 0 to disable the watchdog.

Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>

---
v3: add usage of new devarg in ice.rst
---
v2: enables watchdog when link status changes to down
---
 doc/guides/nics/ice.rst        | 13 ++++++
 drivers/net/iavf/iavf.h        |  5 ++-
 drivers/net/iavf/iavf_ethdev.c | 81 +++++++++++++++++++++++++---------
 drivers/net/iavf/iavf_vchnl.c  | 21 +++++++--
 4 files changed, 94 insertions(+), 26 deletions(-)

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index c351c6bd74..3cf038ede3 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -331,6 +331,19 @@ Additional Options
 
     -a 18:01.0,cap=dcf,acl=off
 
+- ``Control IAVF reset watchdog``
+
+  By default, the reset watchdog is enabled when link state changes to down.
+  The default period is 2000us, defined by ``IAVF_DEV_WATCHDOG_PERIOD``.
+  The user can set ``watchdog_period`` to adjust the watchdog period
+  (microseconds), or set it to 0 to disable the watchdog.
+
+    -a 18:01.0,watchdog_period=5000 (change period to 5000 microseconds)
+
+    or
+
+    -a 18:01.0,watchdog_period=0 (disable reset watchdog)
+
 .. _figure_ice_dcf:
 
 .. figure:: img/ice_dcf.*
diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index aa18650ffa..98861e4242 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -31,7 +31,7 @@
 
 #define IAVF_NUM_MACADDR_MAX      64
 
-#define IAVF_DEV_WATCHDOG_PERIOD     0
+#define IAVF_DEV_WATCHDOG_PERIOD     2000 /* microseconds, set 0 to disable*/
 
 #define IAVF_DEFAULT_RX_PTHRESH      8
 #define IAVF_DEFAULT_RX_HTHRESH      8
@@ -304,6 +304,7 @@ struct iavf_devargs {
 	uint8_t proto_xtr_dflt;
 	uint8_t proto_xtr[IAVF_MAX_QUEUE_NUM];
 	uint16_t quanta_size;
+	uint32_t watchdog_period;
 };
 
 struct iavf_security_ctx;
@@ -498,4 +499,6 @@ int iavf_flow_unsub(struct iavf_adapter *adapter,
 		    struct iavf_fsub_conf *filter);
 int iavf_flow_sub_check(struct iavf_adapter *adapter,
 			struct iavf_fsub_conf *filter);
+void iavf_dev_watchdog_enable(struct iavf_adapter *adapter);
+void iavf_dev_watchdog_disable(struct iavf_adapter *adapter);
 #endif /* _IAVF_ETHDEV_H_ */
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index f6d68403ce..4cf4eb20cd 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -36,6 +36,7 @@
 /* devargs */
 #define IAVF_PROTO_XTR_ARG         "proto_xtr"
 #define IAVF_QUANTA_SIZE_ARG       "quanta_size"
+#define IAVF_RESET_WATCHDOG_ARG    "watchdog_period"
 
 uint64_t iavf_timestamp_dynflag;
 int iavf_timestamp_dynfield_offset = -1;
@@ -43,6 +44,7 @@ int iavf_timestamp_dynfield_offset = -1;
 static const char * const iavf_valid_args[] = {
 	IAVF_PROTO_XTR_ARG,
 	IAVF_QUANTA_SIZE_ARG,
+	IAVF_RESET_WATCHDOG_ARG,
 	NULL
 };
 
@@ -301,40 +303,46 @@ iavf_dev_watchdog(void *cb_arg)
 
 			/* enter reset state with VFLR event */
 			adapter->vf.vf_reset = true;
+			adapter->vf.link_up = false;
 
 			rte_eth_dev_callback_process(adapter->vf.eth_dev,
 				RTE_ETH_EVENT_INTR_RESET, NULL);
 		}
 	}
 
-	/* re-alarm watchdog */
-	rc = rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
-			&iavf_dev_watchdog, cb_arg);
+	if (adapter->devargs.watchdog_period) {
+		/* re-alarm watchdog */
+		rc = rte_eal_alarm_set(adapter->devargs.watchdog_period,
+					&iavf_dev_watchdog, cb_arg);
 
-	if (rc)
-		PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
-			adapter->vf.eth_dev->data->name);
+		if (rc)
+			PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
+				adapter->vf.eth_dev->data->name);
+	}
 }
 
-static void
-iavf_dev_watchdog_enable(struct iavf_adapter *adapter __rte_unused)
-{
-#if (IAVF_DEV_WATCHDOG_PERIOD > 0)
-	PMD_DRV_LOG(INFO, "Enabling device watchdog");
-	adapter->vf.watchdog_enabled = true;
-	if (rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
-			&iavf_dev_watchdog, (void *)adapter))
-		PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
-#endif
+void
+iavf_dev_watchdog_enable(struct iavf_adapter *adapter)
+{
+	if (adapter->devargs.watchdog_period && !adapter->vf.watchdog_enabled) {
+		PMD_DRV_LOG(INFO, "Enabling device watchdog, period is %dμs",
+					adapter->devargs.watchdog_period);
+		adapter->vf.watchdog_enabled = true;
+		if (rte_eal_alarm_set(adapter->devargs.watchdog_period,
+					&iavf_dev_watchdog, (void *)adapter))
+			PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
+	} else {
+		PMD_DRV_LOG(INFO, "Device watchdog is disabled");
+	}
 }
 
-static void
-iavf_dev_watchdog_disable(struct iavf_adapter *adapter __rte_unused)
+void
+iavf_dev_watchdog_disable(struct iavf_adapter *adapter)
 {
-#if (IAVF_DEV_WATCHDOG_PERIOD > 0)
-	PMD_DRV_LOG(INFO, "Disabling device watchdog");
-	adapter->vf.watchdog_enabled = false;
-#endif
+	if (adapter->devargs.watchdog_period && adapter->vf.watchdog_enabled) {
+		PMD_DRV_LOG(INFO, "Disabling device watchdog");
+		adapter->vf.watchdog_enabled = false;
+	}
 }
 
 static int
@@ -2201,6 +2209,25 @@ parse_u16(__rte_unused const char *key, const char *value, void *args)
 	return 0;
 }
 
+static int
+iavf_parse_watchdog_period(__rte_unused const char *key, const char *value, void *args)
+{
+	int *num = (int *)args;
+	int tmp;
+
+	errno = 0;
+	tmp = atoi(value);
+	if (tmp < 0) {
+		PMD_DRV_LOG(WARNING, "%s: \"%s\" is not greater than or equal to zero",
+				key, value);
+		return -1;
+	}
+
+	*num = tmp;
+
+	return 0;
+}
+
 static int iavf_parse_devargs(struct rte_eth_dev *dev)
 {
 	struct iavf_adapter *ad =
@@ -2208,6 +2235,7 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	struct rte_devargs *devargs = dev->device->devargs;
 	struct rte_kvargs *kvlist;
 	int ret;
+	int watchdog_period = -1;
 
 	if (!devargs)
 		return 0;
@@ -2232,6 +2260,15 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	if (ret)
 		goto bail;
 
+	ret = rte_kvargs_process(kvlist, IAVF_RESET_WATCHDOG_ARG,
+				 &iavf_parse_watchdog_period, &watchdog_period);
+	if (ret)
+		goto bail;
+	if (watchdog_period == -1)
+		ad->devargs.watchdog_period = IAVF_DEV_WATCHDOG_PERIOD;
+	else
+		ad->devargs.watchdog_period = watchdog_period;
+
 	if (ad->devargs.quanta_size != 0 &&
 	    (ad->devargs.quanta_size < 256 || ad->devargs.quanta_size > 4096 ||
 	     ad->devargs.quanta_size & 0x40)) {
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 9adaadb173..f2d7331f55 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -256,6 +256,12 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 				vf->link_speed = iavf_convert_link_speed(speed);
 			}
 			iavf_dev_link_update(vf->eth_dev, 0);
+			if (vf->link_up && !vf->vf_reset) {
+				iavf_dev_watchdog_disable(adapter);
+			} else {
+				if (!vf->link_up)
+					iavf_dev_watchdog_enable(adapter);
+			}
 			PMD_DRV_LOG(INFO, "Link status update:%s",
 					vf->link_up ? "up" : "down");
 			break;
@@ -433,9 +439,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 	switch (pf_msg->event) {
 	case VIRTCHNL_EVENT_RESET_IMPENDING:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
-		vf->vf_reset = true;
-		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
-					      NULL, 0);
+		vf->link_up = false;
+		if (!vf->vf_reset) {
+			vf->vf_reset = true;
+			iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
+				NULL, 0);
+		}
 		break;
 	case VIRTCHNL_EVENT_LINK_CHANGE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
@@ -449,6 +458,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 			vf->link_speed = iavf_convert_link_speed(speed);
 		}
 		iavf_dev_link_update(dev, 0);
+		if (vf->link_up && !vf->vf_reset) {
+			iavf_dev_watchdog_disable(adapter);
+		} else {
+			if (!vf->link_up)
+				iavf_dev_watchdog_enable(adapter);
+		}
 		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL, 0);
 		break;
 	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
-- 
2.34.1


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

* RE: [PATCH v3] net/iavf: add devargs to control watchdog
  2023-05-15  5:47   ` [PATCH v3] " Zhichao Zeng
@ 2023-05-15  6:19     ` Zhang, Qi Z
  2023-05-15  6:54     ` [PATCH v4] " Zhichao Zeng
  1 sibling, 0 replies; 8+ messages in thread
From: Zhang, Qi Z @ 2023-05-15  6:19 UTC (permalink / raw)
  To: Zeng, ZhichaoX, dev; +Cc: Yang, Qiming, Wu, Jingjing, Xing, Beilei



> -----Original Message-----
> From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> Sent: Monday, May 15, 2023 1:48 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zeng, ZhichaoX
> <zhichaox.zeng@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: [PATCH v3] net/iavf: add devargs to control watchdog
> 
> This patch enables the watchdog to detect VF FLR when the link state
> changes to down, and the default period is 2000us as defined by
> IAVF_DEV_WATCHDOG_PERIOD.
> 
> In addition, the 'watchdog_period' devargs was added to adjust the
> watchdog period(microseconds), or set to 0 to disable the watchdog.
> 
> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> 
> ---
> v3: add usage of new devarg in ice.rst

My bad in previous comment, this is for AVF driver, it should be added in intel_vf.rst but not ice.rst.

> ---
> v2: enables watchdog when link status changes to down
> ---
>  doc/guides/nics/ice.rst        | 13 ++++++
>  drivers/net/iavf/iavf.h        |  5 ++-
>  drivers/net/iavf/iavf_ethdev.c | 81 +++++++++++++++++++++++++---------
>  drivers/net/iavf/iavf_vchnl.c  | 21 +++++++--
>  4 files changed, 94 insertions(+), 26 deletions(-)
> 
> diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst index
> c351c6bd74..3cf038ede3 100644
> --- a/doc/guides/nics/ice.rst
> +++ b/doc/guides/nics/ice.rst
> @@ -331,6 +331,19 @@ Additional Options
> 
>      -a 18:01.0,cap=dcf,acl=off
> 
> +- ``Control IAVF reset watchdog``
> +
> +  By default, the reset watchdog is enabled when link state changes to
> down.
> +  The default period is 2000us, defined by
> ``IAVF_DEV_WATCHDOG_PERIOD``.
> +  The user can set ``watchdog_period`` to adjust the watchdog period
> + (microseconds), or set it to 0 to disable the watchdog.
> +
> +    -a 18:01.0,watchdog_period=5000 (change period to 5000
> + microseconds)
> +
> +    or
> +
> +    -a 18:01.0,watchdog_period=0 (disable reset watchdog)
> +
>  .. _figure_ice_dcf:
> 
>  .. figure:: img/ice_dcf.*
> diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
> aa18650ffa..98861e4242 100644
> --- a/drivers/net/iavf/iavf.h
> +++ b/drivers/net/iavf/iavf.h
> @@ -31,7 +31,7 @@
> 
>  #define IAVF_NUM_MACADDR_MAX      64
> 
> -#define IAVF_DEV_WATCHDOG_PERIOD     0
> +#define IAVF_DEV_WATCHDOG_PERIOD     2000 /* microseconds, set 0 to
> disable*/
> 
>  #define IAVF_DEFAULT_RX_PTHRESH      8
>  #define IAVF_DEFAULT_RX_HTHRESH      8
> @@ -304,6 +304,7 @@ struct iavf_devargs {
>  	uint8_t proto_xtr_dflt;
>  	uint8_t proto_xtr[IAVF_MAX_QUEUE_NUM];
>  	uint16_t quanta_size;
> +	uint32_t watchdog_period;
>  };
> 
>  struct iavf_security_ctx;
> @@ -498,4 +499,6 @@ int iavf_flow_unsub(struct iavf_adapter *adapter,
>  		    struct iavf_fsub_conf *filter);
>  int iavf_flow_sub_check(struct iavf_adapter *adapter,
>  			struct iavf_fsub_conf *filter);
> +void iavf_dev_watchdog_enable(struct iavf_adapter *adapter); void
> +iavf_dev_watchdog_disable(struct iavf_adapter *adapter);
>  #endif /* _IAVF_ETHDEV_H_ */
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index f6d68403ce..4cf4eb20cd 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -36,6 +36,7 @@
>  /* devargs */
>  #define IAVF_PROTO_XTR_ARG         "proto_xtr"
>  #define IAVF_QUANTA_SIZE_ARG       "quanta_size"
> +#define IAVF_RESET_WATCHDOG_ARG    "watchdog_period"
> 
>  uint64_t iavf_timestamp_dynflag;
>  int iavf_timestamp_dynfield_offset = -1; @@ -43,6 +44,7 @@ int
> iavf_timestamp_dynfield_offset = -1;  static const char * const
> iavf_valid_args[] = {
>  	IAVF_PROTO_XTR_ARG,
>  	IAVF_QUANTA_SIZE_ARG,
> +	IAVF_RESET_WATCHDOG_ARG,
>  	NULL
>  };
> 
> @@ -301,40 +303,46 @@ iavf_dev_watchdog(void *cb_arg)
> 
>  			/* enter reset state with VFLR event */
>  			adapter->vf.vf_reset = true;
> +			adapter->vf.link_up = false;
> 
>  			rte_eth_dev_callback_process(adapter->vf.eth_dev,
>  				RTE_ETH_EVENT_INTR_RESET, NULL);
>  		}
>  	}
> 
> -	/* re-alarm watchdog */
> -	rc = rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
> -			&iavf_dev_watchdog, cb_arg);
> +	if (adapter->devargs.watchdog_period) {
> +		/* re-alarm watchdog */
> +		rc = rte_eal_alarm_set(adapter->devargs.watchdog_period,
> +					&iavf_dev_watchdog, cb_arg);
> 
> -	if (rc)
> -		PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog
> alarm",
> -			adapter->vf.eth_dev->data->name);
> +		if (rc)
> +			PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device
> watchdog alarm",
> +				adapter->vf.eth_dev->data->name);
> +	}
>  }
> 
> -static void
> -iavf_dev_watchdog_enable(struct iavf_adapter *adapter __rte_unused) -{ -
> #if (IAVF_DEV_WATCHDOG_PERIOD > 0)
> -	PMD_DRV_LOG(INFO, "Enabling device watchdog");
> -	adapter->vf.watchdog_enabled = true;
> -	if (rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
> -			&iavf_dev_watchdog, (void *)adapter))
> -		PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
> -#endif
> +void
> +iavf_dev_watchdog_enable(struct iavf_adapter *adapter) {
> +	if (adapter->devargs.watchdog_period && !adapter-
> >vf.watchdog_enabled) {
> +		PMD_DRV_LOG(INFO, "Enabling device watchdog, period
> is %dμs",
> +					adapter->devargs.watchdog_period);
> +		adapter->vf.watchdog_enabled = true;
> +		if (rte_eal_alarm_set(adapter->devargs.watchdog_period,
> +					&iavf_dev_watchdog, (void
> *)adapter))
> +			PMD_DRV_LOG(ERR, "Failed to enabled device
> watchdog");
> +	} else {
> +		PMD_DRV_LOG(INFO, "Device watchdog is disabled");
> +	}
>  }
> 
> -static void
> -iavf_dev_watchdog_disable(struct iavf_adapter *adapter __rte_unused)
> +void
> +iavf_dev_watchdog_disable(struct iavf_adapter *adapter)
>  {
> -#if (IAVF_DEV_WATCHDOG_PERIOD > 0)
> -	PMD_DRV_LOG(INFO, "Disabling device watchdog");
> -	adapter->vf.watchdog_enabled = false;
> -#endif
> +	if (adapter->devargs.watchdog_period && adapter-
> >vf.watchdog_enabled) {
> +		PMD_DRV_LOG(INFO, "Disabling device watchdog");
> +		adapter->vf.watchdog_enabled = false;
> +	}
>  }
> 
>  static int
> @@ -2201,6 +2209,25 @@ parse_u16(__rte_unused const char *key, const
> char *value, void *args)
>  	return 0;
>  }
> 
> +static int
> +iavf_parse_watchdog_period(__rte_unused const char *key, const char
> +*value, void *args) {
> +	int *num = (int *)args;
> +	int tmp;
> +
> +	errno = 0;
> +	tmp = atoi(value);
> +	if (tmp < 0) {
> +		PMD_DRV_LOG(WARNING, "%s: \"%s\" is not greater than or
> equal to zero",
> +				key, value);
> +		return -1;
> +	}
> +
> +	*num = tmp;
> +
> +	return 0;
> +}
> +
>  static int iavf_parse_devargs(struct rte_eth_dev *dev)  {
>  	struct iavf_adapter *ad =
> @@ -2208,6 +2235,7 @@ static int iavf_parse_devargs(struct rte_eth_dev
> *dev)
>  	struct rte_devargs *devargs = dev->device->devargs;
>  	struct rte_kvargs *kvlist;
>  	int ret;
> +	int watchdog_period = -1;
> 
>  	if (!devargs)
>  		return 0;
> @@ -2232,6 +2260,15 @@ static int iavf_parse_devargs(struct rte_eth_dev
> *dev)
>  	if (ret)
>  		goto bail;
> 
> +	ret = rte_kvargs_process(kvlist, IAVF_RESET_WATCHDOG_ARG,
> +				 &iavf_parse_watchdog_period,
> &watchdog_period);
> +	if (ret)
> +		goto bail;
> +	if (watchdog_period == -1)
> +		ad->devargs.watchdog_period =
> IAVF_DEV_WATCHDOG_PERIOD;
> +	else
> +		ad->devargs.watchdog_period = watchdog_period;
> +
>  	if (ad->devargs.quanta_size != 0 &&
>  	    (ad->devargs.quanta_size < 256 || ad->devargs.quanta_size > 4096
> ||
>  	     ad->devargs.quanta_size & 0x40)) { diff --git
> a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index
> 9adaadb173..f2d7331f55 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -256,6 +256,12 @@ iavf_read_msg_from_pf(struct iavf_adapter
> *adapter, uint16_t buf_len,
>  				vf->link_speed =
> iavf_convert_link_speed(speed);
>  			}
>  			iavf_dev_link_update(vf->eth_dev, 0);
> +			if (vf->link_up && !vf->vf_reset) {
> +				iavf_dev_watchdog_disable(adapter);
> +			} else {
> +				if (!vf->link_up)
> +					iavf_dev_watchdog_enable(adapter);
> +			}
>  			PMD_DRV_LOG(INFO, "Link status update:%s",
>  					vf->link_up ? "up" : "down");
>  			break;
> @@ -433,9 +439,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev,
> uint8_t *msg,
>  	switch (pf_msg->event) {
>  	case VIRTCHNL_EVENT_RESET_IMPENDING:
>  		PMD_DRV_LOG(DEBUG,
> "VIRTCHNL_EVENT_RESET_IMPENDING event");
> -		vf->vf_reset = true;
> -		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
> -					      NULL, 0);
> +		vf->link_up = false;
> +		if (!vf->vf_reset) {
> +			vf->vf_reset = true;
> +			iavf_dev_event_post(dev,
> RTE_ETH_EVENT_INTR_RESET,
> +				NULL, 0);
> +		}
>  		break;
>  	case VIRTCHNL_EVENT_LINK_CHANGE:
>  		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE
> event"); @@ -449,6 +458,12 @@ iavf_handle_pf_event_msg(struct
> rte_eth_dev *dev, uint8_t *msg,
>  			vf->link_speed = iavf_convert_link_speed(speed);
>  		}
>  		iavf_dev_link_update(dev, 0);
> +		if (vf->link_up && !vf->vf_reset) {
> +			iavf_dev_watchdog_disable(adapter);
> +		} else {
> +			if (!vf->link_up)
> +				iavf_dev_watchdog_enable(adapter);
> +		}
>  		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL,
> 0);
>  		break;
>  	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
> --
> 2.34.1


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

* [PATCH v4] net/iavf: add devargs to control watchdog
  2023-05-15  5:47   ` [PATCH v3] " Zhichao Zeng
  2023-05-15  6:19     ` Zhang, Qi Z
@ 2023-05-15  6:54     ` Zhichao Zeng
  2023-05-17  0:51       ` Zhang, Qi Z
  1 sibling, 1 reply; 8+ messages in thread
From: Zhichao Zeng @ 2023-05-15  6:54 UTC (permalink / raw)
  To: dev
  Cc: qi.z.zhang, Zhichao Zeng, Qiming Yang, Wenjun Wu, Simei Su,
	Yuying Zhang, Beilei Xing, Jingjing Wu

This patch enables the watchdog to detect VF FLR when the link state
changes to down, and the default period is 2000us as defined by
IAVF_DEV_WATCHDOG_PERIOD.

In addition, the 'watchdog_period' devargs was added to adjust the watchdog
period(microseconds), or set to 0 to disable the watchdog.

Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>

---
v4: add usage of new devarg in intel_vf.rst
---
v3: add usage of new devarg in ice.rst
---
v2: enables watchdog when link status changes to down
---
 doc/guides/nics/intel_vf.rst   |  5 +++
 drivers/net/iavf/iavf.h        |  5 ++-
 drivers/net/iavf/iavf_ethdev.c | 81 +++++++++++++++++++++++++---------
 drivers/net/iavf/iavf_vchnl.c  | 21 +++++++--
 4 files changed, 86 insertions(+), 26 deletions(-)

diff --git a/doc/guides/nics/intel_vf.rst b/doc/guides/nics/intel_vf.rst
index ab97609e5b..d365dbc185 100644
--- a/doc/guides/nics/intel_vf.rst
+++ b/doc/guides/nics/intel_vf.rst
@@ -96,6 +96,11 @@ For more detail on SR-IOV, please refer to the following documents:
     parameter ``quanta_size`` like ``-a 18:00.0,quanta_size=2048``. The default value is 1024, and quanta size should be
     set as the product of 64 in legacy host interface mode.
 
+    When IAVF is backed by an Intel® E810 device or an Intel® 700 Series Ethernet device, the reset watchdog is enabled
+    when link state changes to down. The default period is 2000us, defined by ``IAVF_DEV_WATCHDOG_PERIOD``.
+    Set ``devargs`` parameter ``watchdog_period`` to adjust the watchdog period in microseconds, or set it to 0 to disable the watchdog,
+    for example, ``-a 18:01.0,watchdog_period=5000`` or ``-a 18:01.0,watchdog_period=0``.
+
 The PCIE host-interface of Intel Ethernet Switch FM10000 Series VF infrastructure
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index aa18650ffa..98861e4242 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -31,7 +31,7 @@
 
 #define IAVF_NUM_MACADDR_MAX      64
 
-#define IAVF_DEV_WATCHDOG_PERIOD     0
+#define IAVF_DEV_WATCHDOG_PERIOD     2000 /* microseconds, set 0 to disable*/
 
 #define IAVF_DEFAULT_RX_PTHRESH      8
 #define IAVF_DEFAULT_RX_HTHRESH      8
@@ -304,6 +304,7 @@ struct iavf_devargs {
 	uint8_t proto_xtr_dflt;
 	uint8_t proto_xtr[IAVF_MAX_QUEUE_NUM];
 	uint16_t quanta_size;
+	uint32_t watchdog_period;
 };
 
 struct iavf_security_ctx;
@@ -498,4 +499,6 @@ int iavf_flow_unsub(struct iavf_adapter *adapter,
 		    struct iavf_fsub_conf *filter);
 int iavf_flow_sub_check(struct iavf_adapter *adapter,
 			struct iavf_fsub_conf *filter);
+void iavf_dev_watchdog_enable(struct iavf_adapter *adapter);
+void iavf_dev_watchdog_disable(struct iavf_adapter *adapter);
 #endif /* _IAVF_ETHDEV_H_ */
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index f6d68403ce..4cf4eb20cd 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -36,6 +36,7 @@
 /* devargs */
 #define IAVF_PROTO_XTR_ARG         "proto_xtr"
 #define IAVF_QUANTA_SIZE_ARG       "quanta_size"
+#define IAVF_RESET_WATCHDOG_ARG    "watchdog_period"
 
 uint64_t iavf_timestamp_dynflag;
 int iavf_timestamp_dynfield_offset = -1;
@@ -43,6 +44,7 @@ int iavf_timestamp_dynfield_offset = -1;
 static const char * const iavf_valid_args[] = {
 	IAVF_PROTO_XTR_ARG,
 	IAVF_QUANTA_SIZE_ARG,
+	IAVF_RESET_WATCHDOG_ARG,
 	NULL
 };
 
@@ -301,40 +303,46 @@ iavf_dev_watchdog(void *cb_arg)
 
 			/* enter reset state with VFLR event */
 			adapter->vf.vf_reset = true;
+			adapter->vf.link_up = false;
 
 			rte_eth_dev_callback_process(adapter->vf.eth_dev,
 				RTE_ETH_EVENT_INTR_RESET, NULL);
 		}
 	}
 
-	/* re-alarm watchdog */
-	rc = rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
-			&iavf_dev_watchdog, cb_arg);
+	if (adapter->devargs.watchdog_period) {
+		/* re-alarm watchdog */
+		rc = rte_eal_alarm_set(adapter->devargs.watchdog_period,
+					&iavf_dev_watchdog, cb_arg);
 
-	if (rc)
-		PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
-			adapter->vf.eth_dev->data->name);
+		if (rc)
+			PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
+				adapter->vf.eth_dev->data->name);
+	}
 }
 
-static void
-iavf_dev_watchdog_enable(struct iavf_adapter *adapter __rte_unused)
-{
-#if (IAVF_DEV_WATCHDOG_PERIOD > 0)
-	PMD_DRV_LOG(INFO, "Enabling device watchdog");
-	adapter->vf.watchdog_enabled = true;
-	if (rte_eal_alarm_set(IAVF_DEV_WATCHDOG_PERIOD,
-			&iavf_dev_watchdog, (void *)adapter))
-		PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
-#endif
+void
+iavf_dev_watchdog_enable(struct iavf_adapter *adapter)
+{
+	if (adapter->devargs.watchdog_period && !adapter->vf.watchdog_enabled) {
+		PMD_DRV_LOG(INFO, "Enabling device watchdog, period is %dμs",
+					adapter->devargs.watchdog_period);
+		adapter->vf.watchdog_enabled = true;
+		if (rte_eal_alarm_set(adapter->devargs.watchdog_period,
+					&iavf_dev_watchdog, (void *)adapter))
+			PMD_DRV_LOG(ERR, "Failed to enabled device watchdog");
+	} else {
+		PMD_DRV_LOG(INFO, "Device watchdog is disabled");
+	}
 }
 
-static void
-iavf_dev_watchdog_disable(struct iavf_adapter *adapter __rte_unused)
+void
+iavf_dev_watchdog_disable(struct iavf_adapter *adapter)
 {
-#if (IAVF_DEV_WATCHDOG_PERIOD > 0)
-	PMD_DRV_LOG(INFO, "Disabling device watchdog");
-	adapter->vf.watchdog_enabled = false;
-#endif
+	if (adapter->devargs.watchdog_period && adapter->vf.watchdog_enabled) {
+		PMD_DRV_LOG(INFO, "Disabling device watchdog");
+		adapter->vf.watchdog_enabled = false;
+	}
 }
 
 static int
@@ -2201,6 +2209,25 @@ parse_u16(__rte_unused const char *key, const char *value, void *args)
 	return 0;
 }
 
+static int
+iavf_parse_watchdog_period(__rte_unused const char *key, const char *value, void *args)
+{
+	int *num = (int *)args;
+	int tmp;
+
+	errno = 0;
+	tmp = atoi(value);
+	if (tmp < 0) {
+		PMD_DRV_LOG(WARNING, "%s: \"%s\" is not greater than or equal to zero",
+				key, value);
+		return -1;
+	}
+
+	*num = tmp;
+
+	return 0;
+}
+
 static int iavf_parse_devargs(struct rte_eth_dev *dev)
 {
 	struct iavf_adapter *ad =
@@ -2208,6 +2235,7 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	struct rte_devargs *devargs = dev->device->devargs;
 	struct rte_kvargs *kvlist;
 	int ret;
+	int watchdog_period = -1;
 
 	if (!devargs)
 		return 0;
@@ -2232,6 +2260,15 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev)
 	if (ret)
 		goto bail;
 
+	ret = rte_kvargs_process(kvlist, IAVF_RESET_WATCHDOG_ARG,
+				 &iavf_parse_watchdog_period, &watchdog_period);
+	if (ret)
+		goto bail;
+	if (watchdog_period == -1)
+		ad->devargs.watchdog_period = IAVF_DEV_WATCHDOG_PERIOD;
+	else
+		ad->devargs.watchdog_period = watchdog_period;
+
 	if (ad->devargs.quanta_size != 0 &&
 	    (ad->devargs.quanta_size < 256 || ad->devargs.quanta_size > 4096 ||
 	     ad->devargs.quanta_size & 0x40)) {
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 9adaadb173..f2d7331f55 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -256,6 +256,12 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 				vf->link_speed = iavf_convert_link_speed(speed);
 			}
 			iavf_dev_link_update(vf->eth_dev, 0);
+			if (vf->link_up && !vf->vf_reset) {
+				iavf_dev_watchdog_disable(adapter);
+			} else {
+				if (!vf->link_up)
+					iavf_dev_watchdog_enable(adapter);
+			}
 			PMD_DRV_LOG(INFO, "Link status update:%s",
 					vf->link_up ? "up" : "down");
 			break;
@@ -433,9 +439,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 	switch (pf_msg->event) {
 	case VIRTCHNL_EVENT_RESET_IMPENDING:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
-		vf->vf_reset = true;
-		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
-					      NULL, 0);
+		vf->link_up = false;
+		if (!vf->vf_reset) {
+			vf->vf_reset = true;
+			iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_RESET,
+				NULL, 0);
+		}
 		break;
 	case VIRTCHNL_EVENT_LINK_CHANGE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
@@ -449,6 +458,12 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
 			vf->link_speed = iavf_convert_link_speed(speed);
 		}
 		iavf_dev_link_update(dev, 0);
+		if (vf->link_up && !vf->vf_reset) {
+			iavf_dev_watchdog_disable(adapter);
+		} else {
+			if (!vf->link_up)
+				iavf_dev_watchdog_enable(adapter);
+		}
 		iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL, 0);
 		break;
 	case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
-- 
2.34.1


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

* RE: [PATCH v4] net/iavf: add devargs to control watchdog
  2023-05-15  6:54     ` [PATCH v4] " Zhichao Zeng
@ 2023-05-17  0:51       ` Zhang, Qi Z
  0 siblings, 0 replies; 8+ messages in thread
From: Zhang, Qi Z @ 2023-05-17  0:51 UTC (permalink / raw)
  To: Zeng, ZhichaoX, dev
  Cc: Yang, Qiming, Wu, Wenjun1, Su, Simei, Zhang, Yuying, Xing,
	Beilei, Wu, Jingjing



> -----Original Message-----
> From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> Sent: Monday, May 15, 2023 2:54 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zeng, ZhichaoX
> <zhichaox.zeng@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Wu,
> Wenjun1 <wenjun1.wu@intel.com>; Su, Simei <simei.su@intel.com>; Zhang,
> Yuying <yuying.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>
> Subject: [PATCH v4] net/iavf: add devargs to control watchdog
> 
> This patch enables the watchdog to detect VF FLR when the link state
> changes to down, and the default period is 2000us as defined by
> IAVF_DEV_WATCHDOG_PERIOD.
> 
> In addition, the 'watchdog_period' devargs was added to adjust the
> watchdog period(microseconds), or set to 0 to disable the watchdog.
> 
> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi


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

end of thread, other threads:[~2023-05-17  0:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-18  5:42 [PATCH] net/iavf: add devargs to enable watchdog Zhichao Zeng
2023-04-24  2:33 ` Zhang, Qi Z
2023-05-05  2:27 ` [PATCH v2] net/iavf: add devargs to control watchdog Zhichao Zeng
2023-05-15  2:02   ` Zhang, Qi Z
2023-05-15  5:47   ` [PATCH v3] " Zhichao Zeng
2023-05-15  6:19     ` Zhang, Qi Z
2023-05-15  6:54     ` [PATCH v4] " Zhichao Zeng
2023-05-17  0:51       ` Zhang, Qi Z

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