From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f54.google.com (mail-wg0-f54.google.com [74.125.82.54]) by dpdk.org (Postfix) with ESMTP id DA4D49AD7 for ; Wed, 18 Feb 2015 02:17:51 +0100 (CET) Received: by mail-wg0-f54.google.com with SMTP id y19so39281331wgg.13 for ; Tue, 17 Feb 2015 17:17:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=9DbFwCwyWsCF1RwBpQN2gf3C+2We8TFYrDBvtSBNOPE=; b=d/zdM9PRWvbevhTk26TtQvFKgSrb52QAYV+2esvMDd1OWShGXcf+OgMnfdz8OCGQ6Z iTeba4ggYmIjLr2XckDYHL01cQ/R+d8qieaN5C8lXIJSgBdJga2rsRJMg+jPrDcbz0W6 ilc+P0w+XbFYyBSpIS4uDdom4yTXeUeDe9rMYNPTAqnv0v/c3rY/r58qIHNzxXix1T4v /o6a6O4y2SSnpTIR/vx5IiRqyFZUcKfQNNhmruVtzuz0grfQ7VIWxT0omlmyCFopt9Il WC/O8Kqpfaybra27cSwQ3ghsrkeHXaKa5O0lZHHjVSg5GGEMT1CuJ0Z5J82yUrxu1Vn0 pFJQ== X-Gm-Message-State: ALoCoQmuXdAuTfTYTUh+mUaquii8PozsqNPhRT4gkIfRs4IfXemiMOG7ezXj0j4jvDirbNIiFQAp X-Received: by 10.180.107.201 with SMTP id he9mr62365818wib.46.1424222271623; Tue, 17 Feb 2015 17:17:51 -0800 (PST) Received: from xps13.localnet (guy78-1-82-235-116-147.fbx.proxad.net. [82.235.116.147]) by mx.google.com with ESMTPSA id uo6sm29805139wjc.49.2015.02.17.17.17.50 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 17 Feb 2015 17:17:50 -0800 (PST) From: Thomas Monjalon To: Tetsuya Mukawa Date: Wed, 18 Feb 2015 02:17:13 +0100 Message-ID: <3631728.EyWA3tZ5Au@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: <54E31762.5070106@igel.co.jp> References: <1423470639-15744-2-git-send-email-mukawa@igel.co.jp> <1733734.Ud8Ok1nOed@xps13> <54E31762.5070106@igel.co.jp> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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:17:52 -0000 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.