From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id C63437DEC for ; Mon, 9 May 2016 19:56:53 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP; 09 May 2016 10:56:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,601,1455004800"; d="scan'208";a="99983258" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga004.fm.intel.com with ESMTP; 09 May 2016 10:56:52 -0700 Date: Mon, 9 May 2016 11:01:26 -0700 From: Yuanhan Liu To: "Tan, Jianfeng" Cc: David Marchand , "dev@dpdk.org" , "Xie, Huawei" Message-ID: <20160509180126.GB5641@yliu-dev.sh.intel.com> References: <1461637474-110602-1-git-send-email-jianfeng.tan@intel.com> <1461866939-38689-1-git-send-email-jianfeng.tan@intel.com> <20160504231819.GS5641@yliu-dev.sh.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v2] virtio: fix modify drv_flags for specific device X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 May 2016 17:56:54 -0000 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.