Re: [dpdk-dev] [PATCH v2] virtio: fix modify drv_flags for specific device

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Mon, May 09, 2016 at 09:14:41AM +0000, Tan, Jianfeng wrote:
>     When two virtio devices have different flags, it may lead to wrong
>     result. Then some unexpected behaviors happen, such as virtio would
>     set irq config when it's not supported.

Yes, that's the issue; and that's exactly something I was looking for in
a commit log.

Patch is applied to dpdk-next-virtio, with following changes:

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 1864186..995618a 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1076,11 +1076,6 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 		dev_flags &= ~RTE_ETH_DEV_INTR_LSC;
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
-	/* For virtio devices, dev_flags are decided according to feature
-	 * negotiation, aka if VIRTIO_NET_F_STATUS is set, and which kernel
-	 * driver is used, dynamically. And we should keep drv_flags shared
-	 * and unvaried.
-	 */
 	eth_dev->data->dev_flags = dev_flags;
 
 	rx_func_get(eth_dev);
---

Without this patch as the context, mentioning "drv_flags" here confuses
people.

And FYI, I made folloiwng commit log rewords.

Thanks.

	--yliu

 ---
 virtio: fix overwritten drv_flags

 The "drv_flags" is set with device as the input, which means different
 device (say, modern vs legacy) could end up with a different value. And
 the fact that "drv_flags" is shared by all devices means that every time
 we add a new device, it simply overwrites the value configured from the
 last device.

 Therefore, when two virtio devices have different flags, it may lead to
 wrong result, such as virtio would set irq config when it's not supported.

 Making the flag per device (using "dev->data->dev_flags") could let us
 have different value for each device, which would avoid the above issue.