[dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c: fix missing promiscuous disable at device stop

[dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c: fix missing promiscuous disable at device stop

[Rosen Xu]

In scenario of Kernel Driver runs on PF and PMD runs on VF, PMD exit doesn't
disable promiscuous mode, this will cause vlan filter set by Kernel Driver
will not take effect.

This patch will fix it, add promiscuous disable at device stop.

Signed-off-by: Rosen Xu <rosen.xu@intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index fd003fe..f395b02 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2051,6 +2051,8 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
 
 	if (hw->adapter_stopped == 1)
 		return;
+	i40evf_dev_promiscuous_disable(dev);
+	i40evf_dev_allmulticast_disable(dev);
 	i40evf_stop_queues(dev);
 	i40evf_disable_queues_intr(dev);
 	i40e_dev_clear_queues(dev);
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c: fix missing promiscuous disable at device stop

[Zhang, Qi Z]

Hi Rosen:

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> Sent: Thursday, March 15, 2018 5:46 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c: fix
> missing promiscuous disable at device stop
> 
> In scenario of Kernel Driver runs on PF and PMD runs on VF, PMD exit doesn't
> disable promiscuous mode, this will cause vlan filter set by Kernel Driver will
> not take effect.
> 
> This patch will fix it, add promiscuous disable at device stop.
> 
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index fd003fe..f395b02 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -2051,6 +2051,8 @@ static int eth_i40evf_pci_remove(struct
> rte_pci_device *pci_dev)
> 
>  	if (hw->adapter_stopped == 1)
>  		return;
> +	i40evf_dev_promiscuous_disable(dev);
> +	i40evf_dev_allmulticast_disable(dev);

Device's promiscuous mode is not expected to be changed in a dev_start/dev_stop cycle
Application need to call rte_eth_promiscuous_disable and i40evf_dev_allmulticast_disable to change it explicitly.

Regards
Qi

>  	i40evf_stop_queues(dev);
>  	i40evf_disable_queues_intr(dev);
>  	i40e_dev_clear_queues(dev);
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c: fix missing promiscuous disable at device stop

[Zhang, Helin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> Sent: Friday, March 16, 2018 2:25 PM
> To: Xu, Rosen; Xing, Beilei
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c: fix
> missing promiscuous disable at device stop
> 
> Hi Rosen:
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> > Sent: Thursday, March 15, 2018 5:46 PM
> > To: Xing, Beilei <beilei.xing@intel.com>
> > Cc: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c: fix
> > missing promiscuous disable at device stop
> >
> > In scenario of Kernel Driver runs on PF and PMD runs on VF, PMD exit
> > doesn't disable promiscuous mode, this will cause vlan filter set by
> > Kernel Driver will not take effect.
> >
> > This patch will fix it, add promiscuous disable at device stop.
> >
> > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index fd003fe..f395b02 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -2051,6 +2051,8 @@ static int eth_i40evf_pci_remove(struct
> > rte_pci_device *pci_dev)
> >
> >  	if (hw->adapter_stopped == 1)
> >  		return;
> > +	i40evf_dev_promiscuous_disable(dev);
> > +	i40evf_dev_allmulticast_disable(dev);
> 
> Device's promiscuous mode is not expected to be changed in a
> dev_start/dev_stop cycle Application need to call
> rte_eth_promiscuous_disable and i40evf_dev_allmulticast_disable to change it
> explicitly.
> 
> Regards
> Qi

Basically I'd reject your patch, based on the comments Qi made above.

/Helin
> 
> >  	i40evf_stop_queues(dev);
> >  	i40evf_disable_queues_intr(dev);
> >  	i40e_dev_clear_queues(dev);
> > --
> > 1.8.3.1

Re: [dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c: fix missing promiscuous disable at device stop

[Xu, Rosen]

Hi Helin,

> -----Original Message-----
> From: Zhang, Helin
> Sent: Thursday, March 29, 2018 13:11
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Xu, Rosen <rosen.xu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c: fix
> missing promiscuous disable at device stop
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> > Sent: Friday, March 16, 2018 2:25 PM
> > To: Xu, Rosen; Xing, Beilei
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c:
> > fix missing promiscuous disable at device stop
> >
> > Hi Rosen:
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> > > Sent: Thursday, March 15, 2018 5:46 PM
> > > To: Xing, Beilei <beilei.xing@intel.com>
> > > Cc: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH v2] drivers/net/i40e/i40e_ethdev_vf.c:
> > > fix missing promiscuous disable at device stop
> > >
> > > In scenario of Kernel Driver runs on PF and PMD runs on VF, PMD exit
> > > doesn't disable promiscuous mode, this will cause vlan filter set by
> > > Kernel Driver will not take effect.
> > >
> > > This patch will fix it, add promiscuous disable at device stop.
> > >
> > > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > > ---
> > >  drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > > b/drivers/net/i40e/i40e_ethdev_vf.c
> > > index fd003fe..f395b02 100644
> > > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > > @@ -2051,6 +2051,8 @@ static int eth_i40evf_pci_remove(struct
> > > rte_pci_device *pci_dev)
> > >
> > >  	if (hw->adapter_stopped == 1)
> > >  		return;
> > > +	i40evf_dev_promiscuous_disable(dev);
> > > +	i40evf_dev_allmulticast_disable(dev);
> >
> > Device's promiscuous mode is not expected to be changed in a
> > dev_start/dev_stop cycle Application need to call
> > rte_eth_promiscuous_disable and i40evf_dev_allmulticast_disable to
> > change it explicitly.
> >
> > Regards
> > Qi

This modification comes from some customers, they don't want to take this work in their application.

> Basically I'd reject your patch, based on the comments Qi made above.
> 
> /Helin

I have aligned with Jingjing and Qi, their proposal is to take this modification in vf Init, but after I have
checked kernel driver, I find if we don't disable promiscuous mode before any call i40evf_reset_vf(),
i40evf_reset_vf() will cause open promiscuous mode. So we only take this modification in dev_disable.

> >
> > >  	i40evf_stop_queues(dev);
> > >  	i40evf_disable_queues_intr(dev);
> > >  	i40e_dev_clear_queues(dev);
> > > --
> > > 1.8.3.1