DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* Re: [dpdk-dev] [PATCH] net/i40e: fix request queue fail in VF
  2019-07-18 13:09 ` Ye Xiaolong
@ 2019-07-18  7:05   ` Zhu, TaoX
  2019-07-18 14:05     ` Ye Xiaolong
  0 siblings, 1 reply; 8+ messages in thread
From: Zhu, TaoX @ 2019-07-18  7:05 UTC (permalink / raw)
  To: Ye, Xiaolong; +Cc: Xing, Beilei, Zhang, Qi Z, dev

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

Hi, Xiaolong
	I'm sorry, this is my first time submitting a patch.
	Attachment picture is my patch content and I try to generate fixes with git fixline. But I modified the code location to 'Not Committed Yet'. Should I use nearby id cebe3d7b3d?
	
Thanks,
Zhutao	
	


-----Original Message-----
From: Ye, Xiaolong 
Sent: Thursday, July 18, 2019 9:09 PM
To: Zhu, TaoX <taox.zhu@intel.com>
Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix request queue fail in VF

On 07/18, taox.zhu@intel.com wrote:
>From: Zhu Tao <taox.zhu@intel.com>
>
>When the VF configuration is larger than the number of queues reserved 
>by PF, VF sends the request queue command through admin queue. When PF 
>received this command, it may reset the VF and send a notification 
>before resetting. If this notification is read by the timed task alarm, 
>Task request queue will lost notification. This patch Mark vf_reset, 
>pend_msg flag just as task request queue has received notification in 
>task alarm.

Please add fixes tag and cc stable.


Thanks,
Xiaolong

>
>Signed-off-by: Zhu Tao <taox.zhu@intel.com>
>---
> drivers/net/i40e/i40e_ethdev_vf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
>b/drivers/net/i40e/i40e_ethdev_vf.c
>index 5be32b069..86dfda1c0 100644
>--- a/drivers/net/i40e/i40e_ethdev_vf.c
>+++ b/drivers/net/i40e/i40e_ethdev_vf.c
>@@ -1332,6 +1332,10 @@ i40evf_handle_pf_event(struct rte_eth_dev *dev, uint8_t *msg,
> 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
> 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
> 					      NULL);
>+		if (vf->vf_reset == false) {
>+			vf->vf_reset = true;
>+			vf->pend_msg |= PFMSG_RESET_IMPENDING;
>+		}
> 		break;
> 	case VIRTCHNL_EVENT_LINK_CHANGE:
> 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
>--
>2.17.1
>

[-- Attachment #2: Snipaste_2019-07-18_14-52-44.png --]
[-- Type: image/png, Size: 82568 bytes --]

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

* Re: [dpdk-dev] [PATCH] net/i40e: fix request queue fail in VF
  2019-07-18 14:53 [dpdk-dev] [PATCH] net/i40e: fix request queue fail in VF taox.zhu
@ 2019-07-18 13:09 ` Ye Xiaolong
  2019-07-18  7:05   ` Zhu, TaoX
  2019-07-19  3:18 ` [dpdk-dev] [PATCH v2] " taox.zhu
  1 sibling, 1 reply; 8+ messages in thread
From: Ye Xiaolong @ 2019-07-18 13:09 UTC (permalink / raw)
  To: taox.zhu; +Cc: beilei.xing, qi.z.zhang, dev

On 07/18, taox.zhu@intel.com wrote:
>From: Zhu Tao <taox.zhu@intel.com>
>
>When the VF configuration is larger than the number of queues reserved
>by PF, VF sends the request queue command through admin queue. When PF
>received this command, it may reset the VF and send a notification
>before resetting. If this notification is read by the timed task alarm,
>Task request queue will lost notification. This patch Mark vf_reset,
>pend_msg flag just as task request queue has received notification in
>task alarm.

Please add fixes tag and cc stable.


Thanks,
Xiaolong

>
>Signed-off-by: Zhu Tao <taox.zhu@intel.com>
>---
> drivers/net/i40e/i40e_ethdev_vf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
>index 5be32b069..86dfda1c0 100644
>--- a/drivers/net/i40e/i40e_ethdev_vf.c
>+++ b/drivers/net/i40e/i40e_ethdev_vf.c
>@@ -1332,6 +1332,10 @@ i40evf_handle_pf_event(struct rte_eth_dev *dev, uint8_t *msg,
> 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
> 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
> 					      NULL);
>+		if (vf->vf_reset == false) {
>+			vf->vf_reset = true;
>+			vf->pend_msg |= PFMSG_RESET_IMPENDING;
>+		}
> 		break;
> 	case VIRTCHNL_EVENT_LINK_CHANGE:
> 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
>-- 
>2.17.1
>

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

