* [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-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
* 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
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).