[RFC] net/vdev_netvsc: check for required related drivers

Message ID 20190313151858.2175-1-stephen@networkplumber.org (mailing list archive)
State Rejected, archived
Delegated to: Ferruh Yigit
Headers
Series [RFC] net/vdev_netvsc: check for required related drivers |

Checks

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

Commit Message

Stephen Hemminger March 13, 2019, 3:18 p.m. UTC
  The vdev_netvsc virtual driver that is used to do initialization
on Hyper-V/Azure won't work without failsafe and tap device.
If the related devices aren't present, it causes confusing errors
later in initialization when it crafts devargs and attempts to
send them to a device driver that isn't there.

Unfortunately, this is common with VPP where the TAP and FAILSAFE
PMD's are both optional.  The suggestion here is to detect this
in the startup phase earlier.

Alternative would be to use RTE_BUILD_BUG_ON(!defined(...))
but that would break people doing normal VPP build.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/vdev_netvsc/vdev_netvsc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
  

Comments

Matan Azrad March 14, 2019, 11:26 a.m. UTC | #1
Hi

From: Stephen Hemminger
> The vdev_netvsc virtual driver that is used to do initialization on Hyper-
> V/Azure won't work without failsafe and tap device.
> If the related devices aren't present, it causes confusing errors later in
> initialization when it crafts devargs and attempts to send them to a device
> driver that isn't there.
> 
> Unfortunately, this is common with VPP where the TAP and FAILSAFE PMD's
> are both optional.  The suggestion here is to detect this in the startup phase
> earlier.
> 
> Alternative would be to use RTE_BUILD_BUG_ON(!defined(...)) but that
> would break people doing normal VPP build.
> 

The failsafe and tap devices are created by the vdev_netvsc PMD, so it is not expected to find them in the scan time.
If the VM doesn't want vdev_netvsc driver to run, it have 2 options:
1. assign IP to the netvsc netdevs. 
2. run --vdev="vdev_netvsc0,ignore=1" - see documentation for more info.

> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/net/vdev_netvsc/vdev_netvsc.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c
> b/drivers/net/vdev_netvsc/vdev_netvsc.c
> index 801f54c96e01..9c262358b5ee 100644
> --- a/drivers/net/vdev_netvsc/vdev_netvsc.c
> +++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
> @@ -812,6 +812,20 @@ vdev_netvsc_scan_callback(__rte_unused void
> *arg)
>  	struct rte_devargs *devargs;
>  	struct rte_bus *vbus = rte_bus_find_by_name("vdev");
> 
> +	dev = vbus->find_device(NULL, vdev_netvsc_cmp_rte_device,
> +				"net_failsafe");
> +	if (!dev) {
> +		DRV_LOG(ERR, "failsafe network device not present");
> +		return;
> +	}
> +
> +	dev = vbus->find_device(NULL, vdev_netvsc_cmp_rte_device,
> +				"net_tap");
> +	if (!dev) {
> +		DRV_LOG(ERR, "tap network device driver not present");
> +		return;
> +	}
> +
>  	RTE_EAL_DEVARGS_FOREACH("vdev", devargs)
>  		if (!strncmp(devargs->name, VDEV_NETVSC_DRIVER_NAME,
>  			     VDEV_NETVSC_DRIVER_NAME_LEN))
> --
> 2.17.1
  
Stephen Hemminger March 14, 2019, 3:52 p.m. UTC | #2
On Thu, 14 Mar 2019 11:26:05 +0000
Matan Azrad <matan@mellanox.com> wrote:

> Hi
> 
> From: Stephen Hemminger
> > The vdev_netvsc virtual driver that is used to do initialization on Hyper-
> > V/Azure won't work without failsafe and tap device.
> > If the related devices aren't present, it causes confusing errors later in
> > initialization when it crafts devargs and attempts to send them to a device
> > driver that isn't there.
> > 
> > Unfortunately, this is common with VPP where the TAP and FAILSAFE PMD's
> > are both optional.  The suggestion here is to detect this in the startup phase
> > earlier.
> > 
> > Alternative would be to use RTE_BUILD_BUG_ON(!defined(...)) but that
> > would break people doing normal VPP build.
> >   
> 
> The failsafe and tap devices are created by the vdev_netvsc PMD, so it is not expected to find them in the scan time.
> If the VM doesn't want vdev_netvsc driver to run, it have 2 options:
> 1. assign IP to the netvsc netdevs. 
> 2. run --vdev="vdev_netvsc0,ignore=1" - see documentation for more info.
> 


How do we improve the error reporting for configurations that have run Hyper-V/Azure
but forget to build failsafe, tap or mlx5 device drivers. Right now the error messages
are confusing, misleading and waste significant amount of developer time.

What solution do you propose? The DPDK build system is not as smart as kernel
Kconfig, there is no way to force dependencies.
  
Matan Azrad March 17, 2019, 6:53 a.m. UTC | #3
Hi

