* [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
@ 2019-09-17 7:40 Eelco Chaudron
2019-10-15 10:31 ` Eelco Chaudron
0 siblings, 1 reply; 14+ messages in thread
From: Eelco Chaudron @ 2019-09-17 7:40 UTC (permalink / raw)
To: beilei.xing, qi.z.zhang; +Cc: dev
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-09-17 7:40 [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset Eelco Chaudron
@ 2019-10-15 10:31 ` Eelco Chaudron
2019-10-17 6:34 ` Xing, Beilei
0 siblings, 1 reply; 14+ messages in thread
From: Eelco Chaudron @ 2019-10-15 10:31 UTC (permalink / raw)
To: beilei.xing, qi.z.zhang; +Cc: dev
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-10-15 10:31 ` Eelco Chaudron
@ 2019-10-17 6:34 ` Xing, Beilei
2019-10-25 9:23 ` Eelco Chaudron
0 siblings, 1 reply; 14+ messages in thread
From: Xing, Beilei @ 2019-10-17 6:34 UTC (permalink / raw)
To: Eelco Chaudron, Zhang, Qi Z; +Cc: dev
> -----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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-10-17 6:34 ` Xing, Beilei
@ 2019-10-25 9:23 ` Eelco Chaudron
2019-10-29 7:39 ` Zhang, Xiao
0 siblings, 1 reply; 14+ messages in thread
From: Eelco Chaudron @ 2019-10-25 9:23 UTC (permalink / raw)
To: Xing, Beilei, Zhang, Xiao; +Cc: Zhang, Qi Z, dev
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-10-25 9:23 ` Eelco Chaudron
@ 2019-10-29 7:39 ` Zhang, Xiao
2019-10-29 7:47 ` Eelco Chaudron
0 siblings, 1 reply; 14+ messages in thread
From: Zhang, Xiao @ 2019-10-29 7:39 UTC (permalink / raw)
To: Eelco Chaudron, Xing, Beilei; +Cc: Zhang, Qi Z, dev
> -----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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-10-29 7:39 ` Zhang, Xiao
@ 2019-10-29 7:47 ` Eelco Chaudron
2019-11-01 2:38 ` Xing, Beilei
0 siblings, 1 reply; 14+ messages in thread
From: Eelco Chaudron @ 2019-10-29 7:47 UTC (permalink / raw)
To: Zhang, Xiao, Xing, Beilei; +Cc: Zhang, Qi Z, dev
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-10-29 7:47 ` Eelco Chaudron
@ 2019-11-01 2:38 ` Xing, Beilei
2019-11-01 8:12 ` Eelco Chaudron
0 siblings, 1 reply; 14+ messages in thread
From: Xing, Beilei @ 2019-11-01 2:38 UTC (permalink / raw)
To: Eelco Chaudron, Zhang, Xiao; +Cc: Zhang, Qi Z, dev
> -----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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-11-01 2:38 ` Xing, Beilei
@ 2019-11-01 8:12 ` Eelco Chaudron
2019-11-06 4:58 ` Zhang, Xiao
2019-11-11 13:59 ` Eelco Chaudron
0 siblings, 2 replies; 14+ messages in thread
From: Eelco Chaudron @ 2019-11-01 8:12 UTC (permalink / raw)
To: Xing, Beilei, Zhang, Xiao; +Cc: Zhang, Qi Z, dev
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-11-01 8:12 ` Eelco Chaudron
@ 2019-11-06 4:58 ` Zhang, Xiao
2019-11-12 0:52 ` Zhang, Xiao
2019-11-11 13:59 ` Eelco Chaudron
1 sibling, 1 reply; 14+ messages in thread
From: Zhang, Xiao @ 2019-11-06 4:58 UTC (permalink / raw)
To: Eelco Chaudron, Xing, Beilei; +Cc: Zhang, Qi Z, dev
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-11-01 8:12 ` Eelco Chaudron
2019-11-06 4:58 ` Zhang, Xiao
@ 2019-11-11 13:59 ` Eelco Chaudron
1 sibling, 0 replies; 14+ messages in thread
From: Eelco Chaudron @ 2019-11-11 13:59 UTC (permalink / raw)
To: Zhang, Xiao; +Cc: Zhang, Qi Z, dev, Xing, Beilei
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-11-06 4:58 ` Zhang, Xiao
@ 2019-11-12 0:52 ` Zhang, Xiao
2019-11-12 11:09 ` Eelco Chaudron
0 siblings, 1 reply; 14+ messages in thread
From: Zhang, Xiao @ 2019-11-12 0:52 UTC (permalink / raw)
To: 'Eelco Chaudron'
Cc: Zhang, Qi Z, 'dev@dpdk.org', Xing, Beilei
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-11-12 0:52 ` Zhang, Xiao
@ 2019-11-12 11:09 ` Eelco Chaudron
2019-11-13 1:14 ` Zhang, Xiao
0 siblings, 1 reply; 14+ messages in thread
From: Eelco Chaudron @ 2019-11-12 11:09 UTC (permalink / raw)
To: Zhang, Xiao; +Cc: Zhang, Qi Z, dev, Xing, Beilei
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-11-12 11:09 ` Eelco Chaudron
@ 2019-11-13 1:14 ` Zhang, Xiao
2019-11-19 13:55 ` Eelco Chaudron
0 siblings, 1 reply; 14+ messages in thread
From: Zhang, Xiao @ 2019-11-13 1:14 UTC (permalink / raw)
To: Eelco Chaudron; +Cc: Zhang, Qi Z, dev, Xing, Beilei
> -----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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset
2019-11-13 1:14 ` Zhang, Xiao
@ 2019-11-19 13:55 ` Eelco Chaudron
0 siblings, 0 replies; 14+ messages in thread
From: Eelco Chaudron @ 2019-11-19 13:55 UTC (permalink / raw)
To: Zhang, Xiao; +Cc: Zhang, Qi Z, dev, Xing, Beilei
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”
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-11-19 13:55 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-17 7:40 [dpdk-dev] [PATCH] net/i40e: force promiscuous state after VF reset Eelco Chaudron
2019-10-15 10:31 ` Eelco Chaudron
2019-10-17 6:34 ` Xing, Beilei
2019-10-25 9:23 ` Eelco Chaudron
2019-10-29 7:39 ` Zhang, Xiao
2019-10-29 7:47 ` Eelco Chaudron
2019-11-01 2:38 ` Xing, Beilei
2019-11-01 8:12 ` Eelco Chaudron
2019-11-06 4:58 ` Zhang, Xiao
2019-11-12 0:52 ` Zhang, Xiao
2019-11-12 11:09 ` Eelco Chaudron
2019-11-13 1:14 ` Zhang, Xiao
2019-11-19 13:55 ` Eelco Chaudron
2019-11-11 13:59 ` Eelco Chaudron
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).