diff mbox series

[v2] net/i40e: always re-program promiscuous mode on VF interface

Message ID 20191119134431.13566.87940.stgit@netdev64 (mailing list archive)
State Accepted, archived
Delegated to: xiaolong ye
Headers
Series [v2] net/i40e: always re-program promiscuous mode on VF interface |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation fail Compilation issues

Commit Message

Eelco Chaudron Nov. 19, 2019, 1:45 p.m. UTC 
  During a kernel PF reset, this event is propagated to the VF.
The DPDK VF PMD will execute the reset task before the PF is done
with his. This results in the admin queue message not being responded
to leaving the port in "promiscuous" mode.

This patch makes sure the promiscuous mode is configured independently
of the current admin state.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
v1-v2:
  In the earlier patch, we would force the vf->promisc_unicast_enabled
  state to false after a reset. Based on the review comments it was
  prefered to not take the current state into account when programming.

  v1 patch was called: net/i40e: force promiscuous state after VF reset

 drivers/net/i40e/i40e_ethdev_vf.c |   16 ----------------
 1 file changed, 16 deletions(-)

Comments

Eelco Chaudron Dec. 4, 2019, 3:18 p.m. UTC | #1 
Any update on this patch?


On 19 Nov 2019, at 14:45, Eelco Chaudron wrote:

> During a kernel PF reset, this event is propagated to the VF.
> The DPDK VF PMD will execute the reset task before the PF is done
> with his. This results in the admin queue message not being responded
> to leaving the port in "promiscuous" mode.
>
> This patch makes sure the promiscuous mode is configured independently
> of the current admin state.
>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
> v1-v2:
>   In the earlier patch, we would force the vf->promisc_unicast_enabled
>   state to false after a reset. Based on the review comments it was
>   prefered to not take the current state into account when 
> programming.
>
>   v1 patch was called: net/i40e: force promiscuous state after VF 
> reset
>
>  drivers/net/i40e/i40e_ethdev_vf.c |   16 ----------------
>  1 file changed, 16 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index 5dba092..43f7ab5 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -2162,10 +2162,6 @@ static int eth_i40evf_pci_remove(struct 
> rte_pci_device *pci_dev)
>  	struct i40e_vf *vf = 
> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>  	int ret;
>
> -	/* If enabled, just return */
> -	if (vf->promisc_unicast_enabled)
> -		return 0;
> -
>  	ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled);
>  	if (ret == 0)
>  		vf->promisc_unicast_enabled = TRUE;
> @@ -2181,10 +2177,6 @@ static int eth_i40evf_pci_remove(struct 
> rte_pci_device *pci_dev)
>  	struct i40e_vf *vf = 
> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>  	int ret;
>
> -	/* If disabled, just return */
> -	if (!vf->promisc_unicast_enabled)
> -		return 0;
> -
>  	ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled);
>  	if (ret == 0)
>  		vf->promisc_unicast_enabled = FALSE;
> @@ -2200,10 +2192,6 @@ static int eth_i40evf_pci_remove(struct 
> rte_pci_device *pci_dev)
>  	struct i40e_vf *vf = 
> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>  	int ret;
>
> -	/* If enabled, just return */
> -	if (vf->promisc_multicast_enabled)
> -		return 0;
> -
>  	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1);
>  	if (ret == 0)
>  		vf->promisc_multicast_enabled = TRUE;
> @@ -2219,10 +2207,6 @@ static int eth_i40evf_pci_remove(struct 
> rte_pci_device *pci_dev)
>  	struct i40e_vf *vf = 
> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>  	int ret;
>
> -	/* If enabled, just return */
> -	if (!vf->promisc_multicast_enabled)
> -		return 0;
> -
>  	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0);
>  	if (ret == 0)
>  		vf->promisc_multicast_enabled = FALSE;
Eelco Chaudron Dec. 17, 2019, 1:50 p.m. UTC | #2 
Trying again?

On 4 Dec 2019, at 16:18, Eelco Chaudron wrote:

