Re: [spp] [PATCH v2 01/12] drivers/vhost: add multi process supported vhost PMD for SPP

[Yasufumi Ogawa <yasufum.o@gmail.com>]

Hi,

On 2019/12/25 13:49, Itsuro Oda wrote:
> vhost PMD can not be used by secondary processes since DPDK 18.11.
> SPP project decided to have own vhost PMD which can be used by
> secondary processes at the moment. This vhost PMD is based on the
> original vhost PMD but is simplified very much only to support
> functions used by SPP. Thereby it becomes easy to fix the probrem.
> 
> The main idea of the fix is that execution of vhost start/stop
> is moved to eth_dev_start/stop from probe/remove.
> 
> Note that only process which executes eth_dev_start can use the
> vhost device although the vhost device is shared among the primary
> process and secondary processes. Once eth_dev_stop is executed by
> the process which used the vhost device, it is available to be
> used by any process. It is user responsibility that multipul
> processes don't use the vhost device at the same time.
> 
> Signed-off-by: Itsuro Oda <oda@valinux.co.jp>
> ---
>   src/drivers/vhost/Makefile                    |  28 +
>   .../vhost/rte_pmd_spp_vhost_version.map       |   4 +
>   src/drivers/vhost/rte_spp_vhost.c             | 588 ++++++++++++++++++
>   3 files changed, 620 insertions(+)
>   create mode 100644 src/drivers/vhost/Makefile
>   create mode 100644 src/drivers/vhost/rte_pmd_spp_vhost_version.map
>   create mode 100644 src/drivers/vhost/rte_spp_vhost.c
> 
> diff --git a/src/drivers/vhost/Makefile b/src/drivers/vhost/Makefile
[...]
> +
> +static inline struct pmd_internal *
> +find_internal_resource(int vid)
> +{
> +	struct pmd_internal *internal;
> +	int i;
> +	char ifname[PATH_MAX];
This variable is declared, but not used from anywhere.

> +
> +	if (rte_vhost_get_ifname(vid, ifname, sizeof(ifname)) == -1)
> +		return NULL;
[...]
> +}
> +
> +static const struct eth_dev_ops ops = {
> +	.dev_start = eth_dev_start,
> +	.dev_stop = eth_dev_stop,
> +	.dev_configure = eth_dev_configure,
> +	.dev_infos_get = eth_dev_info, > +	.rx_queue_setup = eth_rx_queue_setup,
> +	.tx_queue_setup = eth_tx_queue_setup,
> +	.rx_queue_release = eth_queue_release,
> +	.tx_queue_release = eth_queue_release,
> +	.link_update = eth_link_update,
> +	.stats_get = eth_stats_get,
> +	.stats_reset = eth_stats_reset,
> +};
This struct causes compilation errors because of incompatible pointer types.

$ make
== src
== drivers
== vhost
   CC rte_spp_vhost.o
rte_spp_vhost.c:371:19: error: initialization from incompatible pointer 
type [-Werror=incompatible-pointer-types]
   .dev_infos_get = eth_dev_info,
                    ^~~~~~~~~~~~
rte_spp_vhost.c:371:19: note: (near initialization for ‘ops.dev_infos_get’)
rte_spp_vhost.c:378:17: error: initialization from incompatible pointer 
type [-Werror=incompatible-pointer-types]
   .stats_reset = eth_stats_reset,
                  ^~~~~~~~~~~~~~~
rte_spp_vhost.c:378:17: note: (near initialization for ‘ops.stats_reset’)
cc1: error: unrecognized command line option 
‘-Wno-address-of-packed-member’ [-Werror]
cc1: all warnings being treated as errors

 From my understanding, eth_dev_info() and eth_stats_reset() are defined 
as void in your patch, but DPDK expects int by referring the definition 
of eth_dev_ops in lib/librte_ethdev/rte_ethdev_core.h:609.

Regards,
Yasufumi
> +
> +static int
[...]