* Re: [dpdk-dev] [PATCH] net/i40e: fix request queue fail in VF
  2019-07-18  7:05   ` Zhu, TaoX
@ 2019-07-18 14:05     ` Ye Xiaolong
  0 siblings, 0 replies; 8+ messages in thread
From: Ye Xiaolong @ 2019-07-18 14:05 UTC (permalink / raw)
  To: Zhu, TaoX; +Cc: Xing, Beilei, Zhang, Qi Z, dev

On 07/18, Zhu, TaoX wrote:
>Hi, Xiaolong
>	I'm sorry, this is my first time submitting a patch.
>	Attachment picture is my patch content and I try to generate fixes with git fixline. But I modified the code location to 'Not Committed Yet'. Should I use nearby id cebe3d7b3d?
>	

Nope, the commit in the fixline should be the one that introduced the issue this
patch tries to solve.

Thanks,
Xiaolong

>Thanks,
>Zhutao	
>	
>
>
>-----Original Message-----
>From: Ye, Xiaolong 
>Sent: Thursday, July 18, 2019 9:09 PM
>To: Zhu, TaoX <taox.zhu@intel.com>
>Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
>Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix request queue fail in VF
>
>On 07/18, taox.zhu@intel.com wrote:
>>From: Zhu Tao <taox.zhu@intel.com>
>>
>>When the VF configuration is larger than the number of queues reserved 
>>by PF, VF sends the request queue command through admin queue. When PF 
>>received this command, it may reset the VF and send a notification 
>>before resetting. If this notification is read by the timed task alarm, 
>>Task request queue will lost notification. This patch Mark vf_reset, 
>>pend_msg flag just as task request queue has received notification in 
>>task alarm.
>
>Please add fixes tag and cc stable.
>
>
>Thanks,
>Xiaolong
>
>>
>>Signed-off-by: Zhu Tao <taox.zhu@intel.com>
>>---
>> drivers/net/i40e/i40e_ethdev_vf.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>>diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
>>b/drivers/net/i40e/i40e_ethdev_vf.c
>>index 5be32b069..86dfda1c0 100644
>>--- a/drivers/net/i40e/i40e_ethdev_vf.c
>>+++ b/drivers/net/i40e/i40e_ethdev_vf.c
>>@@ -1332,6 +1332,10 @@ i40evf_handle_pf_event(struct rte_eth_dev *dev, uint8_t *msg,
>> 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
>> 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
>> 					      NULL);
>>+		if (vf->vf_reset == false) {
>>+			vf->vf_reset = true;
>>+			vf->pend_msg |= PFMSG_RESET_IMPENDING;
>>+		}
>> 		break;
>> 	case VIRTCHNL_EVENT_LINK_CHANGE:
>> 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
>>--
>>2.17.1
>>



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

* [dpdk-dev] [PATCH] net/i40e: fix request queue fail in VF
@ 2019-07-18 14:53 taox.zhu
  2019-07-18 13:09 ` Ye Xiaolong
  2019-07-19  3:18 ` [dpdk-dev] [PATCH v2] " taox.zhu
  0 siblings, 2 replies; 8+ messages in thread
From: taox.zhu @ 2019-07-18 14:53 UTC (permalink / raw)
  To: beilei.xing, qi.z.zhang; +Cc: dev, Zhu Tao

From: Zhu Tao <taox.zhu@intel.com>

When the VF configuration is larger than the number of queues reserved
by PF, VF sends the request queue command through admin queue. When PF
received this command, it may reset the VF and send a notification
before resetting. If this notification is read by the timed task alarm,
Task request queue will lost notification. This patch Mark vf_reset,
pend_msg flag just as task request queue has received notification in
task alarm.

Signed-off-by: Zhu Tao <taox.zhu@intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 5be32b069..86dfda1c0 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1332,6 +1332,10 @@ i40evf_handle_pf_event(struct rte_eth_dev *dev, uint8_t *msg,
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
 					      NULL);
