diff mbox series

net/i40e: add promiscuous configure unsupported check

Message ID 1582860124-48237-1-git-send-email-xiao.zhang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: xiaolong ye
Headers
Series net/i40e: add promiscuous configure unsupported check |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing warning Testing issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed

Commit Message

Xiao Zhang Feb. 28, 2020, 3:22 a.m. UTC 
  Return ENOTSUP error code when configuring i40evf promiscuous mode to
fix port start hang issue on platforms which are unsupported to configure
promiscuous mode.

Fixes: ddc7cb0d9453 ("net/i40e: re-program promiscuous mode on VF
interface")
Cc: stable@dpdk.org

Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Xiaolong Ye March 2, 2020, 2:20 a.m. UTC | #1 
On 02/28, Xiao Zhang wrote:
>Return ENOTSUP error code when configuring i40evf promiscuous mode to
>fix port start hang issue on platforms which are unsupported to configure
>promiscuous mode.
>
>Fixes: ddc7cb0d9453 ("net/i40e: re-program promiscuous mode on VF
>interface")

Please don't truncate the Fixes tag line, otherwise check-git-log.sh will complain.

>Cc: stable@dpdk.org
>
>Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
>---
> drivers/net/i40e/i40e_ethdev_vf.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
>index c34f520..244397e 100644
>--- a/drivers/net/i40e/i40e_ethdev_vf.c
>+++ b/drivers/net/i40e/i40e_ethdev_vf.c
>@@ -2191,6 +2191,8 @@ i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev)
> 	ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled);
> 	if (ret == 0)
> 		vf->promisc_unicast_enabled = TRUE;
>+	else if (ret == I40E_NOT_SUPPORTED)
>+		ret = -ENOTSUP;
> 	else
> 		ret = -EAGAIN;
> 
>@@ -2206,6 +2208,8 @@ i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev)
> 	ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled);
> 	if (ret == 0)
> 		vf->promisc_unicast_enabled = FALSE;
>+	else if (ret == I40E_NOT_SUPPORTED)
>+		ret = -ENOTSUP;
> 	else
> 		ret = -EAGAIN;
> 
>@@ -2221,6 +2225,8 @@ i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev)
> 	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1);
> 	if (ret == 0)
> 		vf->promisc_multicast_enabled = TRUE;
>+	else if (ret == I40E_NOT_SUPPORTED)
>+		ret = -ENOTSUP;
> 	else
> 		ret = -EAGAIN;
> 
>@@ -2236,6 +2242,8 @@ i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev)
> 	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0);
> 	if (ret == 0)
> 		vf->promisc_multicast_enabled = FALSE;
>+	else if (ret == I40E_NOT_SUPPORTED)
>+		ret = -ENOTSUP;
> 	else
> 		ret = -EAGAIN;
> 
>-- 
>2.7.4
>

For the rest,

Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>

Applied to dpdk-next-net-intel with the Fixes line fix, Thanks.
Ferruh Yigit March 2, 2020, 9:08 a.m. UTC | #2 
On 2/28/2020 3:22 AM, Xiao Zhang wrote:
> Return ENOTSUP error code when configuring i40evf promiscuous mode to
> fix port start hang issue on platforms which are unsupported to configure
> promiscuous mode.

Hi Xiao,

What is the cause of the hang, was the application keep trying because of the
"-EAGAIN" error?

