Hi Eelco > -----Original Message----- > From: Eelco Chaudron [mailto:echau...@redhat.com] > Sent: Friday, November 1, 2019 4:13 PM > To: Xing, Beilei <beilei.x...@intel.com>; Zhang, Xiao <xiao.zh...@intel.com> > Cc: Zhang, Qi Z <qi.z.zh...@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:echau...@redhat.com] > >> Sent: Tuesday, October 29, 2019 3:47 PM > >> To: Zhang, Xiao <xiao.zh...@intel.com>; Xing, Beilei > >> <beilei.x...@intel.com> > >> Cc: Zhang, Qi Z <qi.z.zh...@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 <echau...@redhat.com> > >>>> Sent: Friday, October 25, 2019 5:24 PM > >>>> To: Xing, Beilei <beilei.x...@intel.com>; Zhang, Xiao > >>>> <xiao.zh...@intel.com> > >>>> Cc: Zhang, Qi Z <qi.z.zh...@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 <echau...@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
