diff mbox

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

Message ID 1521107140-68349-1-git-send-email-rosen.xu@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Helin Zhang
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Xu, Rosen March 15, 2018, 9:45 a.m. UTC 
  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(+)

Comments

Qi Zhang March 16, 2018, 6:25 a.m. UTC | #1 
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
Zhang, Helin March 29, 2018, 5:10 a.m. UTC | #2 
> -----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
Xu, Rosen April 17, 2018, 3:17 a.m. UTC | #3 
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
diff mbox

Patch

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);