> 
> Fixes: ddc7cb0d9453 ("net/i40e: re-program promiscuous mode on VF
> interface")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev_vf.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index c34f520..244397e 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -2191,6 +2191,8 @@ i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev)
>  	ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled);
>  	if (ret == 0)
>  		vf->promisc_unicast_enabled = TRUE;
> +	else if (ret == I40E_NOT_SUPPORTED)
> +		ret = -ENOTSUP;
>  	else
>  		ret = -EAGAIN;
>  
> @@ -2206,6 +2208,8 @@ i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev)
>  	ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled);
>  	if (ret == 0)
>  		vf->promisc_unicast_enabled = FALSE;
> +	else if (ret == I40E_NOT_SUPPORTED)
> +		ret = -ENOTSUP;
>  	else
>  		ret = -EAGAIN;
>  
> @@ -2221,6 +2225,8 @@ i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev)
>  	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1);
>  	if (ret == 0)
>  		vf->promisc_multicast_enabled = TRUE;
> +	else if (ret == I40E_NOT_SUPPORTED)
> +		ret = -ENOTSUP;
>  	else
>  		ret = -EAGAIN;
>  
> @@ -2236,6 +2242,8 @@ i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev)
>  	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0);
>  	if (ret == 0)
>  		vf->promisc_multicast_enabled = FALSE;
> +	else if (ret == I40E_NOT_SUPPORTED)
> +		ret = -ENOTSUP;
>  	else
>  		ret = -EAGAIN;
>  
>
Xiao Zhang March 3, 2020, 2:01 a.m. UTC | #3 
Hi Ferruh,

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, March 2, 2020 5:09 PM
> To: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] net/i40e: add promiscuous configure unsupported check
> 
> On 2/28/2020 3:22 AM, Xiao Zhang wrote:
> > Return ENOTSUP error code when configuring i40evf promiscuous mode to
> > fix port start hang issue on platforms which are unsupported to
> > configure promiscuous mode.
> 
> Hi Xiao,
> 
> What is the cause of the hang, was the application keep trying because of the "-
> EAGAIN" error?

When starting port, rte_eth_dev_start will call rte_eth_dev_config_restore in which promisc configure will be called to enable/disable promiscuous mode.
Since "-EAGAIN" was returned if platforms not supported to configure promisc mode, it would return error and stop port starting.

        /* replay promiscuous configuration */
        /*
         * use callbacks directly since we don't need port_id check and
         * would like to bypass the same value set
         */
        if (rte_eth_promiscuous_get(port_id) == 1 &&
            *dev->dev_ops->promiscuous_enable != NULL) {
                ret = eth_err(port_id,
                              (*dev->dev_ops->promiscuous_enable)(dev));
                if (ret != 0 && ret != -ENOTSUP) {
                        RTE_ETHDEV_LOG(ERR,
                                "Failed to enable promiscuous mode for device (port %u): %s\n",
                                port_id, rte_strerror(-ret));
                        return ret;
                }
        } else if (rte_eth_promiscuous_get(port_id) == 0 &&
                   *dev->dev_ops->promiscuous_disable != NULL) {
                ret = eth_err(port_id,
                              (*dev->dev_ops->promiscuous_disable)(dev));
                if (ret != 0 && ret != -ENOTSUP) {
                        RTE_ETHDEV_LOG(ERR,
                                "Failed to disable promiscuous mode for device (port %u): %s\n",
                                port_id, rte_strerror(-ret));
                        return ret;
                }
        }

> 
> >
> > Fixes: ddc7cb0d9453 ("net/i40e: re-program promiscuous mode on VF
> > interface")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev_vf.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index c34f520..244397e 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -2191,6 +2191,8 @@ i40evf_dev_promiscuous_enable(struct rte_eth_dev
> *dev)
> >  	ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled);
> >  	if (ret == 0)
> >  		vf->promisc_unicast_enabled = TRUE;
> > +	else if (ret == I40E_NOT_SUPPORTED)
> > +		ret = -ENOTSUP;
> >  	else
> >  		ret = -EAGAIN;
> >
> > @@ -2206,6 +2208,8 @@ i40evf_dev_promiscuous_disable(struct
> rte_eth_dev *dev)
> >  	ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled);
> >  	if (ret == 0)
> >  		vf->promisc_unicast_enabled = FALSE;
> > +	else if (ret == I40E_NOT_SUPPORTED)
> > +		ret = -ENOTSUP;
> >  	else
> >  		ret = -EAGAIN;
> >
> > @@ -2221,6 +2225,8 @@ i40evf_dev_allmulticast_enable(struct rte_eth_dev
> *dev)
> >  	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1);
> >  	if (ret == 0)
> >  		vf->promisc_multicast_enabled = TRUE;
> > +	else if (ret == I40E_NOT_SUPPORTED)
> > +		ret = -ENOTSUP;
> >  	else
> >  		ret = -EAGAIN;
> >
> > @@ -2236,6 +2242,8 @@ i40evf_dev_allmulticast_disable(struct rte_eth_dev
> *dev)
> >  	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0);
> >  	if (ret == 0)
> >  		vf->promisc_multicast_enabled = FALSE;
> > +	else if (ret == I40E_NOT_SUPPORTED)
> > +		ret = -ENOTSUP;
> >  	else
> >  		ret = -EAGAIN;
> >
> >
Xiaolong Ye March 3, 2020, 2:53 a.m. UTC | #4 
On 03/03, Zhang, Xiao wrote:
>Hi Ferruh,
>
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Monday, March 2, 2020 5:09 PM
>> To: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org
>> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>> stable@dpdk.org
>> Subject: Re: [dpdk-dev] net/i40e: add promiscuous configure unsupported check
>> 
>> On 2/28/2020 3:22 AM, Xiao Zhang wrote:
>> > Return ENOTSUP error code when configuring i40evf promiscuous mode to
>> > fix port start hang issue on platforms which are unsupported to
>> > configure promiscuous mode.
>> 
>> Hi Xiao,
>> 
>> What is the cause of the hang, was the application keep trying because of the "-
>> EAGAIN" error?
>
>When starting port, rte_eth_dev_start will call rte_eth_dev_config_restore in which promisc configure will be called to enable/disable promiscuous mode.
>Since "-EAGAIN" was returned if platforms not supported to configure promisc mode, it would return error and stop port starting.

