Re: [dpdk-dev] [PATCH] eal: fix secondary process segfault on multipe virtio devices

[Thomas Monjalon <thomas@monjalon.net>]

02/06/2017 18:20, Jianfeng Tan:
> Suppose we have 2 virtio devices for a VM, with only the first one,
> virtio0, binding to igb_uio. Start a primary DPDK process, driving
> only virtio0. Then start a secondary DPDK process, it encounters
> segfault at eth_virtio_dev_init() because hw is NULL, when trying
> to initialize the 2nd virtio devices.
>     1539                    if (!hw->virtio_user_dev) {
> 
> We could add a precheck to return error when hw is NULL. But the
> root cause is that virtio devices which are not driven by the primary
> process are not exluded by secondary eal probe function.
> 
> To support legacy virtio devices bound to none kernel driver, we
> removed RTE_PCI_DRV_NEED_MAPPING in
> commit 962cf902e6eb ("pci: export device mapping functions").
> At the boot of primary process, ether dev is allocated in rte_eth_devices
> array, rte_eth_dev_data is also allocated in rte_eth_dev_data array; then
> probe function fails; and ether dev is released. However, the entry in
> rte_eth_dev_data array is not cleared. Then we start secondary process,
> and try to attach the virtio device that not used in primary process,
> the field, dev_private (or hw), in rte_eth_dev_data, is NULL.
> 
> To fail the dev attach, we need to clear the field, name, when we
> release any ether devices in primary, so that below loop in
> rte_eth_dev_attach_secondary() will not find any matched names.
>         for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
>                 if (strcmp(rte_eth_dev_data[i].name, name) == 0)
>                         break;
>         }
> 
> Fixes: 6d890f8ab512 ("Fixes: net/virtio: fix multiple process support")

It probably needs to be Cc: stable@dpdk.org

> Reported-by: Reshma Pattan <reshma.pattan@intel.com>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
[...]
>  	eth_dev->data->dev_private = NULL;
>  
> +	/* Secondary process will use this field, name, for secondary attach,
> +	 * clear this field to avoid attaching any released ports in primary.
> +	 */
> +	memset(eth_dev->data->name, 0, RTE_ETH_NAME_MAX_LEN);

I think it may be sufficient to set an empty string:
eth_dev->data->name = '\0';