net/ice: fix flow redirector issue

Message ID 1587558164-5504-1-git-send-email-beilei.xing@intel.com (mailing list archive)
State Accepted, archived
Delegated to: xiaolong ye
Headers
Series net/ice: fix flow redirector issue |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing fail Testing issues

Commit Message

Xing, Beilei April 22, 2020, 12:22 p.m. UTC
  If there's VF reset, the kernel PF will remove rules
associated with the reset VF no matter the HW VSI ID
is changed or not. So DCF should redirector all rules
associated with the reset VF no matter the HW VSI ID
is changed or not.

Fixes: f10cde8e8478 ("net/ice: get VF hardware index in DCF")
Fixes: dc0f06849e50 ("net/ice: redirect switch rule to new VSI")

Signed-off-by: Beilei Xing <beilei.xing@intel.com>
---
 drivers/net/ice/ice_dcf.c        | 2 +-
 drivers/net/ice/ice_dcf_parent.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
  

Comments

Wang, Haiyue April 22, 2020, 5:19 a.m. UTC | #1
> -----Original Message-----
> From: Xing, Beilei <beilei.xing@intel.com>
> Sent: Wednesday, April 22, 2020 20:23
> To: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: [PATCH] net/ice: fix flow redirector issue
> 
> If there's VF reset, the kernel PF will remove rules
> associated with the reset VF no matter the HW VSI ID
> is changed or not. So DCF should redirector all rules
> associated with the reset VF no matter the HW VSI ID
> is changed or not.
> 
> Fixes: f10cde8e8478 ("net/ice: get VF hardware index in DCF")
> Fixes: dc0f06849e50 ("net/ice: redirect switch rule to new VSI")
> 
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> ---
>  drivers/net/ice/ice_dcf.c        | 2 +-
>  drivers/net/ice/ice_dcf_parent.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
> index 4c30f0e..0cd5d1b 100644
> --- a/drivers/net/ice/ice_dcf.c
> +++ b/drivers/net/ice/ice_dcf.c
> @@ -536,7 +536,7 @@ ice_dcf_handle_vsi_update_event(struct ice_dcf_hw *hw)
>  	rte_intr_disable(&pci_dev->intr_handle);
>  	ice_dcf_disable_irq0(hw);
> 
> -	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw))
> +	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw) < 0)
>  		err = -1;
> 
>  	rte_intr_enable(&pci_dev->intr_handle);
> diff --git a/drivers/net/ice/ice_dcf_parent.c b/drivers/net/ice/ice_dcf_parent.c
> index d4b4ede..bdfc7d4 100644
> --- a/drivers/net/ice/ice_dcf_parent.c
> +++ b/drivers/net/ice/ice_dcf_parent.c
> @@ -45,7 +45,7 @@ ice_dcf_update_vsi_ctx(struct ice_hw *hw, uint16_t vsi_handle,
>  			VIRTCHNL_DCF_VF_VSI_ID_S;
> 
>  		/* Redirect rules if vsi mapping table changes. */
> -		if (!first_update && vsi_ctx->vsi_num != new_vsi_num) {
> +		if (!first_update) {
>  			struct ice_flow_redirect rd;
> 
>  			memset(&rd, 0, sizeof(struct ice_flow_redirect));
> --
> 2.7.4

Acked-by: Haiyue Wang <haiyue.wang@intel.com>
  
Xiaolong Ye April 22, 2020, 7:31 a.m. UTC | #2
On 04/22, Beilei Xing wrote:
>If there's VF reset, the kernel PF will remove rules
>associated with the reset VF no matter the HW VSI ID
>is changed or not. So DCF should redirector all rules
>associated with the reset VF no matter the HW VSI ID
>is changed or not.
>
>Fixes: f10cde8e8478 ("net/ice: get VF hardware index in DCF")
>Fixes: dc0f06849e50 ("net/ice: redirect switch rule to new VSI")
>
>Signed-off-by: Beilei Xing <beilei.xing@intel.com>
>---
> drivers/net/ice/ice_dcf.c        | 2 +-
> drivers/net/ice/ice_dcf_parent.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
>index 4c30f0e..0cd5d1b 100644
>--- a/drivers/net/ice/ice_dcf.c
>+++ b/drivers/net/ice/ice_dcf.c
>@@ -536,7 +536,7 @@ ice_dcf_handle_vsi_update_event(struct ice_dcf_hw *hw)
> 	rte_intr_disable(&pci_dev->intr_handle);
> 	ice_dcf_disable_irq0(hw);
> 
>-	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw))
>+	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw) < 0)
> 		err = -1;
> 
> 	rte_intr_enable(&pci_dev->intr_handle);
>diff --git a/drivers/net/ice/ice_dcf_parent.c b/drivers/net/ice/ice_dcf_parent.c
>index d4b4ede..bdfc7d4 100644
>--- a/drivers/net/ice/ice_dcf_parent.c
>+++ b/drivers/net/ice/ice_dcf_parent.c
>@@ -45,7 +45,7 @@ ice_dcf_update_vsi_ctx(struct ice_hw *hw, uint16_t vsi_handle,
> 			VIRTCHNL_DCF_VF_VSI_ID_S;
> 
> 		/* Redirect rules if vsi mapping table changes. */
>-		if (!first_update && vsi_ctx->vsi_num != new_vsi_num) {
>+		if (!first_update) {
> 			struct ice_flow_redirect rd;
> 
> 			memset(&rd, 0, sizeof(struct ice_flow_redirect));
>-- 
>2.7.4
>

Applied to dpdk-next-net-intel, Thanks.
  
Kevin Traynor April 22, 2020, 8:40 a.m. UTC | #3
On 22/04/2020 08:31, Ye Xiaolong wrote:
> On 04/22, Beilei Xing wrote:
>> If there's VF reset, the kernel PF will remove rules
>> associated with the reset VF no matter the HW VSI ID
>> is changed or not. So DCF should redirector all rules
>> associated with the reset VF no matter the HW VSI ID
>> is changed or not.
>>
>> Fixes: f10cde8e8478 ("net/ice: get VF hardware index in DCF")
>> Fixes: dc0f06849e50 ("net/ice: redirect switch rule to new VSI")
>>

I think these commit-id's will need some fix up before reaching master

>> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
>> ---
>> drivers/net/ice/ice_dcf.c        | 2 +-
>> drivers/net/ice/ice_dcf_parent.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
>> index 4c30f0e..0cd5d1b 100644
>> --- a/drivers/net/ice/ice_dcf.c
>> +++ b/drivers/net/ice/ice_dcf.c
>> @@ -536,7 +536,7 @@ ice_dcf_handle_vsi_update_event(struct ice_dcf_hw *hw)
>> 	rte_intr_disable(&pci_dev->intr_handle);
>> 	ice_dcf_disable_irq0(hw);
>>
>> -	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw))
>> +	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw) < 0)
>> 		err = -1;
>>
>> 	rte_intr_enable(&pci_dev->intr_handle);
>> diff --git a/drivers/net/ice/ice_dcf_parent.c b/drivers/net/ice/ice_dcf_parent.c
>> index d4b4ede..bdfc7d4 100644
>> --- a/drivers/net/ice/ice_dcf_parent.c
>> +++ b/drivers/net/ice/ice_dcf_parent.c
>> @@ -45,7 +45,7 @@ ice_dcf_update_vsi_ctx(struct ice_hw *hw, uint16_t vsi_handle,
>> 			VIRTCHNL_DCF_VF_VSI_ID_S;
>>
>> 		/* Redirect rules if vsi mapping table changes. */
>> -		if (!first_update && vsi_ctx->vsi_num != new_vsi_num) {
>> +		if (!first_update) {
>> 			struct ice_flow_redirect rd;
>>
>> 			memset(&rd, 0, sizeof(struct ice_flow_redirect));
>> -- 
>> 2.7.4
>>
> 
> Applied to dpdk-next-net-intel, Thanks.
>
  
Ferruh Yigit April 22, 2020, 9:35 a.m. UTC | #4
On 4/22/2020 9:40 AM, Kevin Traynor wrote:
> On 22/04/2020 08:31, Ye Xiaolong wrote:
>> On 04/22, Beilei Xing wrote:
>>> If there's VF reset, the kernel PF will remove rules
>>> associated with the reset VF no matter the HW VSI ID
>>> is changed or not. So DCF should redirector all rules
>>> associated with the reset VF no matter the HW VSI ID
>>> is changed or not.
>>>
>>> Fixes: f10cde8e8478 ("net/ice: get VF hardware index in DCF")
>>> Fixes: dc0f06849e50 ("net/ice: redirect switch rule to new VSI")
>>>
> 
> I think these commit-id's will need some fix up before reaching master

We are aware of it, but these fixes are in the next-net and next-net keeps
rebasing on the main repo, so commit ids keeps changes there. These should be
fixed while next-net merged to main repo.
I tend to squash fixes in this case whenever reasonable, but not always.
Frequently pulling from next-net to main solves this issue too.

> 
>>> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
>>> ---
>>> drivers/net/ice/ice_dcf.c        | 2 +-
>>> drivers/net/ice/ice_dcf_parent.c | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
>>> index 4c30f0e..0cd5d1b 100644
>>> --- a/drivers/net/ice/ice_dcf.c
>>> +++ b/drivers/net/ice/ice_dcf.c
>>> @@ -536,7 +536,7 @@ ice_dcf_handle_vsi_update_event(struct ice_dcf_hw *hw)
>>> 	rte_intr_disable(&pci_dev->intr_handle);
>>> 	ice_dcf_disable_irq0(hw);
>>>
>>> -	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw))
>>> +	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw) < 0)
>>> 		err = -1;
>>>
>>> 	rte_intr_enable(&pci_dev->intr_handle);
>>> diff --git a/drivers/net/ice/ice_dcf_parent.c b/drivers/net/ice/ice_dcf_parent.c
>>> index d4b4ede..bdfc7d4 100644
>>> --- a/drivers/net/ice/ice_dcf_parent.c
>>> +++ b/drivers/net/ice/ice_dcf_parent.c
>>> @@ -45,7 +45,7 @@ ice_dcf_update_vsi_ctx(struct ice_hw *hw, uint16_t vsi_handle,
>>> 			VIRTCHNL_DCF_VF_VSI_ID_S;
>>>
>>> 		/* Redirect rules if vsi mapping table changes. */
>>> -		if (!first_update && vsi_ctx->vsi_num != new_vsi_num) {
>>> +		if (!first_update) {
>>> 			struct ice_flow_redirect rd;
>>>
>>> 			memset(&rd, 0, sizeof(struct ice_flow_redirect));
>>> -- 
>>> 2.7.4
>>>
>>
>> Applied to dpdk-next-net-intel, Thanks.
>>
>
  
Kevin Traynor April 22, 2020, 9:59 a.m. UTC | #5
On 22/04/2020 10:35, Ferruh Yigit wrote:
> On 4/22/2020 9:40 AM, Kevin Traynor wrote:
>> On 22/04/2020 08:31, Ye Xiaolong wrote:
>>> On 04/22, Beilei Xing wrote:
>>>> If there's VF reset, the kernel PF will remove rules
>>>> associated with the reset VF no matter the HW VSI ID
>>>> is changed or not. So DCF should redirector all rules
>>>> associated with the reset VF no matter the HW VSI ID
>>>> is changed or not.
>>>>
>>>> Fixes: f10cde8e8478 ("net/ice: get VF hardware index in DCF")
>>>> Fixes: dc0f06849e50 ("net/ice: redirect switch rule to new VSI")
>>>>
>>
>> I think these commit-id's will need some fix up before reaching master
> 
> We are aware of it, but these fixes are in the next-net and next-net keeps
> rebasing on the main repo, so commit ids keeps changes there. These should be
> fixed while next-net merged to main repo.
> I tend to squash fixes in this case whenever reasonable, but not always.
> Frequently pulling from next-net to main solves this issue too.
> 

Thanks Ferruh - I know you do a good job on this. Was just flagging as I
noticed while checking if it needed a stable tag (it doesn't).

>>
>>>> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
>>>> ---
>>>> drivers/net/ice/ice_dcf.c        | 2 +-
>>>> drivers/net/ice/ice_dcf_parent.c | 2 +-
>>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
>>>> index 4c30f0e..0cd5d1b 100644
>>>> --- a/drivers/net/ice/ice_dcf.c
>>>> +++ b/drivers/net/ice/ice_dcf.c
>>>> @@ -536,7 +536,7 @@ ice_dcf_handle_vsi_update_event(struct ice_dcf_hw *hw)
>>>> 	rte_intr_disable(&pci_dev->intr_handle);
>>>> 	ice_dcf_disable_irq0(hw);
>>>>
>>>> -	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw))
>>>> +	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw) < 0)
>>>> 		err = -1;
>>>>
>>>> 	rte_intr_enable(&pci_dev->intr_handle);
>>>> diff --git a/drivers/net/ice/ice_dcf_parent.c b/drivers/net/ice/ice_dcf_parent.c
>>>> index d4b4ede..bdfc7d4 100644
>>>> --- a/drivers/net/ice/ice_dcf_parent.c
>>>> +++ b/drivers/net/ice/ice_dcf_parent.c
>>>> @@ -45,7 +45,7 @@ ice_dcf_update_vsi_ctx(struct ice_hw *hw, uint16_t vsi_handle,
>>>> 			VIRTCHNL_DCF_VF_VSI_ID_S;
>>>>
>>>> 		/* Redirect rules if vsi mapping table changes. */
>>>> -		if (!first_update && vsi_ctx->vsi_num != new_vsi_num) {
>>>> +		if (!first_update) {
>>>> 			struct ice_flow_redirect rd;
>>>>
>>>> 			memset(&rd, 0, sizeof(struct ice_flow_redirect));
>>>> -- 
>>>> 2.7.4
>>>>
>>>
>>> Applied to dpdk-next-net-intel, Thanks.
>>>
>>
>
  

Patch

diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c
index 4c30f0e..0cd5d1b 100644
--- a/drivers/net/ice/ice_dcf.c
+++ b/drivers/net/ice/ice_dcf.c
@@ -536,7 +536,7 @@  ice_dcf_handle_vsi_update_event(struct ice_dcf_hw *hw)
 	rte_intr_disable(&pci_dev->intr_handle);
 	ice_dcf_disable_irq0(hw);
 
-	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw))
+	if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw) < 0)
 		err = -1;
 
 	rte_intr_enable(&pci_dev->intr_handle);
diff --git a/drivers/net/ice/ice_dcf_parent.c b/drivers/net/ice/ice_dcf_parent.c
index d4b4ede..bdfc7d4 100644
--- a/drivers/net/ice/ice_dcf_parent.c
+++ b/drivers/net/ice/ice_dcf_parent.c
@@ -45,7 +45,7 @@  ice_dcf_update_vsi_ctx(struct ice_hw *hw, uint16_t vsi_handle,
 			VIRTCHNL_DCF_VF_VSI_ID_S;
 
 		/* Redirect rules if vsi mapping table changes. */
-		if (!first_update && vsi_ctx->vsi_num != new_vsi_num) {
+		if (!first_update) {
 			struct ice_flow_redirect rd;
 
 			memset(&rd, 0, sizeof(struct ice_flow_redirect));