So the real issue caused by this is "Fail to start port", not "hang", right?
I can change the description in the commit log directly.

Thanks,
Xiaolong
Xiao Zhang March 3, 2020, 2:59 a.m. UTC | #5 
> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Tuesday, March 3, 2020 10:53 AM
> To: Zhang, Xiao <xiao.zhang@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] net/i40e: add promiscuous configure unsupported check
> 
> On 03/03, Zhang, Xiao wrote:
> >Hi Ferruh,
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh
> >> Sent: Monday, March 2, 2020 5:09 PM
> >> To: Zhang, Xiao <xiao.zhang@intel.com>; dev@dpdk.org
> >> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>; stable@dpdk.org
> >> Subject: Re: [dpdk-dev] net/i40e: add promiscuous configure
> >> unsupported check
> >>
> >> On 2/28/2020 3:22 AM, Xiao Zhang wrote:
> >> > Return ENOTSUP error code when configuring i40evf promiscuous mode
> >> > to fix port start hang issue on platforms which are unsupported to
> >> > configure promiscuous mode.
> >>
> >> Hi Xiao,
> >>
> >> What is the cause of the hang, was the application keep trying
> >> because of the "- EAGAIN" error?
> >
> >When starting port, rte_eth_dev_start will call rte_eth_dev_config_restore in
> which promisc configure will be called to enable/disable promiscuous mode.
> >Since "-EAGAIN" was returned if platforms not supported to configure promisc
> mode, it would return error and stop port starting.
> 
> So the real issue caused by this is "Fail to start port", not "hang", right?
> I can change the description in the commit log directly.

Yes, it's "Fail to start port". Thanks Xiaolong helping correct the commit log.

> 
> Thanks,
> Xiaolong
diff mbox series

Patch

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index c34f520..244397e 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2191,6 +2191,8 @@  i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev)
 	ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled);
 	if (ret == 0)
 		vf->promisc_unicast_enabled = TRUE;
+	else if (ret == I40E_NOT_SUPPORTED)
+		ret = -ENOTSUP;
 	else
 		ret = -EAGAIN;
 
@@ -2206,6 +2208,8 @@  i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev)
 	ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled);
 	if (ret == 0)
 		vf->promisc_unicast_enabled = FALSE;
+	else if (ret == I40E_NOT_SUPPORTED)
+		ret = -ENOTSUP;
 	else
 		ret = -EAGAIN;
 
@@ -2221,6 +2225,8 @@  i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev)
 	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1);
 	if (ret == 0)
 		vf->promisc_multicast_enabled = TRUE;
+	else if (ret == I40E_NOT_SUPPORTED)
+		ret = -ENOTSUP;
 	else
 		ret = -EAGAIN;
 
@@ -2236,6 +2242,8 @@  i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev)
 	ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0);
 	if (ret == 0)
 		vf->promisc_multicast_enabled = FALSE;
+	else if (ret == I40E_NOT_SUPPORTED)
+		ret = -ENOTSUP;
 	else
 		ret = -EAGAIN;