From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f169.google.com (mail-pd0-f169.google.com [209.85.192.169]) by dpdk.org (Postfix) with ESMTP id 2B5E6B614 for ; Wed, 18 Feb 2015 02:55:50 +0100 (CET) Received: by pdbfp1 with SMTP id fp1so47814626pdb.9 for ; Tue, 17 Feb 2015 17:55:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=RQN3hkACZBkcyu3POHqlio9zFhGHrr1xy9+qJCHW7+c=; b=V0sEj63WiFzVvTJe/WuPYFlsOBnLu+9TwFFISNRY7bvVMpyma71vpY9QCWlYoWBBvr VlEK0LYvJz2foElWUe+0ng7joZdKsCmfPtwykTjmdG9G1wGJJmA4bqutQs/groYs8Iu4 c09/KZz3Q7s/oPGVlc5b0I+mAeQ8YqYnCAHsmqUzM07f207dcH5X/FE3ccDDG3EOtTDk zI0av1903uQpN+4nSHFfiCTXMTMLN2R2ULKGy2zIEHeS2D9mSi/SuI0BZkpTR8zoTqEq hOU2mFVt3Sgsod3eUndu1IdUSgFbcnAyWa4WqzUE5pXuARYvcxXDDPr48eoEgWDZ4EO6 2hfA== X-Gm-Message-State: ALoCoQl9DdIM5uolUj6EFZOHhlTpZh45qZ1QFH3OJQv2YFkHVXHpyB9oIPfS2RZgJr3giq3nUwvf X-Received: by 10.68.135.166 with SMTP id pt6mr23627712pbb.74.1424224549589; Tue, 17 Feb 2015 17:55:49 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id zd14sm4198701pab.20.2015.02.17.17.55.48 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 17 Feb 2015 17:55:49 -0800 (PST) Message-ID: <54E3F123.7010304@igel.co.jp> Date: Wed, 18 Feb 2015 10:55:47 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Thomas Monjalon References: <1423470639-15744-2-git-send-email-mukawa@igel.co.jp> <1733734.Ud8Ok1nOed@xps13> <54E31762.5070106@igel.co.jp> <3631728.EyWA3tZ5Au@xps13> In-Reply-To: <3631728.EyWA3tZ5Au@xps13> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v8 12/14] eal/pci: Add rte_eal_dev_attach/detach() functions 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: Wed, 18 Feb 2015 01:55:50 -0000 On 2015/02/18 10:17, Thomas Monjalon wrote: > 2015-02-17 19:26, Tetsuya Mukawa: >> On 2015/02/17 18:23, Thomas Monjalon wrote: >>> 2015-02-17 17:51, Tetsuya Mukawa: >>>> On 2015/02/17 10:48, Thomas Monjalon wrote: >>>>> 2015-02-16 13:14, Tetsuya Mukawa: >>>>>> +/* attach the new physical device, then store port_id of the device */ >>>>>> +static int >>>>>> +rte_eal_dev_attach_pdev(struct rte_pci_addr *addr, uint8_t *port_id) >>>>>> +{ >>>>>> + uint8_t new_port_id; >>>>>> + struct rte_eth_dev devs[RTE_MAX_ETHPORTS]; >>>>>> + >>>>>> + if ((addr == NULL) || (port_id == NULL)) >>>>>> + goto err; >>>>>> + >>>>>> + /* save current port status */ >>>>>> + if (rte_eth_dev_save(devs, sizeof(devs))) >>>>>> + goto err; >>>>>> + /* re-construct pci_device_list */ >>>>>> + if (rte_eal_pci_scan()) >>>>>> + goto err; >>>>>> + /* invoke probe func of the driver can handle the new device */ >>>>>> + if (rte_eal_pci_probe_one(addr)) >>>>>> + goto err; >>>>> You should get the port_id from the previous function instead of searching it. >>>> I agree this will beautify this code, but actually to do like above >>>> changes current DPDK code much more, and it will not be clear, and not >>>> beautiful. >>>> >>>> Could I explain it more. >>>> Problem is initialization sequence of virtual device and physical device >>>> are completely different. >>>> >>>> (1) Attaching a physical device case >>>> - To return port id, pci_invoke_all_drivers() needs to return port id. >>>> - It means "devinit" of "struct rte_pci_driver" needs to return port_id. >>>> - "devinit" will be rte_eth_dev_init(). But if the device is virtual, >>>> this function is not implemented. >>>> >>>> (2) Attaching virtual device case >>>> - To return port id from rte_eal_pci_probe_one(), >>>> pci_invoke_all_drivers() needs to return port id. >>>> - It means "init" of "struct rte_driver" needs to return port_id. >>>> - "init" will be implemented in PMD. But this function has different >>>> usage in physical device and virtual device. >>>> - Especially, In the case of physical device, "init" doesn't allocate >>>> eth device, so cannot return port id. >>>> >>>> As a result, to remove rte_eth_dev_save() and >>>> rte_eth_dev_get_changed_port(), below different functions needs to >>>> return port id. >>>> - "devinit" of "struct rte_pci_driver". >>>> - "init" of "struct rte_driver". >>> Yes, exactly, >>> I think you shouldn't hesitate to improve existing EAL code. >>> And I also think we should try to remove differences between virtual and >>> pci devices. >> I strongly agree with it. But I haven't investigated how to remove it so >> far. >> To be honest, I want to submit hotplug patches to DPDK-2.0. >> Is above functionality needed to merge the hotplug patches? >> I guess I will not be able to remove it by 23rd. > Obviously, it would be better to have it in dpdk-2.0.0-rc1. > If not possible to fix it, would it be possible to work on other comments > and keep this cleanup for post-rc1 integration? > I feel this cleanup is important to get the right design but it wouldn't be > fair to block this (old) patchset for this reason. > I appreciate for your suggestion. I will keep working on it for post-rc1. Thanks, Tetsuya