From patchwork Thu Aug 4 13:21:57 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Ferruh Yigit X-Patchwork-Id: 15124 Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 82E60377E; Thu, 4 Aug 2016 15:22:04 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 43EAD2C0C for ; Thu, 4 Aug 2016 15:22:03 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 04 Aug 2016 06:22:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.28,470,1464678000"; d="scan'208"; a="1008360072" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.104]) ([10.237.220.104]) by orsmga001.jf.intel.com with ESMTP; 04 Aug 2016 06:21:57 -0700 To: Igor Ryzhov References: <57A32814.1000404@intel.com> <0AB336DF-9C66-4826-BA17-EDE1F8D6A2EA@nfware.com> Cc: dev@dpdk.org, David Marchand , "Liu, Yuanhan" From: Ferruh Yigit Message-ID: <57A34175.1040204@intel.com> Date: Thu, 4 Aug 2016 14:21:57 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <0AB336DF-9C66-4826-BA17-EDE1F8D6A2EA@nfware.com> Subject: Re: [dpdk-dev] rte_eth_dev_attach returns 0, although device is not attached 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 8/4/2016 12:51 PM, Igor Ryzhov wrote: > Hello Ferruh, > >> 4 авг. 2016 г., в 14:33, Ferruh Yigit написал(а): >> >> Hi Igor, >> >> On 8/3/2016 5:58 PM, Igor Ryzhov wrote: >>> Hello. >>> >>> Function rte_eth_dev_attach can return false positive result. >>> It happens because rte_eal_pci_probe_one returns zero if no driver is found for the device: >>> ret = pci_probe_all_drivers(dev); >>> if (ret < 0) >>> goto err_return; >>> return 0; >>> (pci_probe_all_drivers returns 1 in that case) >>> >>> For example, it can be easily reproduced by trying to attach virtio device, managed by kernel driver. >> >> You are right, and I did able to reproduce this issue with virtio as you >> suggest. >> >> But I wonder why rte_eth_dev_get_port_by_addr() is not catching this. >> Perhaps a dev->attached check needs to be added into this function. With a second check, rte_eth_dev_get_port_by_addr() catches it if the driver is missing. But for virtio case, problem is not missing driver. Problem is eth_virtio_dev_init() is returning a positive value on fail. Call stack is: rte_eal_pci_probe_one pci_probe_all_drivers rte_eal_pci_probe_one_driver rte_eth_dev_init eth_virtio_dev_init So rte_eal_pci_probe_one_driver() also returns positive value, as no driver found, and rte_eth_dev_get_port_by_addr() returns a valid port_id, since rte_eth_dev_init() allocated an eth_dev. Briefly, this can be fixed in virtio pmd, instead of eal pci. >> >>> >>> I think it should be: >>> ret = pci_probe_all_drivers(dev); >>> if (ret) >>> goto err_return; >>> return 0; >> >> Your proposal looks good to me. Will you send a patch? > Original code silently ignores the if driver is missing for that dev, although it is still questionable, I think we can keep this as it is. > Patch sent. Sorry for this, but can you please test with following modification in virtio: index 07d6449..c74eeee 100644 > >> >>> Best regards, >>> Igor >>> >> > --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -1156,7 +1156,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) if (pci_dev) { ret = vtpci_init(pci_dev, hw, &dev_flags); if (ret) - return ret; + return -1; } /* Reset the device although not necessary at startup */