[dpdk-stable] [PATCH v1] net/failsafe: fix VLAN stripping configuration

[dpdk-stable] [PATCH v1] net/failsafe: fix VLAN stripping configuration

[Ophir Munk]

failsafe device has vlan stripping configured at startup however once
a sub device is found as non-capable of vlan-stripping failsafe
updates it configuration and removes vlan stripping from it.
This update occurs only once at startup. Following a later plugin
attempt and in case of vlan stripping mismatch between failsafe
configuration and device capability - failsafe cannot recover and the
device remains constantly in plug out state.

The sequence of events leading to this situation is described as
follows:
1. Start testpmd with failsafe where mlx4 is a sub device (not capable
of vlan stripping). Expected printout:
PMD: net_failsafe: Disabling VLAN stripping offload
2. Execute:
testpmd> port stop all
testpmd> port config all max-pkt-len 2048
testpmd> port start all
3. Do a plug out (e.g. disable sriov)
4. Do a plug in (e.g. enable sriov)
5. Expected result: failsafe successfully configures and starts its sub
devices
Actual result: failsafe is continuously failing with these messages:
PMD: net_failsafe: VLAN stripping offload requested but not supported by
sub_device 0
PMD: net_failsafe: device already configured, cannot fix live
configuration
PMD: net_failsafe: Unable to synchronize sub device state

Root cause analysis: at startup failsafe removes vlan stripping from its
configuration. After executing "port config all max-pkt-len 2048"
testpmd marks failsafe in need for configuration update.
After executing "port start all" testpmd overrides failsafe
configuration with its own configuration which includes vlan stripping

During the plugin attempt failsafe refuses to update its configuration
by removing vlan stripping since it has already updated its
configuration at startup.

The fix is to remove the limitation of one time configuration at
startup and allow it during plugin attempts.