From: Stephen Hemminger
> wrote:
> 
> > Hi
> >
> > From: Stephen Hemminger
> > > The vdev_netvsc virtual driver that is used to do initialization on
> > > Hyper- V/Azure won't work without failsafe and tap device.
> > > If the related devices aren't present, it causes confusing errors
> > > later in initialization when it crafts devargs and attempts to send
> > > them to a device driver that isn't there.
> > >
> > > Unfortunately, this is common with VPP where the TAP and FAILSAFE
> > > PMD's are both optional.  The suggestion here is to detect this in
> > > the startup phase earlier.
> > >
> > > Alternative would be to use RTE_BUILD_BUG_ON(!defined(...)) but that
> > > would break people doing normal VPP build.
> > >
> >
> > The failsafe and tap devices are created by the vdev_netvsc PMD, so it is
> not expected to find them in the scan time.
> > If the VM doesn't want vdev_netvsc driver to run, it have 2 options:
> > 1. assign IP to the netvsc netdevs.
> > 2. run --vdev="vdev_netvsc0,ignore=1" - see documentation for more info.
> >
> 
> 
> How do we improve the error reporting for configurations that have run
> Hyper-V/Azure but forget to build failsafe, tap or mlx5 device drivers. Right
> now the error messages are confusing, misleading and waste significant
> amount of developer time.
> 
> What solution do you propose? The DPDK build system is not as smart as
> kernel Kconfig, there is no way to force dependencies.

Why?
Can't you condition the vdev_netvsc compilation in failsafe and tap compilation?

1 more option:
To condition the vdev_netvsc automated probing (in vdev_netvsc_custom_scan_add) by "ifdef" statement in failsafe and tap compilation existence.

Moreover, you can add clearer massage for this case if vdev_netvsc is running (vdev_netvsc_vdev_probe).

Matan.
  
Stephen Hemminger March 18, 2019, 3:05 p.m. UTC | #4
On Sun, 17 Mar 2019 06:53:02 +0000
Matan Azrad <matan@mellanox.com> wrote:

> Hi
> 
> From: Stephen Hemminger
> > wrote:
> >   
> > > Hi
> > >
> > > From: Stephen Hemminger  
> > > > The vdev_netvsc virtual driver that is used to do initialization on
> > > > Hyper- V/Azure won't work without failsafe and tap device.
> > > > If the related devices aren't present, it causes confusing errors
> > > > later in initialization when it crafts devargs and attempts to send
> > > > them to a device driver that isn't there.
> > > >
> > > > Unfortunately, this is common with VPP where the TAP and FAILSAFE
> > > > PMD's are both optional.  The suggestion here is to detect this in
> > > > the startup phase earlier.
> > > >
> > > > Alternative would be to use RTE_BUILD_BUG_ON(!defined(...)) but that
> > > > would break people doing normal VPP build.
> > > >  
> > >
> > > The failsafe and tap devices are created by the vdev_netvsc PMD, so it is  
> > not expected to find them in the scan time.  
> > > If the VM doesn't want vdev_netvsc driver to run, it have 2 options:
> > > 1. assign IP to the netvsc netdevs.
> > > 2. run --vdev="vdev_netvsc0,ignore=1" - see documentation for more info.
> > >  
> > 
> > 
> > How do we improve the error reporting for configurations that have run
> > Hyper-V/Azure but forget to build failsafe, tap or mlx5 device drivers. Right
> > now the error messages are confusing, misleading and waste significant
> > amount of developer time.
> > 
> > What solution do you propose? The DPDK build system is not as smart as
> > kernel Kconfig, there is no way to force dependencies.  
> 
> Why?
> Can't you condition the vdev_netvsc compilation in failsafe and tap compilation?
> 
> 1 more option:
> To condition the vdev_netvsc automated probing (in vdev_netvsc_custom_scan_add) by "ifdef" statement in failsafe and tap compilation existence.
> 
> Moreover, you can add clearer massage for this case if vdev_netvsc is running (vdev_netvsc_vdev_probe).

Right now FAILSAFE, TAP and MLX5 are all optional in how VPP builds.
If FAILSAFE or TAP is not configured then vdev_netvsc will try and configure them anyway
and fail obscurely.

My current preference would be:
	RTE_BUILD_BUG_ON(!defined(CONFIG_RTE_LIBRTE_PMD_FAILSAFE));
  

Patch

diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c b/drivers/net/vdev_netvsc/vdev_netvsc.c
index 801f54c96e01..9c262358b5ee 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -812,6 +812,20 @@  vdev_netvsc_scan_callback(__rte_unused void *arg)
 	struct rte_devargs *devargs;
 	struct rte_bus *vbus = rte_bus_find_by_name("vdev");
 
+	dev = vbus->find_device(NULL, vdev_netvsc_cmp_rte_device,
+				"net_failsafe");
+	if (!dev) {
+		DRV_LOG(ERR, "failsafe network device not present");
+		return;
+	}
+
+	dev = vbus->find_device(NULL, vdev_netvsc_cmp_rte_device,
+				"net_tap");
+	if (!dev) {
+		DRV_LOG(ERR, "tap network device driver not present");
+		return;
+	}
+
 	RTE_EAL_DEVARGS_FOREACH("vdev", devargs)
 		if (!strncmp(devargs->name, VDEV_NETVSC_DRIVER_NAME,
 			     VDEV_NETVSC_DRIVER_NAME_LEN))