From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 5637B2C49 for ; Tue, 8 Mar 2016 18:00:57 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 08 Mar 2016 09:00:56 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,557,1449561600"; d="scan'208";a="665925703" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by FMSMGA003.fm.intel.com with ESMTP; 08 Mar 2016 09:00:57 -0800 Received: from fmsmsx118.amr.corp.intel.com (10.18.116.18) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 8 Mar 2016 09:00:56 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx118.amr.corp.intel.com (10.18.116.18) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 8 Mar 2016 09:00:55 -0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.136]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.132]) with mapi id 14.03.0248.002; Wed, 9 Mar 2016 01:00:54 +0800 From: "Xie, Huawei" To: Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH v4 4/4] virtio: return 1 to tell the upper layer we don't take over this device Thread-Index: AdFzj4MuUDrymegJT1SI/9bUpaqDKw== Date: Tue, 8 Mar 2016 17:00:53 +0000 Message-ID: References: <20151222035041.GA7532@pxdev.xzpeter.org> <8208032.DLqNqWtROc@xps13> <1581750.D14419ZT36@xps13> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" , "Troitsky, Nikita" Subject: Re: [dpdk-dev] [PATCH v4 4/4] virtio: return 1 to tell the upper layer we don't take over this 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: Tue, 08 Mar 2016 17:00:58 -0000 On 3/1/2016 6:09 PM, Xie, Huawei wrote:=0A= > On 3/1/2016 5:57 PM, Thomas Monjalon wrote:=0A= >> 2016-03-01 08:39, Xie, Huawei:=0A= >>> On 3/1/2016 4:24 PM, Thomas Monjalon wrote:=0A= >>>> 2016-03-01 07:53, Xie, Huawei:=0A= >>>>> On 3/1/2016 3:18 PM, Thomas Monjalon wrote:=0A= >>>>>> 2016-02-26 09:53, Huawei Xie:=0A= >>>>>>> @@ -1037,8 +1039,11 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_= dev)=0A= >>>>>>> =0A= >>>>>>> pci_dev =3D eth_dev->pci_dev;=0A= >>>>>>> =0A= >>>>>>> - if (vtpci_init(pci_dev, hw) < 0)=0A= >>>>>>> - return -1;=0A= >>>>>>> + ret =3D vtpci_init(pci_dev, hw);=0A= >>>>>>> + if (ret) {=0A= >>>>>>> + rte_free(eth_dev->data->mac_addrs);=0A= >>>>>> The freeing seems not related to this patch.=0A= >>>>> I can send a separate patch, ok within this patchset?=0A= >>>> Yes=0A= >>>>=0A= >>>>>> [...]=0A= >>>>>>> PMD_INIT_LOG(INFO, "trying with legacy virtio pci.");=0A= >>>>>>> - if (legacy_virtio_resource_init(dev, hw) < 0)=0A= >>>>>>> + if (legacy_virtio_resource_init(dev, hw) < 0) {=0A= >>>>>>> + if (dev->kdrv =3D=3D RTE_KDRV_UNKNOWN) {=0A= >>>>>>> + PMD_INIT_LOG(INFO,=0A= >>>>>>> + "skip kernel managed virtio device.");=0A= >>>>>>> + return 1;=0A= >>>>>>> + }=0A= >>>>>>> return -1;=0A= >>>>>>> + }=0A= >>>>>> You cannot skip a device if it was whitelisted.=0A= >>>>>> I think you should check RTE_DEVTYPE_WHITELISTED_PCI and throw an er= ror=0A= >>>>>> in this case.=0A= >>>>> I feel there is a subtle difference on the understanding of -w args. = To=0A= >>>>> me, without it, probe all devices; with it, only probe whiltelisted A= PI.=0A= >>>>> That is all.=0A= >>>> I don't know if it is clearly documented indeed.=0A= >>>>=0A= >>>>> Do you mean that -w implies that devices whitelisted must be probed= =0A= >>>>> successfully otherwise we throw an error? If i get it right, then wha= t=0A= >>>>> about the devices whitelisted but without PMD driver?=0A= >>>> Yes we should probably consider the whitelist as a "forced" init.=0A= >>>> Later, we could introduce some device flags for probing/discovery:=0A= >>>> PROBE_AUTO, PROBE_FORCE, PROBE_IGNORE. It would make white/black list= =0A= >>>> more precise.=0A= >>>>=0A= >>>>> I will fix, :).=0A= >>>>> if (dev->kdrv =3D=3D RTE_KDRV_UNKNOWN && dev->devargs->type !=3D=0A= >>>>> RTE_DEVTYPE_WHITELISTED_PCI) {=0A= >>>>> ....=0A= >>>>> return 1;=0A= >>>>> }=0A= >>>> You should also consider the blacklist case: if there is a blacklist,= =0A= >>>> the not blacklisted devices must be initialised or throw an error.=0A= >>>>=0A= >>> Don't we already skip probing the blacklisted device in=0A= >>> rte_eal_pci_probe_one_driver?=0A= >> Yes, I'm talking about the devices which are not blacklisted.=0A= >> Having some blacklisted devices imply that others are implicitly whiteli= sted.=0A= > For blacklist, it only means the blacklisted device should be excluded=0A= > from being probed. It doesn't mean all other devices should be probed=0A= > either successfully or otherwise throw an error which cause DPDK exit.=0A= > Even that, the upper layer should explicitly put the non-blacklisted=0A= > device into whitelist, i mean here we should only deal with whitelist.=0A= =0A= Thomas, comments?=0A= Btw, i have reworked the patch, but git am always complains patch format=0A= detection failed. Investigating. Will send it later.=0A= =0A= >=0A= =0A=