net/i40e: force promiscuous state after VF reset

  Even though the device reset is successful, disabling promiscuous mode
might not always succeed, causing enabling it after reset to
fail. This would happen when the kernel driver requires a reset of the
VF.

This patch resets the internal state, so next time promiscuous mode is
configured it will be enabled.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

Any update on this patch?

Thanks,

Eelco


On 17 Sep 2019, at 9:40, Eelco Chaudron wrote:

> Even though the device reset is successful, disabling promiscuous mode
> might not always succeed, causing enabling it after reset to
> fail. This would happen when the kernel driver requires a reset of the
> VF.
>
> This patch resets the internal state, so next time promiscuous mode is
> configured it will be enabled.
>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
>  drivers/net/i40e/i40e_ethdev_vf.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index 551f6fa..e0f99a4 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -2276,11 +2276,21 @@ static int eth_i40evf_pci_remove(struct 
> rte_pci_device *pci_dev)
>  i40evf_dev_reset(struct rte_eth_dev *dev)
>  {
>  	int ret;
> +	struct i40e_vf *vf = 
> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>
>  	ret = i40evf_dev_uninit(dev);
>  	if (ret)
>  		return ret;
>
> +	/*
> +	 * Even though the device reset is successful disabling promiscuous
> +	 * mode might not always succeed, causing enabling it after reset to
> +	 * fail. This would happen when the kernel driver requires a reset
> +	 * of the VF.
> +	 */
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +		vf->promisc_unicast_enabled = FALSE;
> +
>  	ret = i40evf_dev_init(dev);
>
>  	return ret;
> -- 
> 1.8.3.1
  

> -----Original Message-----
> From: Eelco Chaudron [mailto:echaudro@redhat.com]
> Sent: Tuesday, October 15, 2019 6:32 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF
> reset
> 
> Any update on this patch?
> 
> Thanks,
> 
> Eelco
> 
> 
> On 17 Sep 2019, at 9:40, Eelco Chaudron wrote:
> 
> > Even though the device reset is successful, disabling promiscuous mode
> > might not always succeed, causing enabling it after reset to fail.
> > This would happen when the kernel driver requires a reset of the VF.
> >
> > This patch resets the internal state, so next time promiscuous mode is
> > configured it will be enabled.
> >
> > Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev_vf.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index 551f6fa..e0f99a4 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -2276,11 +2276,21 @@ static int eth_i40evf_pci_remove(struct
> > rte_pci_device *pci_dev)  i40evf_dev_reset(struct rte_eth_dev *dev)  {
> >  	int ret;
> > +	struct i40e_vf *vf =
> > I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> >
> >  	ret = i40evf_dev_uninit(dev);
> >  	if (ret)
> >  		return ret;
> >
> > +	/*
> > +	 * Even though the device reset is successful disabling promiscuous
> > +	 * mode might not always succeed, causing enabling it after reset to

I think we need to root cause why fail to disable promiscuous mode and try to fix it first.

Thanks.
Beilei

> > +	 * fail. This would happen when the kernel driver requires a reset
> > +	 * of the VF.
> > +	 */
> > +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> > +		vf->promisc_unicast_enabled = FALSE;
> > +
> >  	ret = i40evf_dev_init(dev);
> >
> >  	return ret;
> > --
> > 1.8.3.1
  

On 17 Oct 2019, at 8:34, Xing, Beilei wrote:

>> On 17 Sep 2019, at 9:40, Eelco Chaudron wrote:
>>
>>> Even though the device reset is successful, disabling promiscuous 
>>> mode
>>> might not always succeed, causing enabling it after reset to fail.
>>> This would happen when the kernel driver requires a reset of the VF.
>>>
>>> This patch resets the internal state, so next time promiscuous mode 
>>> is
>>> configured it will be enabled.
>>>
>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>> ---
>>>  drivers/net/i40e/i40e_ethdev_vf.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
>>> b/drivers/net/i40e/i40e_ethdev_vf.c
>>> index 551f6fa..e0f99a4 100644
>>> --- a/drivers/net/i40e/i40e_ethdev_vf.c
>>> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
>>> @@ -2276,11 +2276,21 @@ static int eth_i40evf_pci_remove(struct
>>> rte_pci_device *pci_dev)  i40evf_dev_reset(struct rte_eth_dev *dev)  
>>> {
>>>  	int ret;
>>> +	struct i40e_vf *vf =
>>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>>>
>>>  	ret = i40evf_dev_uninit(dev);
>>>  	if (ret)
>>>  		return ret;
>>>
>>> +	/*
>>> +	 * Even though the device reset is successful disabling 
>>> promiscuous
>>> +	 * mode might not always succeed, causing enabling it after reset 
>>> to
>
> I think we need to root cause why fail to disable promiscuous mode and 
> try to fix it first.

I’ve copied in Xiao who helped me identify the issue in your driver.

The issue is because the change from kernel pf was not synced to DPDK vf 
during the closing period of reset, so we get this failure. Xiao can you 
add more details?


>>> +	 * fail. This would happen when the kernel driver requires a reset
>>> +	 * of the VF.
>>> +	 */
>>> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
>>> +		vf->promisc_unicast_enabled = FALSE;
>>> +
>>>  	ret = i40evf_dev_init(dev);
>>>
>>>  	return ret;
>>> --
>>> 1.8.3.1
  

> -----Original Message-----
> From: Eelco Chaudron <echaudro@redhat.com>
> Sent: Friday, October 25, 2019 5:24 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Xiao <xiao.zhang@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF
> reset
> 
> 
> 
> On 17 Oct 2019, at 8:34, Xing, Beilei wrote:
> 
> >> On 17 Sep 2019, at 9:40, Eelco Chaudron wrote:
> >>
> >>> Even though the device reset is successful, disabling promiscuous
> >>> mode might not always succeed, causing enabling it after reset to
> >>> fail.
> >>> This would happen when the kernel driver requires a reset of the VF.
> >>>
> >>> This patch resets the internal state, so next time promiscuous mode
> >>> is configured it will be enabled.
> >>>
> >>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> >>> ---
> >>>  drivers/net/i40e/i40e_ethdev_vf.c | 10 ++++++++++
> >>>  1 file changed, 10 insertions(+)
> >>>
> >>> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> >>> b/drivers/net/i40e/i40e_ethdev_vf.c
> >>> index 551f6fa..e0f99a4 100644
> >>> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> >>> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> >>> @@ -2276,11 +2276,21 @@ static int eth_i40evf_pci_remove(struct
> >>> rte_pci_device *pci_dev)  i40evf_dev_reset(struct rte_eth_dev *dev)
> >>> {
> >>>  	int ret;
> >>> +	struct i40e_vf *vf =
> >>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> >>>
> >>>  	ret = i40evf_dev_uninit(dev);
> >>>  	if (ret)
> >>>  		return ret;
> >>>
> >>> +	/*
> >>> +	 * Even though the device reset is successful disabling
> >>> promiscuous
> >>> +	 * mode might not always succeed, causing enabling it after reset
> >>> to
> >
> > I think we need to root cause why fail to disable promiscuous mode and
> > try to fix it first.
> 
> I’ve copied in Xiao who helped me identify the issue in your driver.
> 
> The issue is because the change from kernel pf was not synced to DPDK vf during
> the closing period of reset, so we get this failure. Xiao can you add more details?
> 

The root cause is when kernel pf generate DPDK vf reset event, the flag 
vf->promisc_unicast_enabled will not be cleared but promiscuous mode is
disabled. When trying to enable promiscuous mode next time by calling 
i40evf_dev_promiscuous_enable won't work because it will check the 
vf->promisc_unicast_enabled flag first.

static int
i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev)
{
...
    /* If enabled, just return */
    if (vf->promisc_unicast_enabled)
        return 0;
...
}

Hi Eelco,

I think you may need add more detailed message in the commit log or comments.

> 
> >>> +	 * fail. This would happen when the kernel driver requires a reset
> >>> +	 * of the VF.
> >>> +	 */
> >>> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> >>> +		vf->promisc_unicast_enabled = FALSE;
> >>> +
> >>>  	ret = i40evf_dev_init(dev);
> >>>
> >>>  	return ret;
> >>> --
> >>> 1.8.3.1
  

On 29 Oct 2019, at 8:39, Zhang, Xiao wrote:

>> -----Original Message-----
>> From: Eelco Chaudron <echaudro@redhat.com>
>> Sent: Friday, October 25, 2019 5:24 PM
>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Xiao 
>> <xiao.zhang@intel.com>
>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state 
>> after VF
>> reset
>>
>>
>>
>> On 17 Oct 2019, at 8:34, Xing, Beilei wrote:
>>
>>>> On 17 Sep 2019, at 9:40, Eelco Chaudron wrote:
>>>>
>>>>> Even though the device reset is successful, disabling promiscuous
>>>>> mode might not always succeed, causing enabling it after reset to
>>>>> fail.
>>>>> This would happen when the kernel driver requires a reset of the 
>>>>> VF.
>>>>>
>>>>> This patch resets the internal state, so next time promiscuous 
>>>>> mode
>>>>> is configured it will be enabled.
>>>>>
>>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>>>> ---
>>>>>  drivers/net/i40e/i40e_ethdev_vf.c | 10 ++++++++++
>>>>>  1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
>>>>> b/drivers/net/i40e/i40e_ethdev_vf.c
>>>>> index 551f6fa..e0f99a4 100644
>>>>> --- a/drivers/net/i40e/i40e_ethdev_vf.c
>>>>> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
>>>>> @@ -2276,11 +2276,21 @@ static int eth_i40evf_pci_remove(struct
>>>>> rte_pci_device *pci_dev)  i40evf_dev_reset(struct rte_eth_dev 
>>>>> *dev)
>>>>> {
>>>>>  	int ret;
>>>>> +	struct i40e_vf *vf =
>>>>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>>>>>
>>>>>  	ret = i40evf_dev_uninit(dev);
>>>>>  	if (ret)
>>>>>  		return ret;
>>>>>
>>>>> +	/*
>>>>> +	 * Even though the device reset is successful disabling
>>>>> promiscuous
>>>>> +	 * mode might not always succeed, causing enabling it after 
>>>>> reset
>>>>> to
>>>
>>> I think we need to root cause why fail to disable promiscuous mode 
>>> and
>>> try to fix it first.
>>
>> I’ve copied in Xiao who helped me identify the issue in your 
>> driver.
>>
>> The issue is because the change from kernel pf was not synced to DPDK 
>> vf during
>> the closing period of reset, so we get this failure. Xiao can you add 
>> more details?
>>
>
> The root cause is when kernel pf generate DPDK vf reset event, the 
> flag
> vf->promisc_unicast_enabled will not be cleared but promiscuous mode 
> is
> disabled. When trying to enable promiscuous mode next time by calling
> i40evf_dev_promiscuous_enable won't work because it will check the
> vf->promisc_unicast_enabled flag first.
>
> static int
> i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev)
> {
> ...
>     /* If enabled, just return */
>     if (vf->promisc_unicast_enabled)
>         return 0;
> ...
> }
>
> Hi Eelco,
>
> I think you may need add more detailed message in the commit log or 
> comments.

My interpretation of the request was that Beilei wanted to know why 
disabling promiscuous mode in HW was failing. Beilei can you comment, is 
the additional description from Xiao enough?

>>
>>>>> +	 * fail. This would happen when the kernel driver requires a 
>>>>> reset
>>>>> +	 * of the VF.
>>>>> +	 */
>>>>> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
>>>>> +		vf->promisc_unicast_enabled = FALSE;
>>>>> +
>>>>>  	ret = i40evf_dev_init(dev);
>>>>>
>>>>>  	return ret;
>>>>> --
>>>>> 1.8.3.1
  

> -----Original Message-----
> From: Eelco Chaudron [mailto:echaudro@redhat.com]
> Sent: Tuesday, October 29, 2019 3:47 PM
> To: Zhang, Xiao <xiao.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF
> reset
> 
> 
> 
> On 29 Oct 2019, at 8:39, Zhang, Xiao wrote:
> 
> >> -----Original Message-----
> >> From: Eelco Chaudron <echaudro@redhat.com>
> >> Sent: Friday, October 25, 2019 5:24 PM
> >> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Xiao
> >> <xiao.zhang@intel.com>
> >> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state
> >> after VF reset
> >>
> >>
> >>
> >> On 17 Oct 2019, at 8:34, Xing, Beilei wrote:
> >>
> >>>> On 17 Sep 2019, at 9:40, Eelco Chaudron wrote:
> >>>>
> >>>>> Even though the device reset is successful, disabling promiscuous
> >>>>> mode might not always succeed, causing enabling it after reset to
> >>>>> fail.
> >>>>> This would happen when the kernel driver requires a reset of the
> >>>>> VF.
> >>>>>
> >>>>> This patch resets the internal state, so next time promiscuous
> >>>>> mode
> >>>>> is configured it will be enabled.
> >>>>>
> >>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> >>>>> ---
> >>>>>  drivers/net/i40e/i40e_ethdev_vf.c | 10 ++++++++++
> >>>>>  1 file changed, 10 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> >>>>> b/drivers/net/i40e/i40e_ethdev_vf.c
> >>>>> index 551f6fa..e0f99a4 100644
> >>>>> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> >>>>> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> >>>>> @@ -2276,11 +2276,21 @@ static int eth_i40evf_pci_remove(struct
> >>>>> rte_pci_device *pci_dev)  i40evf_dev_reset(struct rte_eth_dev
> >>>>> *dev)
> >>>>> {
> >>>>>  	int ret;
> >>>>> +	struct i40e_vf *vf =
> >>>>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> >>>>>
> >>>>>  	ret = i40evf_dev_uninit(dev);
> >>>>>  	if (ret)
> >>>>>  		return ret;
> >>>>>
> >>>>> +	/*
> >>>>> +	 * Even though the device reset is successful disabling
> >>>>> promiscuous
> >>>>> +	 * mode might not always succeed, causing enabling it after
> >>>>> reset
> >>>>> to
> >>>
> >>> I think we need to root cause why fail to disable promiscuous mode
> >>> and
> >>> try to fix it first.
> >>
> >> I’ve copied in Xiao who helped me identify the issue in your
> >> driver.
> >>
> >> The issue is because the change from kernel pf was not synced to DPDK
> >> vf during
> >> the closing period of reset, so we get this failure. Xiao can you add
> >> more details?
> >>
> >
> > The root cause is when kernel pf generate DPDK vf reset event, the
> > flag
> > vf->promisc_unicast_enabled will not be cleared but promiscuous mode
> > is
> > disabled. When trying to enable promiscuous mode next time by calling
> > i40evf_dev_promiscuous_enable won't work because it will check the
> > vf->promisc_unicast_enabled flag first.
> >
> > static int
> > i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev)
> > {
> > ...
> >     /* If enabled, just return */
> >     if (vf->promisc_unicast_enabled)
> >         return 0;
> > ...
> > }
> >
> > Hi Eelco,
> >
> > I think you may need add more detailed message in the commit log or
> > comments.
> 
> My interpretation of the request was that Beilei wanted to know why
> disabling promiscuous mode in HW was failing. Beilei can you comment, is
> the additional description from Xiao enough?

Yes, promisc_unicast_enabled flag is not cleared during vf reset because fail to disable promiscuous mode,
So I think we need to root cause why fail to  disable promiscuous mode first.
This patch looks like a workaround but not a fix.

> 
> >>
> >>>>> +	 * fail. This would happen when the kernel driver requires a
> >>>>> reset
> >>>>> +	 * of the VF.
> >>>>> +	 */
> >>>>> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> >>>>> +		vf->promisc_unicast_enabled = FALSE;
> >>>>> +
> >>>>>  	ret = i40evf_dev_init(dev);
> >>>>>
> >>>>>  	return ret;
> >>>>> --
> >>>>> 1.8.3.1
  

On 1 Nov 2019, at 3:38, Xing, Beilei wrote:

>> -----Original Message-----
>> From: Eelco Chaudron [mailto:echaudro@redhat.com]
>> Sent: Tuesday, October 29, 2019 3:47 PM
>> To: Zhang, Xiao <xiao.zhang@intel.com>; Xing, Beilei 
>> <beilei.xing@intel.com>
>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state 
>> after VF
>> reset
>>
>>
>>
>> On 29 Oct 2019, at 8:39, Zhang, Xiao wrote:
>>
>>>> -----Original Message-----
>>>> From: Eelco Chaudron <echaudro@redhat.com>
>>>> Sent: Friday, October 25, 2019 5:24 PM
>>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Xiao
>>>> <xiao.zhang@intel.com>
>>>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
>>>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state
>>>> after VF reset
>>>>
>>>>
>>>>
>>>> On 17 Oct 2019, at 8:34, Xing, Beilei wrote:
>>>>
>>>>>> On 17 Sep 2019, at 9:40, Eelco Chaudron wrote:
>>>>>>
>>>>>>> Even though the device reset is successful, disabling 
>>>>>>> promiscuous
>>>>>>> mode might not always succeed, causing enabling it after reset 
>>>>>>> to
>>>>>>> fail.
>>>>>>> This would happen when the kernel driver requires a reset of the
>>>>>>> VF.
>>>>>>>
>>>>>>> This patch resets the internal state, so next time promiscuous
>>>>>>> mode
>>>>>>> is configured it will be enabled.
>>>>>>>
>>>>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>>>>>> ---
>>>>>>>  drivers/net/i40e/i40e_ethdev_vf.c | 10 ++++++++++
>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
>>>>>>> b/drivers/net/i40e/i40e_ethdev_vf.c
>>>>>>> index 551f6fa..e0f99a4 100644
>>>>>>> --- a/drivers/net/i40e/i40e_ethdev_vf.c
>>>>>>> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
>>>>>>> @@ -2276,11 +2276,21 @@ static int eth_i40evf_pci_remove(struct
>>>>>>> rte_pci_device *pci_dev)  i40evf_dev_reset(struct rte_eth_dev
>>>>>>> *dev)
>>>>>>> {
>>>>>>>  	int ret;
>>>>>>> +	struct i40e_vf *vf =
>>>>>>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
>>>>>>>
>>>>>>>  	ret = i40evf_dev_uninit(dev);
>>>>>>>  	if (ret)
>>>>>>>  		return ret;
>>>>>>>
>>>>>>> +	/*
>>>>>>> +	 * Even though the device reset is successful disabling
>>>>>>> promiscuous
>>>>>>> +	 * mode might not always succeed, causing enabling it after
>>>>>>> reset
>>>>>>> to
>>>>>
>>>>> I think we need to root cause why fail to disable promiscuous mode
>>>>> and
>>>>> try to fix it first.
>>>>
>>>> I’ve copied in Xiao who helped me identify the issue in your
>>>> driver.
>>>>
>>>> The issue is because the change from kernel pf was not synced to 
>>>> DPDK
>>>> vf during
>>>> the closing period of reset, so we get this failure. Xiao can you 
>>>> add
>>>> more details?
>>>>
>>>
>>> The root cause is when kernel pf generate DPDK vf reset event, the
>>> flag
>>> vf->promisc_unicast_enabled will not be cleared but promiscuous mode
>>> is
>>> disabled. When trying to enable promiscuous mode next time by 
>>> calling
>>> i40evf_dev_promiscuous_enable won't work because it will check the
>>> vf->promisc_unicast_enabled flag first.
>>>
>>> static int
>>> i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev)
>>> {
>>> ...
>>>     /* If enabled, just return */
>>>     if (vf->promisc_unicast_enabled)
>>>         return 0;
>>> ...
>>> }
>>>
>>> Hi Eelco,
>>>
>>> I think you may need add more detailed message in the commit log or
>>> comments.
>>
>> My interpretation of the request was that Beilei wanted to know why
>> disabling promiscuous mode in HW was failing. Beilei can you comment, 
>> is
>> the additional description from Xiao enough?
>
> Yes, promisc_unicast_enabled flag is not cleared during vf reset 
> because fail to disable promiscuous mode,
> So I think we need to root cause why fail to  disable promiscuous mode 
> first.
> This patch looks like a workaround but not a fix.
>

This was debugged together with Xiao and from what I understand is that 
DPDK fails to reset promiscuous mode in hardware as PF and VF operations 
are not synced between kernel and DPDK.

Xiao told me this could not be fixed in another way, Xiao can you 
comment?

>>
>>>>
>>>>>>> +	 * fail. This would happen when the kernel driver requires a
>>>>>>> reset
>>>>>>> +	 * of the VF.
>>>>>>> +	 */
>>>>>>> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
>>>>>>> +		vf->promisc_unicast_enabled = FALSE;
>>>>>>> +
>>>>>>>  	ret = i40evf_dev_init(dev);
>>>>>>>
>>>>>>>  	return ret;
>>>>>>> --
>>>>>>> 1.8.3.1
  

Hi Eelco

> -----Original Message-----
> From: Eelco Chaudron [mailto:echaudro@redhat.com]
> Sent: Friday, November 1, 2019 4:13 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Xiao <xiao.zhang@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF
> reset
> 
> 
> 
> On 1 Nov 2019, at 3:38, Xing, Beilei wrote:
> 
> >> -----Original Message-----
> >> From: Eelco Chaudron [mailto:echaudro@redhat.com]
> >> Sent: Tuesday, October 29, 2019 3:47 PM
> >> To: Zhang, Xiao <xiao.zhang@intel.com>; Xing, Beilei
> >> <beilei.xing@intel.com>
> >> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state
> >> after VF reset
> >>
> >>
> >>
> >> On 29 Oct 2019, at 8:39, Zhang, Xiao wrote:
> >>
> >>>> -----Original Message-----
> >>>> From: Eelco Chaudron <echaudro@redhat.com>
> >>>> Sent: Friday, October 25, 2019 5:24 PM
> >>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Xiao
> >>>> <xiao.zhang@intel.com>
> >>>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> >>>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state
> >>>> after VF reset
> >>>>
> >>>>
> >>>>
> >>>> On 17 Oct 2019, at 8:34, Xing, Beilei wrote:
> >>>>
> >>>>>> On 17 Sep 2019, at 9:40, Eelco Chaudron wrote:
> >>>>>>
> >>>>>>> Even though the device reset is successful, disabling
> >>>>>>> promiscuous
> >>>>>>> mode might not always succeed, causing enabling it after reset
> >>>>>>> to
> >>>>>>> fail.
> >>>>>>> This would happen when the kernel driver requires a reset of the
> >>>>>>> VF.
> >>>>>>>
> >>>>>>> This patch resets the internal state, so next time promiscuous
> >>>>>>> mode
> >>>>>>> is configured it will be enabled.
> >>>>>>>
> >>>>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> >>>>>>> ---
> >>>>>>>  drivers/net/i40e/i40e_ethdev_vf.c | 10 ++++++++++
> >>>>>>>  1 file changed, 10 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> >>>>>>> b/drivers/net/i40e/i40e_ethdev_vf.c
> >>>>>>> index 551f6fa..e0f99a4 100644
> >>>>>>> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> >>>>>>> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> >>>>>>> @@ -2276,11 +2276,21 @@ static int eth_i40evf_pci_remove(struct
> >>>>>>> rte_pci_device *pci_dev)  i40evf_dev_reset(struct rte_eth_dev
> >>>>>>> *dev)
> >>>>>>> {
> >>>>>>>  	int ret;
> >>>>>>> +	struct i40e_vf *vf =
> >>>>>>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> >>>>>>>
> >>>>>>>  	ret = i40evf_dev_uninit(dev);
> >>>>>>>  	if (ret)
> >>>>>>>  		return ret;
> >>>>>>>
> >>>>>>> +	/*
> >>>>>>> +	 * Even though the device reset is successful disabling
> >>>>>>> promiscuous
> >>>>>>> +	 * mode might not always succeed, causing enabling it after
> >>>>>>> reset
> >>>>>>> to
> >>>>>
> >>>>> I think we need to root cause why fail to disable promiscuous mode
> >>>>> and
> >>>>> try to fix it first.
> >>>>
> >>>> I’ve copied in Xiao who helped me identify the issue in your
> >>>> driver.
> >>>>
> >>>> The issue is because the change from kernel pf was not synced to
> >>>> DPDK
> >>>> vf during
> >>>> the closing period of reset, so we get this failure. Xiao can you
> >>>> add
> >>>> more details?
> >>>>
> >>>
> >>> The root cause is when kernel pf generate DPDK vf reset event, the
> >>> flag
> >>> vf->promisc_unicast_enabled will not be cleared but promiscuous mode
> >>> is
> >>> disabled. When trying to enable promiscuous mode next time by
> >>> calling
> >>> i40evf_dev_promiscuous_enable won't work because it will check the
> >>> vf->promisc_unicast_enabled flag first.
> >>>
> >>> static int
> >>> i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev)
> >>> {
> >>> ...
> >>>     /* If enabled, just return */
> >>>     if (vf->promisc_unicast_enabled)
> >>>         return 0;
> >>> ...
> >>> }
> >>>
> >>> Hi Eelco,
> >>>
> >>> I think you may need add more detailed message in the commit log or
> >>> comments.
> >>
> >> My interpretation of the request was that Beilei wanted to know why
> >> disabling promiscuous mode in HW was failing. Beilei can you comment,
> >> is
> >> the additional description from Xiao enough?
> >
> > Yes, promisc_unicast_enabled flag is not cleared during vf reset
> > because fail to disable promiscuous mode,
> > So I think we need to root cause why fail to  disable promiscuous mode
> > first.
> > This patch looks like a workaround but not a fix.
> >
> 
> This was debugged together with Xiao and from what I understand is that
> DPDK fails to reset promiscuous mode in hardware as PF and VF operations
> are not synced between kernel and DPDK.
> 
> Xiao told me this could not be fixed in another way, Xiao can you
> comment?
> 

Checked again, the root cause is not synced issue between kernel and DPDK during reset.
Suggest to remove the checking and setting of promisc_unicast_enabled flag, since this flag is only used when enable/disable promiscuous mode.
Considering the un-synced issue, it will be more clean if remove the flag.
Also same with flag promisc_multicast_enabled.

> >>
> >>>>
> >>>>>>> +	 * fail. This would happen when the kernel driver requires a
> >>>>>>> reset
> >>>>>>> +	 * of the VF.
> >>>>>>> +	 */
> >>>>>>> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> >>>>>>> +		vf->promisc_unicast_enabled = FALSE;
> >>>>>>> +
> >>>>>>>  	ret = i40evf_dev_init(dev);
> >>>>>>>
> >>>>>>>  	return ret;
> >>>>>>> --
> >>>>>>> 1.8.3.1
  

On 1 Nov 2019, at 9:12, Eelco Chaudron wrote:

<SNIP>

>>>>>>>>
>>>>>>>> +	/*
>>>>>>>> +	 * Even though the device reset is successful disabling
>>>>>>>> promiscuous
>>>>>>>> +	 * mode might not always succeed, causing enabling it after
>>>>>>>> reset
>>>>>>>> to
>>>>>>
>>>>>> I think we need to root cause why fail to disable promiscuous 
>>>>>> mode
>>>>>> and
>>>>>> try to fix it first.
>>>>>
>>>>> I’ve copied in Xiao who helped me identify the issue in your
>>>>> driver.
>>>>>
>>>>> The issue is because the change from kernel pf was not synced to 
>>>>> DPDK
>>>>> vf during
>>>>> the closing period of reset, so we get this failure. Xiao can you 
>>>>> add
>>>>> more details?
>>>>>
>>>>
>>>> The root cause is when kernel pf generate DPDK vf reset event, the
>>>> flag
>>>> vf->promisc_unicast_enabled will not be cleared but promiscuous 
>>>> mode
>>>> is
>>>> disabled. When trying to enable promiscuous mode next time by 
>>>> calling
>>>> i40evf_dev_promiscuous_enable won't work because it will check the
>>>> vf->promisc_unicast_enabled flag first.
>>>>
>>>> static int
>>>> i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev)
>>>> {
>>>> ...
>>>>     /* If enabled, just return */
>>>>     if (vf->promisc_unicast_enabled)
>>>>         return 0;
>>>> ...
>>>> }
>>>>
>>>> Hi Eelco,
>>>>
>>>> I think you may need add more detailed message in the commit log or
>>>> comments.
>>>
>>> My interpretation of the request was that Beilei wanted to know why
>>> disabling promiscuous mode in HW was failing. Beilei can you 
>>> comment, is
>>> the additional description from Xiao enough?
>>
>> Yes, promisc_unicast_enabled flag is not cleared during vf reset 
>> because fail to disable promiscuous mode,
>> So I think we need to root cause why fail to  disable promiscuous 
>> mode first.
>> This patch looks like a workaround but not a fix.
>>
>
> This was debugged together with Xiao and from what I understand is 
> that DPDK fails to reset promiscuous mode in hardware as PF and VF 
> operations are not synced between kernel and DPDK.
>
> Xiao told me this could not be fixed in another way, Xiao can you 
> comment?
>

Xiao any update you can add to this so it’s more clear for Beilei?

Thanks,

Eelco
  

Hi Eelco,

Seems you missed this mail.

Thanks,
Xiao

> -----Original Message-----
> From: Zhang, Xiao
> Sent: Wednesday, November 6, 2019 12:58 PM
> To: Eelco Chaudron <echaudro@redhat.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF
> reset
> 
> Hi Eelco
> 
> > -----Original Message-----
> > From: Eelco Chaudron [mailto:echaudro@redhat.com]
> > Sent: Friday, November 1, 2019 4:13 PM
> > To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Xiao
> > <xiao.zhang@intel.com>
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state
> > after VF reset
> >
> >
> >
> > On 1 Nov 2019, at 3:38, Xing, Beilei wrote:
> >
> > >> -----Original Message-----
> > >> From: Eelco Chaudron [mailto:echaudro@redhat.com]
> > >> Sent: Tuesday, October 29, 2019 3:47 PM
> > >> To: Zhang, Xiao <xiao.zhang@intel.com>; Xing, Beilei
> > >> <beilei.xing@intel.com>
> > >> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> > >> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state
> > >> after VF reset
> > >>
> > >>
> > >>
> > >> On 29 Oct 2019, at 8:39, Zhang, Xiao wrote:
> > >>
> > >>>> -----Original Message-----
> > >>>> From: Eelco Chaudron <echaudro@redhat.com>
> > >>>> Sent: Friday, October 25, 2019 5:24 PM
> > >>>> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Xiao
> > >>>> <xiao.zhang@intel.com>
> > >>>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> > >>>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state
> > >>>> after VF reset
> > >>>>
> > >>>>
> > >>>>
> > >>>> On 17 Oct 2019, at 8:34, Xing, Beilei wrote:
> > >>>>
> > >>>>>> On 17 Sep 2019, at 9:40, Eelco Chaudron wrote:
> > >>>>>>
> > >>>>>>> Even though the device reset is successful, disabling
> > >>>>>>> promiscuous mode might not always succeed, causing enabling it
> > >>>>>>> after reset to fail.
> > >>>>>>> This would happen when the kernel driver requires a reset of
> > >>>>>>> the VF.
> > >>>>>>>
> > >>>>>>> This patch resets the internal state, so next time promiscuous
> > >>>>>>> mode is configured it will be enabled.
> > >>>>>>>
> > >>>>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> > >>>>>>> ---
> > >>>>>>>  drivers/net/i40e/i40e_ethdev_vf.c | 10 ++++++++++
> > >>>>>>>  1 file changed, 10 insertions(+)
> > >>>>>>>
> > >>>>>>> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > >>>>>>> b/drivers/net/i40e/i40e_ethdev_vf.c
> > >>>>>>> index 551f6fa..e0f99a4 100644
> > >>>>>>> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > >>>>>>> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > >>>>>>> @@ -2276,11 +2276,21 @@ static int
> > >>>>>>> eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
> > >>>>>>> i40evf_dev_reset(struct rte_eth_dev
> > >>>>>>> *dev)
> > >>>>>>> {
> > >>>>>>>  	int ret;
> > >>>>>>> +	struct i40e_vf *vf =
> > >>>>>>> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> > >>>>>>>
> > >>>>>>>  	ret = i40evf_dev_uninit(dev);
> > >>>>>>>  	if (ret)
> > >>>>>>>  		return ret;
> > >>>>>>>
> > >>>>>>> +	/*
> > >>>>>>> +	 * Even though the device reset is successful disabling
> > >>>>>>> promiscuous
> > >>>>>>> +	 * mode might not always succeed, causing enabling it after
> > >>>>>>> reset
> > >>>>>>> to
> > >>>>>
> > >>>>> I think we need to root cause why fail to disable promiscuous
> > >>>>> mode and try to fix it first.
> > >>>>
> > >>>> I’ve copied in Xiao who helped me identify the issue in your
> > >>>> driver.
> > >>>>
> > >>>> The issue is because the change from kernel pf was not synced to
> > >>>> DPDK vf during the closing period of reset, so we get this
> > >>>> failure. Xiao can you add more details?
> > >>>>
> > >>>
> > >>> The root cause is when kernel pf generate DPDK vf reset event, the
> > >>> flag
> > >>> vf->promisc_unicast_enabled will not be cleared but promiscuous
> > >>> vf->mode
> > >>> is
> > >>> disabled. When trying to enable promiscuous mode next time by
> > >>> calling i40evf_dev_promiscuous_enable won't work because it will
> > >>> check the
> > >>> vf->promisc_unicast_enabled flag first.
> > >>>
> > >>> static int
> > >>> i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev) { ...
> > >>>     /* If enabled, just return */
> > >>>     if (vf->promisc_unicast_enabled)
> > >>>         return 0;
> > >>> ...
> > >>> }
> > >>>
> > >>> Hi Eelco,
> > >>>
> > >>> I think you may need add more detailed message in the commit log
> > >>> or comments.
> > >>
> > >> My interpretation of the request was that Beilei wanted to know why
> > >> disabling promiscuous mode in HW was failing. Beilei can you
> > >> comment, is the additional description from Xiao enough?
> > >
> > > Yes, promisc_unicast_enabled flag is not cleared during vf reset
> > > because fail to disable promiscuous mode, So I think we need to root
> > > cause why fail to  disable promiscuous mode first.
> > > This patch looks like a workaround but not a fix.
> > >
> >
> > This was debugged together with Xiao and from what I understand is
> > that DPDK fails to reset promiscuous mode in hardware as PF and VF
> > operations are not synced between kernel and DPDK.
> >
> > Xiao told me this could not be fixed in another way, Xiao can you
> > comment?
> >
> 
> Checked again, the root cause is not synced issue between kernel and DPDK
> during reset.
> Suggest to remove the checking and setting of promisc_unicast_enabled flag,
> since this flag is only used when enable/disable promiscuous mode.
> Considering the un-synced issue, it will be more clean if remove the flag.
> Also same with flag promisc_multicast_enabled.
> 
> > >>
> > >>>>
> > >>>>>>> +	 * fail. This would happen when the kernel driver requires a
> > >>>>>>> reset
> > >>>>>>> +	 * of the VF.
> > >>>>>>> +	 */
> > >>>>>>> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> > >>>>>>> +		vf->promisc_unicast_enabled = FALSE;
> > >>>>>>> +
> > >>>>>>>  	ret = i40evf_dev_init(dev);
> > >>>>>>>
> > >>>>>>>  	return ret;
> > >>>>>>> --
> > >>>>>>> 1.8.3.1
  

On 12 Nov 2019, at 1:52, Zhang, Xiao wrote:

> Hi Eelco,
>
> Seems you missed this mail.

Not sure why I missed this email, as it does not show up in my email 
client :(
See below…


<SNIP>
>>>>>> Hi Eelco,
>>>>>>
>>>>>> I think you may need add more detailed message in the commit log
>>>>>> or comments.
>>>>>
>>>>> My interpretation of the request was that Beilei wanted to know 
>>>>> why
>>>>> disabling promiscuous mode in HW was failing. Beilei can you
>>>>> comment, is the additional description from Xiao enough?
>>>>
>>>> Yes, promisc_unicast_enabled flag is not cleared during vf reset
>>>> because fail to disable promiscuous mode, So I think we need to 
>>>> root
>>>> cause why fail to  disable promiscuous mode first.
>>>> This patch looks like a workaround but not a fix.
>>>>
>>>
>>> This was debugged together with Xiao and from what I understand is
>>> that DPDK fails to reset promiscuous mode in hardware as PF and VF
>>> operations are not synced between kernel and DPDK.
>>>
>>> Xiao told me this could not be fixed in another way, Xiao can you
>>> comment?
>>>
>>
>> Checked again, the root cause is not synced issue between kernel and 
>> DPDK
>> during reset.

What is the root cause, so I can update the patch description for the 
next revision.

>> Suggest to remove the checking and setting of promisc_unicast_enabled 
>> flag,
>> since this flag is only used when enable/disable promiscuous mode.
>> Considering the un-synced issue, it will be more clean if remove the 
>> flag.
>> Also same with flag promisc_multicast_enabled.

So if I understand it correctly remove the following code:

2203  	/* If enabled, just return */
2204  	if (vf->promisc_multicast_enabled)
2205  		return 0;

and

2222  	/* If enabled, just return */
2223  	if (!vf->promisc_multicast_enabled)
2224  		return 0;

Or to remove the flags from the i40e_vf structure (and relative  code):

1051  	bool promisc_unicast_enabled;
1052  	bool promisc_multicast_enabled;

Let me know and I craft up a patch…


Cheers,


Eelco
  

> -----Original Message-----
> From: Eelco Chaudron [mailto:echaudro@redhat.com]
> Sent: Tuesday, November 12, 2019 7:09 PM
> To: Zhang, Xiao <xiao.zhang@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org; Xing, Beilei
> <beilei.xing@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF
> reset
> 
> 
> 
> On 12 Nov 2019, at 1:52, Zhang, Xiao wrote:
> 
> > Hi Eelco,
> >
> > Seems you missed this mail.
> 
> Not sure why I missed this email, as it does not show up in my email client :( See
> below…
> 
> 
> <SNIP>
> >>>>>> Hi Eelco,
> >>>>>>
> >>>>>> I think you may need add more detailed message in the commit log
> >>>>>> or comments.
> >>>>>
> >>>>> My interpretation of the request was that Beilei wanted to know
> >>>>> why
> >>>>> disabling promiscuous mode in HW was failing. Beilei can you
> >>>>> comment, is the additional description from Xiao enough?
> >>>>
> >>>> Yes, promisc_unicast_enabled flag is not cleared during vf reset
> >>>> because fail to disable promiscuous mode, So I think we need to
> >>>> root
> >>>> cause why fail to  disable promiscuous mode first.
> >>>> This patch looks like a workaround but not a fix.
> >>>>
> >>>
> >>> This was debugged together with Xiao and from what I understand is
> >>> that DPDK fails to reset promiscuous mode in hardware as PF and VF
> >>> operations are not synced between kernel and DPDK.
> >>>
> >>> Xiao told me this could not be fixed in another way, Xiao can you
> >>> comment?
> >>>
> >>
> >> Checked again, the root cause is not synced issue between kernel and
> >> DPDK
> >> during reset.
> 
> What is the root cause, so I can update the patch description for the
> next revision.

Kernel PF do reset once VF mac changed and send reset event to VF,
VF do reset once received even if reset task not done by PF, so admin q message will not get response.

> 
> >> Suggest to remove the checking and setting of promisc_unicast_enabled
> >> flag,
> >> since this flag is only used when enable/disable promiscuous mode.
> >> Considering the un-synced issue, it will be more clean if remove the
> >> flag.
> >> Also same with flag promisc_multicast_enabled.
> 
> So if I understand it correctly remove the following code:
> 
> 2203  	/* If enabled, just return */
> 2204  	if (vf->promisc_multicast_enabled)
> 2205  		return 0;
> 
> and
> 
> 2222  	/* If enabled, just return */
> 2223  	if (!vf->promisc_multicast_enabled)
> 2224  		return 0;
> 
> Or to remove the flags from the i40e_vf structure (and relative  code):
> 
> 1051  	bool promisc_unicast_enabled;
> 1052  	bool promisc_multicast_enabled;
> 
> Let me know and I craft up a patch…

You can remove promisc_unicast_enabled/promisc_multicast_enabled and related code.

Thanks,
Xiao
> 
> 
> Cheers,
> 
> 
> Eelco
  

On 13 Nov 2019, at 2:14, Zhang, Xiao wrote:

>> -----Original Message-----
>> From: Eelco Chaudron [mailto:echaudro@redhat.com]
>> Sent: Tuesday, November 12, 2019 7:09 PM
>> To: Zhang, Xiao <xiao.zhang@intel.com>
>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org; Xing, Beilei
>> <beilei.xing@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state 
>> after VF
>> reset
>>
>>
>>
>> On 12 Nov 2019, at 1:52, Zhang, Xiao wrote:
>>
>>> Hi Eelco,
>>>
>>> Seems you missed this mail.
>>
>> Not sure why I missed this email, as it does not show up in my email 
>> client :( See
>> below…
>>
>>
>> <SNIP>
>>>>>>>> Hi Eelco,
>>>>>>>>
>>>>>>>> I think you may need add more detailed message in the commit 
>>>>>>>> log
>>>>>>>> or comments.
>>>>>>>
>>>>>>> My interpretation of the request was that Beilei wanted to know
>>>>>>> why
>>>>>>> disabling promiscuous mode in HW was failing. Beilei can you
>>>>>>> comment, is the additional description from Xiao enough?
>>>>>>
>>>>>> Yes, promisc_unicast_enabled flag is not cleared during vf reset
>>>>>> because fail to disable promiscuous mode, So I think we need to
>>>>>> root
>>>>>> cause why fail to  disable promiscuous mode first.
>>>>>> This patch looks like a workaround but not a fix.
>>>>>>
>>>>>
>>>>> This was debugged together with Xiao and from what I understand is
>>>>> that DPDK fails to reset promiscuous mode in hardware as PF and VF
>>>>> operations are not synced between kernel and DPDK.
>>>>>
>>>>> Xiao told me this could not be fixed in another way, Xiao can you
>>>>> comment?
>>>>>
>>>>
>>>> Checked again, the root cause is not synced issue between kernel 
>>>> and
>>>> DPDK
>>>> during reset.
>>
>> What is the root cause, so I can update the patch description for the
>> next revision.
>
> Kernel PF do reset once VF mac changed and send reset event to VF,
> VF do reset once received even if reset task not done by PF, so admin 
> q message will not get response.

Thanks, added this to the v2 commit message.

>>
>>>> Suggest to remove the checking and setting of 
>>>> promisc_unicast_enabled
>>>> flag,
>>>> since this flag is only used when enable/disable promiscuous mode.
>>>> Considering the un-synced issue, it will be more clean if remove 
>>>> the
>>>> flag.
>>>> Also same with flag promisc_multicast_enabled.
>>
>> So if I understand it correctly remove the following code:
>>
>> 2203  	/* If enabled, just return */
>> 2204  	if (vf->promisc_multicast_enabled)
>> 2205  		return 0;
>>
>> and
>>
>> 2222  	/* If enabled, just return */
>> 2223  	if (!vf->promisc_multicast_enabled)
>> 2224  		return 0;
>>
>> Or to remove the flags from the i40e_vf structure (and relative  
>> code):
>>
>> 1051  	bool promisc_unicast_enabled;
>> 1052  	bool promisc_multicast_enabled;
>>
>> Let me know and I craft up a patch…
>
> You can remove promisc_unicast_enabled/promisc_multicast_enabled and 
> related code.

Removing the flags did not work, as they are needed for programming one 
or the other, so I kept them.
In the end, I removed the “If enabled” checks above, see the v2 
patch with the name
“[PATCH v2] net/i40e: always re-program promiscuous mode on VF 
interface”