+		if (vf->vf_reset == false) {
+			vf->vf_reset = true;
+			vf->pend_msg |= PFMSG_RESET_IMPENDING;
+		}
 		break;
 	case VIRTCHNL_EVENT_LINK_CHANGE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2] net/i40e: fix request queue fail in VF
  2019-07-18 14:53 [dpdk-dev] [PATCH] net/i40e: fix request queue fail in VF taox.zhu
  2019-07-18 13:09 ` Ye Xiaolong
@ 2019-07-19  3:18 ` " taox.zhu
  2019-07-23  6:52   ` Zhang, Qi Z
  2019-07-24  8:32   ` [dpdk-dev] [PATCH v3] " taox.zhu
  1 sibling, 2 replies; 8+ messages in thread
From: taox.zhu @ 2019-07-19  3:18 UTC (permalink / raw)
  To: beilei.xing, qi.z.zhang; +Cc: dev, Zhu Tao, stable

From: Zhu Tao <taox.zhu@intel.com>

When the VF configuration is larger than the number of queues reserved
by PF, VF sends the request queue command through admin queue. When PF
received this command, it may reset the VF and send a notification
before resetting. If this notification is read by the timed task alarm,
Task request queue will lost notification. This patch Mark vf_reset,
pend_msg flag just as task request queue has received notification in
task alarm.

Fixes: 864a800d70 ("net/i40e: remove VF interrupt handler")
Fixes: ee653bd800 ("net/i40e: determine number of queues per VF at run time")
Cc: stable@dpdk.org

Signed-off-by: Zhu Tao <taox.zhu@intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 5be32b069..3b9740c08 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1332,6 +1332,10 @@ i40evf_handle_pf_event(struct rte_eth_dev *dev, uint8_t *msg,
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event");
 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
 					      NULL);
+		if (!vf->vf_reset) {
+			vf->vf_reset = true;
+			vf->pend_msg |= PFMSG_RESET_IMPENDING;
+		}
 		break;
 	case VIRTCHNL_EVENT_LINK_CHANGE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v2] net/i40e: fix request queue fail in VF
  2019-07-19  3:18 ` [dpdk-dev] [PATCH v2] " taox.zhu
@ 2019-07-23  6:52   ` Zhang, Qi Z
  2019-07-24  8:32   ` [dpdk-dev] [PATCH v3] " taox.zhu
  1 sibling, 0 replies; 8+ messages in thread
From: Zhang, Qi Z @ 2019-07-23  6:52 UTC (permalink / raw)
  To: Zhu, TaoX, Xing, Beilei; +Cc: dev, stable



> -----Original Message-----
> From: Zhu, TaoX
> Sent: Friday, July 19, 2019 11:18 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Zhu, TaoX <taox.zhu@intel.com>; stable@dpdk.org
> Subject: [PATCH v2] net/i40e: fix request queue fail in VF
> 
> From: Zhu Tao <taox.zhu@intel.com>
> 
> When the VF configuration is larger than the number of queues reserved by PF,
> VF sends the request queue command through admin queue. When PF
> received this command, it may reset the VF and send a notification before
> resetting. If this notification is read by the timed task alarm, Task request
> queue will lost notification. This patch Mark vf_reset, pend_msg flag just as
> task request queue has received notification in task alarm.
> 
> Fixes: 864a800d70 ("net/i40e: remove VF interrupt handler")
> Fixes: ee653bd800 ("net/i40e: determine number of queues per VF at run
> time")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zhu Tao <taox.zhu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev_vf.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index 5be32b069..3b9740c08 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -1332,6 +1332,10 @@ i40evf_handle_pf_event(struct rte_eth_dev *dev,
> uint8_t *msg,
>  		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING
> event");
>  		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
>  					      NULL);
> +		if (!vf->vf_reset) {
> +			vf->vf_reset = true;
> +			vf->pend_msg|= PFMSG_RESET_IMPENDING;

You current solution rely on we can always can get a dummy event after the VIRTCHNL_EVENT_RESET_IMPENDING event we needed, but I'm not sure if it is guaranteed

If the issue is due to the race condition between i40evf_execute_vf_cmd and i40evf_dev_alarm_handler
Why not just disable the adminq interrupt before i40evf_request_queues and enable it back after it?

> +		}
>  		break;
>  	case VIRTCHNL_EVENT_LINK_CHANGE:
>  		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
> --
> 2.17.1


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

* [dpdk-dev] [PATCH v3] net/i40e: fix request queue fail in VF
  2019-07-19  3:18 ` [dpdk-dev] [PATCH v2] " taox.zhu
  2019-07-23  6:52   ` Zhang, Qi Z
@ 2019-07-24  8:32   ` " taox.zhu
  2019-07-24 12:54     ` Zhang, Qi Z
  1 sibling, 1 reply; 8+ messages in thread
From: taox.zhu @ 2019-07-24  8:32 UTC (permalink / raw)
  To: beilei.xing, qi.z.zhang; +Cc: dev, Zhu Tao, stable

From: Zhu Tao <taox.zhu@intel.com>

When the VF configuration is larger than the number of queues reserved
by PF, VF sends the request queue command through admin queue. When PF
received this command, it may reset the VF and send a notification
before resetting. If this notification is read by the timed task alarm,
Task request queue will lost notification. This patch prevents two
tasks from running simultaneously.

Fixes: 864a800d70 ("net/i40e: remove VF interrupt handler")
Fixes: ee653bd800 ("net/i40e: determine number of queues per VF at run time")
Cc: stable@dpdk.org

Signed-off-by: Zhu Tao <taox.zhu@intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 5be32b069..34aaaf207 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -140,6 +140,8 @@ static int
 i40evf_set_mc_addr_list(struct rte_eth_dev *dev,
 			struct rte_ether_addr *mc_addr_set,
 			uint32_t nb_mc_addr);
+static void
+i40evf_dev_alarm_handler(void *param);
 
 /* Default hash key buffer for RSS */
 static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
@@ -1051,10 +1053,14 @@ i40evf_request_queues(struct rte_eth_dev *dev, uint16_t num)
 	args.in_args_size = sizeof(vfres);
 	args.out_buffer = vf->aq_resp;
 	args.out_size = I40E_AQ_BUF_SZ;
+
+	rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev);
 	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command OP_REQUEST_QUEUES");
 
+	rte_eal_alarm_set(I40EVF_ALARM_INTERVAL,
+			  i40evf_dev_alarm_handler, dev);
 	return err;
 }
 
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v3] net/i40e: fix request queue fail in VF
  2019-07-24  8:32   ` [dpdk-dev] [PATCH v3] " taox.zhu