Cc: stable@dpdk.org
Fixes: bbc6a53dda44 ("net/failsafe: support Rx offload capabilities")

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
The commit message includes bug and fix descriptions
---
 drivers/net/failsafe/failsafe_ops.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index f460551..953ee65 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -187,16 +187,6 @@
 			continue;
 		DEBUG("Checking capabilities for sub_device %d", i);
 		while ((capa_flag = fs_port_offload_validate(dev, sdev))) {
-			/*
-			 * Refuse to change configuration if multiple devices
-			 * are present and we already have configured at least
-			 * some of them.
-			 */
-			if (PRIV(dev)->state >= DEV_ACTIVE &&
-			    PRIV(dev)->subs_tail > 1) {
-				ERROR("device already configured, cannot fix live configuration");
-				return -1;
-			}
 			ret = fs_port_disable_offload(&dev->data->dev_conf,
 						      capa_flag);
 			if (ret) {
-- 
1.8.3.1

Re: [dpdk-stable] [PATCH v1] net/failsafe: fix VLAN stripping configuration

[Gaëtan Rivet]

On Wed, Nov 01, 2017 at 08:12:38PM +0000, Ophir Munk wrote:
> failsafe device has vlan stripping configured at startup however once
> a sub device is found as non-capable of vlan-stripping failsafe
> updates it configuration and removes vlan stripping from it.
> This update occurs only once at startup. Following a later plugin
> attempt and in case of vlan stripping mismatch between failsafe
> configuration and device capability - failsafe cannot recover and the
> device remains constantly in plug out state.
> 
> The sequence of events leading to this situation is described as
> follows:
> 1. Start testpmd with failsafe where mlx4 is a sub device (not capable
> of vlan stripping). Expected printout:
> PMD: net_failsafe: Disabling VLAN stripping offload
> 2. Execute:
> testpmd> port stop all
> testpmd> port config all max-pkt-len 2048
> testpmd> port start all
> 3. Do a plug out (e.g. disable sriov)
> 4. Do a plug in (e.g. enable sriov)
> 5. Expected result: failsafe successfully configures and starts its sub
> devices
> Actual result: failsafe is continuously failing with these messages:
> PMD: net_failsafe: VLAN stripping offload requested but not supported by
> sub_device 0
> PMD: net_failsafe: device already configured, cannot fix live
> configuration
> PMD: net_failsafe: Unable to synchronize sub device state
> 
> Root cause analysis: at startup failsafe removes vlan stripping from its
> configuration. After executing "port config all max-pkt-len 2048"
> testpmd marks failsafe in need for configuration update.
> After executing "port start all" testpmd overrides failsafe
> configuration with its own configuration which includes vlan stripping
> 

Have you tried launching testpmd with the option

"--disable-hw-vlan"

as your mlx4 port does not support it?

> During the plugin attempt failsafe refuses to update its configuration
> by removing vlan stripping since it has already updated its
> configuration at startup.
> 
> The fix is to remove the limitation of one time configuration at
> startup and allow it during plugin attempts.
> 
> Cc: stable@dpdk.org
> Fixes: bbc6a53dda44 ("net/failsafe: support Rx offload capabilities")
> 
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> ---
> The commit message includes bug and fix descriptions
> ---
>  drivers/net/failsafe/failsafe_ops.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
> index f460551..953ee65 100644
> --- a/drivers/net/failsafe/failsafe_ops.c
> +++ b/drivers/net/failsafe/failsafe_ops.c
> @@ -187,16 +187,6 @@
>  			continue;
>  		DEBUG("Checking capabilities for sub_device %d", i);
>  		while ((capa_flag = fs_port_offload_validate(dev, sdev))) {
> -			/*
> -			 * Refuse to change configuration if multiple devices
> -			 * are present and we already have configured at least
> -			 * some of them.
> -			 */
> -			if (PRIV(dev)->state >= DEV_ACTIVE &&
> -			    PRIV(dev)->subs_tail > 1) {
> -				ERROR("device already configured, cannot fix live configuration");
> -				return -1;
> -			}
>  			ret = fs_port_disable_offload(&dev->data->dev_conf,
>  						      capa_flag);
>  			if (ret) {
> -- 
> 1.8.3.1
> 

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-stable] [PATCH v1] net/failsafe: fix VLAN stripping configuration

[Gaëtan Rivet]

On Thu, Nov 02, 2017 at 02:52:16PM +0100, Gaëtan Rivet wrote:
> On Wed, Nov 01, 2017 at 08:12:38PM +0000, Ophir Munk wrote:
> > failsafe device has vlan stripping configured at startup however once
> > a sub device is found as non-capable of vlan-stripping failsafe
> > updates it configuration and removes vlan stripping from it.
> > This update occurs only once at startup. Following a later plugin
> > attempt and in case of vlan stripping mismatch between failsafe
> > configuration and device capability - failsafe cannot recover and the
> > device remains constantly in plug out state.
> > 
> > The sequence of events leading to this situation is described as
> > follows:
> > 1. Start testpmd with failsafe where mlx4 is a sub device (not capable
> > of vlan stripping). Expected printout:
> > PMD: net_failsafe: Disabling VLAN stripping offload
> > 2. Execute:
> > testpmd> port stop all
> > testpmd> port config all max-pkt-len 2048
> > testpmd> port start all
> > 3. Do a plug out (e.g. disable sriov)
> > 4. Do a plug in (e.g. enable sriov)
> > 5. Expected result: failsafe successfully configures and starts its sub
> > devices
> > Actual result: failsafe is continuously failing with these messages:
> > PMD: net_failsafe: VLAN stripping offload requested but not supported by
> > sub_device 0
> > PMD: net_failsafe: device already configured, cannot fix live
> > configuration
> > PMD: net_failsafe: Unable to synchronize sub device state
> > 
> > Root cause analysis: at startup failsafe removes vlan stripping from its
> > configuration. After executing "port config all max-pkt-len 2048"
> > testpmd marks failsafe in need for configuration update.
> > After executing "port start all" testpmd overrides failsafe
> > configuration with its own configuration which includes vlan stripping
> > 
> 
> Have you tried launching testpmd with the option
> 
> "--disable-hw-vlan"
> 
> as your mlx4 port does not support it?
> 

On a second thought, I think there is a simple solution:

The fail-safe should stop trying to be clever with port configuration.
On rte_eth_dev_configure, simply apply the user configuration (without
trying to detect support and disabling flags on the fly).

If a PMD has an issue, it should warn the user. If it has an issue but
does not warn, it is a bug for this PMD. This is the case for MLX4:
either the PMD changes its behavior, or not, as long as users are fine
with it.

So a proper fix would be to remove the checks (fs_port_offload_validate
and fs_port_disable_offload) and depend on the sub-device for proper
configuration vetting.

Thoughts?

> > During the plugin attempt failsafe refuses to update its configuration
> > by removing vlan stripping since it has already updated its
> > configuration at startup.
> > 
> > The fix is to remove the limitation of one time configuration at
> > startup and allow it during plugin attempts.
> > 
> > Cc: stable@dpdk.org
> > Fixes: bbc6a53dda44 ("net/failsafe: support Rx offload capabilities")
> > 
> > Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> > ---
> > The commit message includes bug and fix descriptions
> > ---
> >  drivers/net/failsafe/failsafe_ops.c | 10 ----------
> >  1 file changed, 10 deletions(-)
> > 
> > diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
> > index f460551..953ee65 100644
> > --- a/drivers/net/failsafe/failsafe_ops.c
> > +++ b/drivers/net/failsafe/failsafe_ops.c
> > @@ -187,16 +187,6 @@
> >  			continue;
> >  		DEBUG("Checking capabilities for sub_device %d", i);
> >  		while ((capa_flag = fs_port_offload_validate(dev, sdev))) {
> > -			/*
> > -			 * Refuse to change configuration if multiple devices
> > -			 * are present and we already have configured at least
> > -			 * some of them.
> > -			 */
> > -			if (PRIV(dev)->state >= DEV_ACTIVE &&
> > -			    PRIV(dev)->subs_tail > 1) {
> > -				ERROR("device already configured, cannot fix live configuration");
> > -				return -1;
> > -			}
> >  			ret = fs_port_disable_offload(&dev->data->dev_conf,
> >  						      capa_flag);
> >  			if (ret) {
> > -- 
> > 1.8.3.1
> > 
> 
> -- 
> Gaëtan Rivet
> 6WIND

-- 
Gaëtan Rivet
6WIND

[dpdk-stable] [PATCH v2] net/failsafe: fix VLAN stripping configuration

[Ophir Munk]

failsafe device has vlan stripping configured at startup however once
a sub device is found as non-capable of vlan-stripping failsafe
updates it configuration and removes vlan stripping from it.
This update occurs only once at startup. Following a later plugin
attempt and in case of vlan stripping mismatch between failsafe
configuration and device capability - failsafe cannot recover and the
device remains constantly in plug out state.

The sequence of events leading to this situation is described as
follows:
1. Start testpmd with failsafe where mlx4 is a sub device (not capable
of vlan stripping). Expected printout:
PMD: net_failsafe: Disabling VLAN stripping offload
2. Execute:
testpmd> port stop all
testpmd> port config all max-pkt-len 2048
testpmd> port start all
3. Do a plug out (e.g. disable sriov)
4. Do a plug in (e.g. enable sriov)
5. Expected result: failsafe successfully configures and starts its sub
devices
Actual result: failsafe is continuously failing with these messages:
PMD: net_failsafe: VLAN stripping offload requested but not supported by
sub_device 0
PMD: net_failsafe: device already configured, cannot fix live
configuration
PMD: net_failsafe: Unable to synchronize sub device state

Root cause analysis: at startup failsafe removes vlan stripping from its
configuration. After executing "port config all max-pkt-len 2048"
testpmd marks failsafe in need for configuration update.
After executing "port start all" testpmd overrides failsafe
configuration with its own configuration which includes vlan stripping

During the plugin attempt failsafe refuses to update its configuration
by removing vlan stripping since it has already updated its
configuration at startup.

The fix is for failsafe to stop validation and disabling non-supported
offloads in its sub-devices.

Cc: stable@dpdk.org
Fixes: bbc6a53dda44 ("net/failsafe: support Rx offload capabilities")

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
 drivers/net/failsafe/failsafe_ops.c | 118 ------------------------------------
 1 file changed, 118 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index f460551..e16a590 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -80,132 +80,14 @@
 	.flow_type_rss_offloads = 0x0,
 };
 
-/**
- * Check whether a specific offloading capability
- * is supported by a sub_device.
- *
- * @return
- *   0: all requested capabilities are supported by the sub_device
- *   positive value: This flag at least is not supported by the sub_device
- */
-static int
-fs_port_offload_validate(struct rte_eth_dev *dev,
-			 struct sub_device *sdev)
-{
-	struct rte_eth_dev_info infos = {0};
-	struct rte_eth_conf *cf;
-	uint32_t cap;
-
-	cf = &dev->data->dev_conf;
-	SUBOPS(sdev, dev_infos_get)(ETH(sdev), &infos);
-	/* RX capabilities */
-	cap = infos.rx_offload_capa;
-	if (cf->rxmode.hw_vlan_strip &&
-	    ((cap & DEV_RX_OFFLOAD_VLAN_STRIP) == 0)) {
-		WARN("VLAN stripping offload requested but not supported by sub_device %d",
-		      SUB_ID(sdev));
-		return DEV_RX_OFFLOAD_VLAN_STRIP;
-	}
-	if (cf->rxmode.hw_ip_checksum &&
-	    ((cap & (DEV_RX_OFFLOAD_IPV4_CKSUM |
-		     DEV_RX_OFFLOAD_UDP_CKSUM |
-		     DEV_RX_OFFLOAD_TCP_CKSUM)) !=
-	     (DEV_RX_OFFLOAD_IPV4_CKSUM |
-	      DEV_RX_OFFLOAD_UDP_CKSUM |
-	      DEV_RX_OFFLOAD_TCP_CKSUM))) {
-		WARN("IP checksum offload requested but not supported by sub_device %d",
-		      SUB_ID(sdev));
-		return DEV_RX_OFFLOAD_IPV4_CKSUM |
-		       DEV_RX_OFFLOAD_UDP_CKSUM |
-		       DEV_RX_OFFLOAD_TCP_CKSUM;
-	}
-	if (cf->rxmode.enable_lro &&
-	    ((cap & DEV_RX_OFFLOAD_TCP_LRO) == 0)) {
-		WARN("TCP LRO offload requested but not supported by sub_device %d",
-		      SUB_ID(sdev));
-		return DEV_RX_OFFLOAD_TCP_LRO;
-	}
-	if (cf->rxmode.hw_vlan_extend &&
-	    ((cap & DEV_RX_OFFLOAD_QINQ_STRIP) == 0)) {
-		WARN("Stacked VLAN stripping offload requested but not supported by sub_device %d",
-		      SUB_ID(sdev));
-		return DEV_RX_OFFLOAD_QINQ_STRIP;
-	}
-	/* TX capabilities */
-	/* Nothing to do, no tx capa supported */
-	return 0;
-}
-
-/*
- * Disable the dev_conf flag related to an offload capability flag
- * within an ethdev configuration.
- */
-static int
-fs_port_disable_offload(struct rte_eth_conf *cf,
-			uint32_t ol_cap)
-{
-	switch (ol_cap) {
-	case DEV_RX_OFFLOAD_VLAN_STRIP:
-		INFO("Disabling VLAN stripping offload");
-		cf->rxmode.hw_vlan_strip = 0;
-		break;
-	case DEV_RX_OFFLOAD_IPV4_CKSUM:
-	case DEV_RX_OFFLOAD_UDP_CKSUM:
-	case DEV_RX_OFFLOAD_TCP_CKSUM:
-	case (DEV_RX_OFFLOAD_IPV4_CKSUM |
-	      DEV_RX_OFFLOAD_UDP_CKSUM |
-	      DEV_RX_OFFLOAD_TCP_CKSUM):
-		INFO("Disabling IP checksum offload");
-		cf->rxmode.hw_ip_checksum = 0;
-		break;
-	case DEV_RX_OFFLOAD_TCP_LRO:
-		INFO("Disabling TCP LRO offload");
-		cf->rxmode.enable_lro = 0;
-		break;
-	case DEV_RX_OFFLOAD_QINQ_STRIP:
-		INFO("Disabling stacked VLAN stripping offload");
-		cf->rxmode.hw_vlan_extend = 0;
-		break;
-	default:
-		DEBUG("Unable to disable offload capability: %" PRIx32,
-		      ol_cap);
-		return -1;
-	}
-	return 0;
-}
-
 static int
 fs_dev_configure(struct rte_eth_dev *dev)
 {
 	struct sub_device *sdev;
 	uint8_t i;
-	int capa_flag;
 	int ret;
 
 	FOREACH_SUBDEV(sdev, i, dev) {
-		if (sdev->state != DEV_PROBED)
-			continue;
-		DEBUG("Checking capabilities for sub_device %d", i);
-		while ((capa_flag = fs_port_offload_validate(dev, sdev))) {
-			/*
-			 * Refuse to change configuration if multiple devices
-			 * are present and we already have configured at least
-			 * some of them.
-			 */
-			if (PRIV(dev)->state >= DEV_ACTIVE &&
-			    PRIV(dev)->subs_tail > 1) {
-				ERROR("device already configured, cannot fix live configuration");
-				return -1;
-			}
-			ret = fs_port_disable_offload(&dev->data->dev_conf,
-						      capa_flag);
-			if (ret) {
-				ERROR("Unable to disable offload capability");
-				return ret;
-			}
-		}
-	}
-	FOREACH_SUBDEV(sdev, i, dev) {
 		int rmv_interrupt = 0;
 		int lsc_interrupt = 0;
 		int lsc_enabled;
-- 
1.8.3.1

[dpdk-stable] [PATCH v2] net/failsafe: fix VLAN stripping configuration

[Ophir Munk]

failsafe device has vlan stripping configured at startup however once
a sub device is found as non-capable of vlan-stripping failsafe
updates it configuration and removes vlan stripping from it.
This update occurs only once at startup. Following a later plugin
attempt and in case of vlan stripping mismatch between failsafe
configuration and device capability - failsafe cannot recover and the
device remains constantly in plug out state.

The sequence of events leading to this situation is described as
follows:
1. Start testpmd with failsafe where mlx4 is a sub device (not capable
of vlan stripping). Expected printout:
PMD: net_failsafe: Disabling VLAN stripping offload
2. Execute:
testpmd> port stop all
testpmd> port config all max-pkt-len 2048
testpmd> port start all
3. Do a plug out (e.g. disable sriov)
4. Do a plug in (e.g. enable sriov)
5. Expected result: failsafe successfully configures and starts its sub
devices
Actual result: failsafe is continuously failing with these messages:
PMD: net_failsafe: VLAN stripping offload requested but not supported by
sub_device 0
PMD: net_failsafe: device already configured, cannot fix live
configuration
PMD: net_failsafe: Unable to synchronize sub device state

Root cause analysis: at startup failsafe removes vlan stripping from its
configuration. After executing "port config all max-pkt-len 2048"
testpmd marks failsafe in need for configuration update.
After executing "port start all" testpmd overrides failsafe
configuration with its own configuration which includes vlan stripping

During the plugin attempt failsafe refuses to update its configuration
by removing vlan stripping since it has already updated its
configuration at startup.

The fix is for failsafe to stop validation and disabling non-supported
offloads in its sub-devices.

Cc: stable@dpdk.org
Fixes: bbc6a53dda44 ("net/failsafe: support Rx offload capabilities")

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
 drivers/net/failsafe/failsafe_ops.c | 118 ------------------------------------
 1 file changed, 118 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index f460551..e16a590 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -80,132 +80,14 @@
 	.flow_type_rss_offloads = 0x0,
 };
 
-/**
- * Check whether a specific offloading capability
- * is supported by a sub_device.
- *
- * @return
- *   0: all requested capabilities are supported by the sub_device
- *   positive value: This flag at least is not supported by the sub_device
- */
-static int
-fs_port_offload_validate(struct rte_eth_dev *dev,
-			 struct sub_device *sdev)
-{
-	struct rte_eth_dev_info infos = {0};
-	struct rte_eth_conf *cf;
-	uint32_t cap;
-
-	cf = &dev->data->dev_conf;
-	SUBOPS(sdev, dev_infos_get)(ETH(sdev), &infos);
-	/* RX capabilities */
-	cap = infos.rx_offload_capa;
-	if (cf->rxmode.hw_vlan_strip &&
-	    ((cap & DEV_RX_OFFLOAD_VLAN_STRIP) == 0)) {
-		WARN("VLAN stripping offload requested but not supported by sub_device %d",
-		      SUB_ID(sdev));
-		return DEV_RX_OFFLOAD_VLAN_STRIP;
-	}
-	if (cf->rxmode.hw_ip_checksum &&
-	    ((cap & (DEV_RX_OFFLOAD_IPV4_CKSUM |
-		     DEV_RX_OFFLOAD_UDP_CKSUM |
-		     DEV_RX_OFFLOAD_TCP_CKSUM)) !=
-	     (DEV_RX_OFFLOAD_IPV4_CKSUM |
-	      DEV_RX_OFFLOAD_UDP_CKSUM |
-	      DEV_RX_OFFLOAD_TCP_CKSUM))) {
-		WARN("IP checksum offload requested but not supported by sub_device %d",
-		      SUB_ID(sdev));
-		return DEV_RX_OFFLOAD_IPV4_CKSUM |
-		       DEV_RX_OFFLOAD_UDP_CKSUM |
-		       DEV_RX_OFFLOAD_TCP_CKSUM;
-	}
-	if (cf->rxmode.enable_lro &&
-	    ((cap & DEV_RX_OFFLOAD_TCP_LRO) == 0)) {
-		WARN("TCP LRO offload requested but not supported by sub_device %d",
-		      SUB_ID(sdev));
-		return DEV_RX_OFFLOAD_TCP_LRO;
-	}
-	if (cf->rxmode.hw_vlan_extend &&
-	    ((cap & DEV_RX_OFFLOAD_QINQ_STRIP) == 0)) {
-		WARN("Stacked VLAN stripping offload requested but not supported by sub_device %d",
-		      SUB_ID(sdev));
-		return DEV_RX_OFFLOAD_QINQ_STRIP;
-	}
-	/* TX capabilities */
-	/* Nothing to do, no tx capa supported */
-	return 0;
-}
-
-/*
- * Disable the dev_conf flag related to an offload capability flag
- * within an ethdev configuration.
- */
-static int
-fs_port_disable_offload(struct rte_eth_conf *cf,
-			uint32_t ol_cap)
-{
-	switch (ol_cap) {
-	case DEV_RX_OFFLOAD_VLAN_STRIP:
-		INFO("Disabling VLAN stripping offload");
-		cf->rxmode.hw_vlan_strip = 0;
-		break;
-	case DEV_RX_OFFLOAD_IPV4_CKSUM:
-	case DEV_RX_OFFLOAD_UDP_CKSUM:
-	case DEV_RX_OFFLOAD_TCP_CKSUM:
-	case (DEV_RX_OFFLOAD_IPV4_CKSUM |
-	      DEV_RX_OFFLOAD_UDP_CKSUM |
-	      DEV_RX_OFFLOAD_TCP_CKSUM):
-		INFO("Disabling IP checksum offload");
-		cf->rxmode.hw_ip_checksum = 0;
-		break;
-	case DEV_RX_OFFLOAD_TCP_LRO:
-		INFO("Disabling TCP LRO offload");
-		cf->rxmode.enable_lro = 0;
-		break;
-	case DEV_RX_OFFLOAD_QINQ_STRIP:
-		INFO("Disabling stacked VLAN stripping offload");
-		cf->rxmode.hw_vlan_extend = 0;
-		break;
-	default:
-		DEBUG("Unable to disable offload capability: %" PRIx32,
-		      ol_cap);
-		return -1;
-	}
-	return 0;
-}
-
 static int
 fs_dev_configure(struct rte_eth_dev *dev)
 {
 	struct sub_device *sdev;
 	uint8_t i;
-	int capa_flag;
 	int ret;
 
 	FOREACH_SUBDEV(sdev, i, dev) {
-		if (sdev->state != DEV_PROBED)
-			continue;
-		DEBUG("Checking capabilities for sub_device %d", i);
-		while ((capa_flag = fs_port_offload_validate(dev, sdev))) {
-			/*
-			 * Refuse to change configuration if multiple devices
-			 * are present and we already have configured at least
-			 * some of them.
-			 */
-			if (PRIV(dev)->state >= DEV_ACTIVE &&
-			    PRIV(dev)->subs_tail > 1) {
-				ERROR("device already configured, cannot fix live configuration");
-				return -1;
-			}
-			ret = fs_port_disable_offload(&dev->data->dev_conf,
-						      capa_flag);
-			if (ret) {
-				ERROR("Unable to disable offload capability");
-				return ret;
-			}
-		}
-	}
-	FOREACH_SUBDEV(sdev, i, dev) {
 		int rmv_interrupt = 0;
 		int lsc_interrupt = 0;
 		int lsc_enabled;
-- 
1.8.3.1

Re: [dpdk-stable] [PATCH v1] net/failsafe: fix VLAN stripping configuration

[Ophir Munk]

Hi,
Please see below

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Thursday, November 02, 2017 4:16 PM
> To: Ophir Munk <ophirmu@mellanox.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Olga Shern
> <olgas@mellanox.com>; stable@dpdk.org
> Subject: Re: [PATCH v1] net/failsafe: fix VLAN stripping configuration
> 
> On Thu, Nov 02, 2017 at 02:52:16PM +0100, Gaëtan Rivet wrote:
> > On Wed, Nov 01, 2017 at 08:12:38PM +0000, Ophir Munk wrote:
> > > failsafe device has vlan stripping configured at startup however
> > > once a sub device is found as non-capable of vlan-stripping failsafe
> > > updates it configuration and removes vlan stripping from it.
> > > This update occurs only once at startup. Following a later plugin
> > > attempt and in case of vlan stripping mismatch between failsafe
> > > configuration and device capability - failsafe cannot recover and
> > > the device remains constantly in plug out state.
> > >
> > > The sequence of events leading to this situation is described as
> > > follows:
> > > 1. Start testpmd with failsafe where mlx4 is a sub device (not
> > > capable of vlan stripping). Expected printout:
> > > PMD: net_failsafe: Disabling VLAN stripping offload 2. Execute:
> > > testpmd> port stop all
> > > testpmd> port config all max-pkt-len 2048 port start all
> > > 3. Do a plug out (e.g. disable sriov) 4. Do a plug in (e.g. enable
> > > sriov) 5. Expected result: failsafe successfully configures and
> > > starts its sub devices Actual result: failsafe is continuously
> > > failing with these messages:
> > > PMD: net_failsafe: VLAN stripping offload requested but not
> > > supported by sub_device 0
> > > PMD: net_failsafe: device already configured, cannot fix live
> > > configuration
> > > PMD: net_failsafe: Unable to synchronize sub device state
> > >
> > > Root cause analysis: at startup failsafe removes vlan stripping from
> > > its configuration. After executing "port config all max-pkt-len 2048"
> > > testpmd marks failsafe in need for configuration update.
> > > After executing "port start all" testpmd overrides failsafe
> > > configuration with its own configuration which includes vlan
> > > stripping
> > >
> >
> > Have you tried launching testpmd with the option
> >
> > "--disable-hw-vlan"
> >
> > as your mlx4 port does not support it?
> >
> 
> On a second thought, I think there is a simple solution:
> 
> The fail-safe should stop trying to be clever with port configuration.
> On rte_eth_dev_configure, simply apply the user configuration (without
> trying to detect support and disabling flags on the fly).
> 
> If a PMD has an issue, it should warn the user. If it has an issue but does not
> warn, it is a bug for this PMD. This is the case for MLX4:
> either the PMD changes its behavior, or not, as long as users are fine with it.
> 
> So a proper fix would be to remove the checks (fs_port_offload_validate and
> fs_port_disable_offload) and depend on the sub-device for proper
> configuration vetting.
> 
> Thoughts?

Agreed. I have sent v2 based on your suggestion. 

> 
> > > During the plugin attempt failsafe refuses to update its
> > > configuration by removing vlan stripping since it has already
> > > updated its configuration at startup.
> > >
> > > The fix is to remove the limitation of one time configuration at
> > > startup and allow it during plugin attempts.
> > >
> > > Cc: stable@dpdk.org
> > > Fixes: bbc6a53dda44 ("net/failsafe: support Rx offload
> > > capabilities")
> > >
> > > Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> > > ---
> > > The commit message includes bug and fix descriptions
> > > ---
> > >  drivers/net/failsafe/failsafe_ops.c | 10 ----------
> > >  1 file changed, 10 deletions(-)
> > >
> > > diff --git a/drivers/net/failsafe/failsafe_ops.c
> > > b/drivers/net/failsafe/failsafe_ops.c
> > > index f460551..953ee65 100644
> > > --- a/drivers/net/failsafe/failsafe_ops.c
> > > +++ b/drivers/net/failsafe/failsafe_ops.c
> > > @@ -187,16 +187,6 @@
> > >  			continue;
> > >  		DEBUG("Checking capabilities for sub_device %d", i);
> > >  		while ((capa_flag = fs_port_offload_validate(dev, sdev))) {
> > > -			/*
> > > -			 * Refuse to change configuration if multiple devices
> > > -			 * are present and we already have configured at
> least
> > > -			 * some of them.
> > > -			 */
> > > -			if (PRIV(dev)->state >= DEV_ACTIVE &&
> > > -			    PRIV(dev)->subs_tail > 1) {
> > > -				ERROR("device already configured, cannot
> fix live configuration");
> > > -				return -1;
> > > -			}
> > >  			ret = fs_port_disable_offload(&dev->data-
> >dev_conf,
> > >  						      capa_flag);
> > >  			if (ret) {
> > > --
> > > 1.8.3.1
> > >
> >
> > --
> > Gaëtan Rivet
> > 6WIND
> 
> --
> Gaëtan Rivet
> 6WIND

Re: [dpdk-stable] [dpdk-dev] [PATCH v2] net/failsafe: fix VLAN stripping configuration

[Thomas Monjalon]

02/11/2017 18:27, Ophir Munk:
> failsafe device has vlan stripping configured at startup however once
> a sub device is found as non-capable of vlan-stripping failsafe
> updates it configuration and removes vlan stripping from it.
> This update occurs only once at startup. Following a later plugin
> attempt and in case of vlan stripping mismatch between failsafe
> configuration and device capability - failsafe cannot recover and the
> device remains constantly in plug out state.
> 
> The sequence of events leading to this situation is described as
> follows:
> 1. Start testpmd with failsafe where mlx4 is a sub device (not capable
> of vlan stripping). Expected printout:
> PMD: net_failsafe: Disabling VLAN stripping offload
> 2. Execute:
> testpmd> port stop all
> testpmd> port config all max-pkt-len 2048
> testpmd> port start all
> 3. Do a plug out (e.g. disable sriov)
> 4. Do a plug in (e.g. enable sriov)
> 5. Expected result: failsafe successfully configures and starts its sub
> devices
> Actual result: failsafe is continuously failing with these messages:
> PMD: net_failsafe: VLAN stripping offload requested but not supported by
> sub_device 0
> PMD: net_failsafe: device already configured, cannot fix live
> configuration
> PMD: net_failsafe: Unable to synchronize sub device state
> 
> Root cause analysis: at startup failsafe removes vlan stripping from its
> configuration. After executing "port config all max-pkt-len 2048"
> testpmd marks failsafe in need for configuration update.
> After executing "port start all" testpmd overrides failsafe
> configuration with its own configuration which includes vlan stripping
> 
> During the plugin attempt failsafe refuses to update its configuration
> by removing vlan stripping since it has already updated its
> configuration at startup.
> 
> The fix is for failsafe to stop validation and disabling non-supported
> offloads in its sub-devices.
> 
> Cc: stable@dpdk.org
> Fixes: bbc6a53dda44 ("net/failsafe: support Rx offload capabilities")

The Cc:stable line is usually below the Fixes: line.

> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>

The fix in this v2 matches what Gaetan requested after v1.

I agree it is better that failsafe does not interfere in configurations.
The responsibility of failsafe is to build the capability set.
The responsibility of the application is to check the capabilities
before configuring the failsafe device (like any other device).

Acked-by: Thomas Monjalon <thomas@monjalon.net>

Re: [dpdk-stable] [dpdk-dev] [PATCH v2] net/failsafe: fix VLAN stripping configuration

[Ferruh Yigit]

On 11/3/2017 4:11 PM, Thomas Monjalon wrote:
> 02/11/2017 18:27, Ophir Munk:
>> failsafe device has vlan stripping configured at startup however once
>> a sub device is found as non-capable of vlan-stripping failsafe
>> updates it configuration and removes vlan stripping from it.
>> This update occurs only once at startup. Following a later plugin
>> attempt and in case of vlan stripping mismatch between failsafe
>> configuration and device capability - failsafe cannot recover and the
>> device remains constantly in plug out state.
>>
>> The sequence of events leading to this situation is described as
>> follows:
>> 1. Start testpmd with failsafe where mlx4 is a sub device (not capable
>> of vlan stripping). Expected printout:
>> PMD: net_failsafe: Disabling VLAN stripping offload
>> 2. Execute:
>> testpmd> port stop all
>> testpmd> port config all max-pkt-len 2048
>> testpmd> port start all
>> 3. Do a plug out (e.g. disable sriov)
>> 4. Do a plug in (e.g. enable sriov)
>> 5. Expected result: failsafe successfully configures and starts its sub
>> devices
>> Actual result: failsafe is continuously failing with these messages:
>> PMD: net_failsafe: VLAN stripping offload requested but not supported by
>> sub_device 0
>> PMD: net_failsafe: device already configured, cannot fix live
>> configuration
>> PMD: net_failsafe: Unable to synchronize sub device state
>>
>> Root cause analysis: at startup failsafe removes vlan stripping from its
>> configuration. After executing "port config all max-pkt-len 2048"
>> testpmd marks failsafe in need for configuration update.
>> After executing "port start all" testpmd overrides failsafe
>> configuration with its own configuration which includes vlan stripping
>>
>> During the plugin attempt failsafe refuses to update its configuration
>> by removing vlan stripping since it has already updated its
>> configuration at startup.
>>
>> The fix is for failsafe to stop validation and disabling non-supported
>> offloads in its sub-devices.
>>
>> Cc: stable@dpdk.org
>> Fixes: bbc6a53dda44 ("net/failsafe: support Rx offload capabilities")
> 
> The Cc:stable line is usually below the Fixes: line.
> 
>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> 
> The fix in this v2 matches what Gaetan requested after v1.
> 
> I agree it is better that failsafe does not interfere in configurations.
> The responsibility of failsafe is to build the capability set.
> The responsibility of the application is to check the capabilities
> before configuring the failsafe device (like any other device).
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

Applied to dpdk-next-net/master, thanks.