> Any update on this patch?
>
>
> On 19 Nov 2019, at 14:45, Eelco Chaudron wrote:
>
>> During a kernel PF reset, this event is propagated to the VF.
>> The DPDK VF PMD will execute the reset task before the PF is done
>> with his. This results in the admin queue message not being responded
>> to leaving the port in "promiscuous" mode.
>>
>> This patch makes sure the promiscuous mode is configured 
>> independently
>> of the current admin state.
>>
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>> ---
>> v1-v2:
>>   In the earlier patch, we would force the 
>> vf->promisc_unicast_enabled
>>   state to false after a reset. Based on the review comments it was
>>   prefered to not take the current state into account when 
>> programming.
>>
>>   v1 patch was called: net/i40e: force promiscuous state after VF 
>> reset
>>
>>  drivers/net/i40e/i40e_ethdev_vf.c |   16 ----------------
>>  1 file changed, 16 deletions(-)
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
>> b/drivers/net/i40e/i40e_ethdev_vf.c
>> index 5dba092..43f7ab5 100644
>> --- a/drivers/net/i40e/i40e_ethdev_vf.c
>> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
>> @@ -2162,10 +2162,6 @@ static int eth_i40evf_pci_remove(struct 
>> rte_pci_device *pci_dev)
>>  	struct i40e_vf *vf = 
>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>>  	int ret;
>>
>> -	/* If enabled, just return */
>> -	if (vf->promisc_unicast_enabled)
>> -		return 0;
>> -
>>  	ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled);
>>  	if (ret == 0)
>>  		vf->promisc_unicast_enabled = TRUE;
>> @@ -2181,10 +2177,6 @@ static int eth_i40evf_pci_remove(struct 
>> rte_pci_device *pci_dev)
>>  	struct i40e_vf *vf = 
>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>>  	int ret;
>>
>> -	/* If disabled, just return */
>> -	if (!vf->promisc_unicast_enabled)
>> -		return 0;
>> -
>>  	ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled);
>>  	if (ret == 0)
>>  		vf->promisc_unicast_enabled = FALSE;
>> @@ -2200,10 +2192,6 @@ static int eth_i40evf_pci_remove(struct 
>> rte_pci_device *pci_dev)
>>  	struct i40e_vf *vf = 
>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>>  	int ret;
>>
>> -	/* If enabled, just return */
>> -	if (vf->promisc_multicast_enabled)
>> -		return 0;
>> -
>>  	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1);
>>  	if (ret == 0)
>>  		vf->promisc_multicast_enabled = TRUE;
>> @@ -2219,10 +2207,6 @@ static int eth_i40evf_pci_remove(struct 
>> rte_pci_device *pci_dev)
>>  	struct i40e_vf *vf = 
>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>>  	int ret;
>>
>> -	/* If enabled, just return */
>> -	if (!vf->promisc_multicast_enabled)
>> -		return 0;
>> -
>>  	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0);
>>  	if (ret == 0)
>>  		vf->promisc_multicast_enabled = FALSE;
Xiao Zhang Dec. 18, 2019, 1:23 a.m. UTC | #3 
> -----Original Message-----
> From: Eelco Chaudron [mailto:echaudro@redhat.com]
> Sent: Tuesday, November 19, 2019 9:45 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org
> Subject: [PATCH v2] net/i40e: always re-program promiscuous mode on VF
> interface
> 
> During a kernel PF reset, this event is propagated to the VF.
> The DPDK VF PMD will execute the reset task before the PF is done with his. This
> results in the admin queue message not being responded to leaving the port in
> "promiscuous" mode.
> 
> This patch makes sure the promiscuous mode is configured independently of the
> current admin state.
> 
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>

Reviewed-by: Xiao Zhang <xiao.zhang@intel.com>
Xiaolong Ye Dec. 18, 2019, 2:57 a.m. UTC | #4 
On 12/18, Zhang, Xiao wrote:
>
>> -----Original Message-----
>> From: Eelco Chaudron [mailto:echaudro@redhat.com]
>> Sent: Tuesday, November 19, 2019 9:45 PM
>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
>> Cc: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org
>> Subject: [PATCH v2] net/i40e: always re-program promiscuous mode on VF
>> interface
>> 
>> During a kernel PF reset, this event is propagated to the VF.
>> The DPDK VF PMD will execute the reset task before the PF is done with his. This
>> results in the admin queue message not being responded to leaving the port in
>> "promiscuous" mode.
>> 
>> This patch makes sure the promiscuous mode is configured independently of the
>> current admin state.
>> 
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>
>Reviewed-by: Xiao Zhang <xiao.zhang@intel.com>

Applied to dpdk-next-net-intel, Thanks.
Eelco Chaudron Dec. 18, 2019, 8:33 a.m. UTC | #5 
Thanks!



On 18 Dec 2019, at 3:57, Ye Xiaolong wrote:

> On 12/18, Zhang, Xiao wrote:
>>
>>> -----Original Message-----
>>> From: Eelco Chaudron [mailto:echaudro@redhat.com]
>>> Sent: Tuesday, November 19, 2019 9:45 PM
>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z 
>>> <qi.z.zhang@intel.com>
>>> Cc: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org
>>> Subject: [PATCH v2] net/i40e: always re-program promiscuous mode on 
>>> VF
>>> interface
>>>
>>> During a kernel PF reset, this event is propagated to the VF.
>>> The DPDK VF PMD will execute the reset task before the PF is done 
>>> with his. This
>>> results in the admin queue message not being responded to leaving 
>>> the port in
>>> "promiscuous" mode.
>>>
>>> This patch makes sure the promiscuous mode is configured 
>>> independently of the
>>> current admin state.
>>>
>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>
>> Reviewed-by: Xiao Zhang <xiao.zhang@intel.com>
>
> Applied to dpdk-next-net-intel, Thanks.
diff mbox series

Patch

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 5dba092..43f7ab5 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2162,10 +2162,6 @@  static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	int ret;
 
-	/* If enabled, just return */
-	if (vf->promisc_unicast_enabled)
-		return 0;
-
 	ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled);
 	if (ret == 0)
 		vf->promisc_unicast_enabled = TRUE;
@@ -2181,10 +2177,6 @@  static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	int ret;
 
-	/* If disabled, just return */
-	if (!vf->promisc_unicast_enabled)
-		return 0;
-
 	ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled);
 	if (ret == 0)
 		vf->promisc_unicast_enabled = FALSE;
@@ -2200,10 +2192,6 @@  static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	int ret;
 
-	/* If enabled, just return */
-	if (vf->promisc_multicast_enabled)
-		return 0;
-
 	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1);
 	if (ret == 0)
 		vf->promisc_multicast_enabled = TRUE;
@@ -2219,10 +2207,6 @@  static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	int ret;
 
-	/* If enabled, just return */
-	if (!vf->promisc_multicast_enabled)
-		return 0;
-
 	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0);
 	if (ret == 0)
 		vf->promisc_multicast_enabled = FALSE;