@ 2019-07-24 12:54     ` Zhang, Qi Z
  0 siblings, 0 replies; 8+ messages in thread
From: Zhang, Qi Z @ 2019-07-24 12:54 UTC (permalink / raw)
  To: Zhu, TaoX, Xing, Beilei; +Cc: dev, stable



> -----Original Message-----
> From: Zhu, TaoX
> Sent: Wednesday, July 24, 2019 4:33 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Zhu, TaoX <taox.zhu@intel.com>; stable@dpdk.org
> Subject: [PATCH v3] net/i40e: fix request queue fail in VF
> 
> From: Zhu Tao <taox.zhu@intel.com>
> 
> When the VF configuration is larger than the number of queues reserved by PF,
> VF sends the request queue command through admin queue. When PF received
> this command, it may reset the VF and send a notification before resetting. If
> this notification is read by the timed task alarm, Task request queue will lost
> notification. This patch prevents two tasks from running simultaneously.
> 
> Fixes: 864a800d70 ("net/i40e: remove VF interrupt handler")

This fix line can be removed since it does not bring the trouble.

> Fixes: ee653bd800 ("net/i40e: determine number of queues per VF at run
> time")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zhu Tao <taox.zhu@intel.com>
> ---

Please remind to add change log next time.

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, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-18 14:53 [dpdk-dev] [PATCH] net/i40e: fix request queue fail in VF taox.zhu
2019-07-18 13:09 ` Ye Xiaolong
2019-07-18  7:05   ` Zhu, TaoX
2019-07-18 14:05     ` Ye Xiaolong
2019-07-19  3:18 ` [dpdk-dev] [PATCH v2] " taox.zhu
2019-07-23  6:52   ` Zhang, Qi Z
2019-07-24  8:32   ` [dpdk-dev] [PATCH v3] " taox.zhu
2019-07-24 12:54     ` Zhang, Qi Z

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org dpdk-dev@archiver.kernel.org
	public-inbox-index dpdk-dev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox