From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM03-DM3-obe.outbound.protection.outlook.com (mail-dm3nam03on0075.outbound.protection.outlook.com [104.47.41.75]) by dpdk.org (Postfix) with ESMTP id D072947D0 for ; Fri, 15 Jul 2016 12:35:53 +0200 (CEST) Received: from BN3PR0301CA0013.namprd03.prod.outlook.com (10.160.180.151) by MWHPR03MB2509.namprd03.prod.outlook.com (10.169.201.11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.523.12; Fri, 15 Jul 2016 10:35:51 +0000 Received: from BN1AFFO11FD014.protection.gbl (2a01:111:f400:7c10::101) by BN3PR0301CA0013.outlook.office365.com (2a01:111:e400:4000::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.539.14 via Frontend Transport; Fri, 15 Jul 2016 10:35:51 +0000 Authentication-Results: spf=fail (sender IP is 192.88.168.50) smtp.mailfrom=nxp.com; rehivetech.com; dkim=none (message not signed) header.d=none;rehivetech.com; dmarc=fail action=none header.from=nxp.com; Received-SPF: Fail (protection.outlook.com: domain of nxp.com does not designate 192.88.168.50 as permitted sender) receiver=protection.outlook.com; client-ip=192.88.168.50; helo=tx30smr01.am.freescale.net; Received: from tx30smr01.am.freescale.net (192.88.168.50) by BN1AFFO11FD014.mail.protection.outlook.com (10.58.52.74) with Microsoft SMTP Server (TLS) id 15.1.523.9 via Frontend Transport; Fri, 15 Jul 2016 10:35:50 +0000 Received: from [10.232.14.199] (Tophie.ap.freescale.net [10.232.14.199]) by tx30smr01.am.freescale.net (8.14.3/8.14.0) with ESMTP id u6FAZlqe025194; Fri, 15 Jul 2016 03:35:48 -0700 To: Jan Viktorin References: <1466510566-9240-1-git-send-email-shreyansh.jain@nxp.com> <1468303282-2806-1-git-send-email-shreyansh.jain@nxp.com> <1468303282-2806-17-git-send-email-shreyansh.jain@nxp.com> <20160714185148.738f1fa3@jvn> CC: , , From: Shreyansh jain Message-ID: <5788BCA9.5010205@nxp.com> Date: Fri, 15 Jul 2016 16:06:25 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <20160714185148.738f1fa3@jvn> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-EOPAttributedMessage: 0 X-Matching-Connectors: 131130525507491796; (91ab9b29-cfa4-454e-5278-08d120cd25b8); () X-Forefront-Antispam-Report: CIP:192.88.168.50; IPV:NLI; CTRY:US; EFV:NLI; SFV:NSPM; SFS:(10009020)(6009001)(7916002)(2980300002)(1109001)(1110001)(339900001)(189002)(199003)(377454003)(24454002)(87936001)(230700001)(81166006)(81156014)(33656002)(85426001)(7846002)(2906002)(4326007)(356003)(11100500001)(586003)(86362001)(54356999)(59896002)(65806001)(23746002)(8676002)(65816999)(50466002)(87266999)(76176999)(50986999)(64126003)(65956001)(104016004)(5890100001)(106466001)(2950100001)(36756003)(6806005)(47776003)(105606002)(68736007)(4001350100001)(110136002)(8936002)(97736004)(19580405001)(80316001)(83506001)(92566002)(77096005)(93886004)(189998001)(19580395003)(305945005); DIR:OUT; SFP:1101; SCL:1; SRVR:MWHPR03MB2509; H:tx30smr01.am.freescale.net; FPR:; SPF:Fail; PTR:InfoDomainNonexistent; MX:1; A:1; LANG:en; X-Microsoft-Exchange-Diagnostics: 1; BN1AFFO11FD014; 1:lTn+xU3L0w/bcCVvxWm57PEOenRj7wgm8nZH0PHXgImRoZ9tOMY5sKk3BeUkqL7FUE4coTIDIRS5BLvLXeJSv8lKNSWQeEA4ONzpEz42N/AsEVo/pMB5tOehgcVWFwNd4mYx2LxNQI4aoLjvh+mkVtPkBbIDcAE3V0rfM0rfPFFKVShPfUxBpHMDNR8vsGmmQgM3Q4wH27HupfxlPjFKlSPXIi1Ars3yfBcREOHA5sBloOKepWIfaCJLvM/Xr5w7TwzawMXxn9hcRqyoKdAvnetIHs+yTRhlonV0wnuz/yzpCOmcmgaQRc/d/o8XEU/fP8s1vIriBVlP8BjqCxkWUoSIL6GzjkpZRTrqcwY0kIJvdrNvi8xnwYJNKM+itH69sfPbaPZeeb/UwNhry67DedpTY84YKjs0u7nFnXyW3xCqJBJ4RwFKr12FmAFKKT1pIJJODWUvmsodU812rJmqNdZacgVt5KpPqSfsO7UW/A8htaXusJz02fVQcAzP9eyVd71KvMQzrTPmZ/uiZ98kWB4OCif025tQknkL8ADG+7J/3s9eYVr2CgpmjlUVlLcsQ3nNabiAnXCyDPgI76X6kQ== X-MS-Office365-Filtering-Correlation-Id: cf5b3342-979f-44ce-3675-08d3ac9bca8a X-Microsoft-Exchange-Diagnostics: 1; MWHPR03MB2509; 2:3XBmjI2iS86GQJE6uWjWmmcjpP7XQuOh1eilyytsfboBm3YQqtVSAOjEvrrnHXio3Sq4KPopFKHO/ZiIMHZDP1un/Wyx1OEZWsGWyHbHdTiNDp9U6CbfmbzAXeRMR8+IJGUpCb6p6C01fBG9PwtURLgZ68aTaFm9QAjFH3EokKg2tp30UJgouzURqqkIAWaD; 3:UT9IcPfty6YvPbRtodbD5hOhT0TQPrp5saimBWvpdTeH7OfF//f5srSOCk6zabB8865dxF3yGqwY57jv31WgNxc15elrTtv/NuUQOnbOFi+RceHPtmlL5nvgmDEjGPVe2TrMKYNK3n1vKmz7yjuE5BXMbwUaHoLJoy3nyd2Y+SgcZLerW3+P5l8j1sIOrKo9A+ZQaERX84G3eHLQQsah+qcnbvl/cfhPd6KHGDFj9J0=; 25:cr6QnRKgL/prJPVw+FRfobKYsn/Q67c0kYiMe0diZO8KsAUQjMGfPBybpsTI+M/5TTLpMZCJ970WdbSUoU6kzxLVk43mNdKkFFJpca7CLOwiGLJNIwJTvQHC0td/S7FCyNTFCXzJIA3SGK8Rayk6U0IQQKFU2C7TDiFGytnsSx0TAOhO2uzy1yF443v7dlaAFivacpwpHPRMJ/jDiv35b12x/h8zj0u8Ec2ExQOkCfaK4Ru28ktB87L86McqUuLkP5eLipSJkcglXGWYd9+hwAJnz39hO67wbl42MFpGXJZbnTPgHfEHo4ey0dUgBymOAjyFxfEG/98j0AfaDRw/tFVwNCiBvgQ4WeISUJzla1nrXWYsKqvY0NGcQzgtf24qlEyC3eNg6F8LRmcsJ18wnveyVN9we+viJcTde4H/mRM= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:MWHPR03MB2509; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(278428928389397)(185117386973197); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(601004)(2401047)(13017025)(13015025)(13024025)(13023025)(13018025)(5005006)(8121501046)(10201501046)(3002001)(6055026); SRVR:MWHPR03MB2509; BCL:0; PCL:0; RULEID:(400006); SRVR:MWHPR03MB2509; X-Microsoft-Exchange-Diagnostics: 1; MWHPR03MB2509; 4:d4ZMzAyNamVDVWBZmxvI4MwE1PuTHUcHwOIQ29KZxj4vCdhINOH7RGGroAL8UXNCtRVCwRvNB7Vlnz7S3dSH8QlWa4kMAinDXH9u+CL1RYgREiN+FpuDuxfQyEijyPOHk/ECOjgu5HoCF5HXTS6FlR7HbmjcpK70xGF6hqGEzNZmRkB96Ej2oMW8qQUnaMhORG/QMbO6pCCpIVSNVQFbji/Q6SiOArIrH8M4lZCwM1XvSgeEZsaOl3h35G5750mQ42rt24BBR71SBx7rEXv+NKsBlhvZvLAQB7TDyNP4WwhE1gexBBR45VDQSFSuZpYK9J4bAMniVYB4T+OQzv09oUig4OogSSuPLomJXViP48kyKGtLHO1tjL45df0jUhoeXGYM3BIVf1Cj7kk71NFX+vgKVZYmNkpw37DggT3hk6smG2DMFxCENQ32R4FkjvBWEZX8yxaY95L/yBekbQBT3eQV+ZEPvnJ1RuuQlM/lokdsvTmS3sghUrpe4k0tmaQBFyUJdoBvxOZgHi/SMmartBfOBqO970q2U0AKq4Yy6ANjJR7lXdtrpazDvZxFg3WU X-Forefront-PRVS: 00046D390F X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1; MWHPR03MB2509; 23:Cfga2SnwF/pDtEGV3Jns4MadAkNAAUDXa5W/F?= =?Windows-1252?Q?wwENBOIgxrRpP7QOYW/adbZ01atMaOS9QJtUaEpzLzLuNQnFECBK1PgI?= =?Windows-1252?Q?dtYU9P6iK5qYU+mj+TD0M0hhZvq6sldbG/x9Lbu4tXgG4P3KR4XtrQLk?= =?Windows-1252?Q?1VRYaTgpUCQuIL9GRA43kg1MPTHsqaTc7k7trTVcqq1pNUxkzzWuLv2T?= =?Windows-1252?Q?pDPfnvzhWlppVIhYfIu6Xr/g+fWn7mnLEEATNeYCtc4TM96WSoWkTFT9?= =?Windows-1252?Q?huqETFSW8JJ2Qa6qq7UQDLeck0uXX225uNUwPlRjGz5FgKCFXN3GMI3V?= =?Windows-1252?Q?GyYDX/4AYwPHYoERRzxyUOCnEg6lpPUtMnPo7IwhDuW6ItNb6sk6Uk9L?= =?Windows-1252?Q?/GK4DVjzbGDBK+SFMiu5XuU2uK6GK0q7R23JKLdr7doXweTsgBrR0KZ5?= =?Windows-1252?Q?meUc0yqigMY8uoqAehncIWJLiIN+GKHIpzbM+hlhOgRWvhWOE33E2pyQ?= =?Windows-1252?Q?rTk/eGMJxdJXP4z9rcnYTbdGOCl3589y1UtFbFQujLPGPSnl4gjaNtq+?= =?Windows-1252?Q?qljJi1OOHwYdMdJkD3mwek/qjKQCdsfTEVnjDGLBMyepKxKpsRJXaeQr?= =?Windows-1252?Q?1z+843K6K5rncEZ+gupH2Z51P6hx7Kz9EaydB63SHXHX4q3hpDrYTYXd?= =?Windows-1252?Q?1fS15OsqzcopLJqOUg1iISk1Ui+Kj+aqCjdl8vsDmw3IZr9jnJ6zLQKo?= =?Windows-1252?Q?lmQr53fgZCJ+5kXacyxAMyyOpbccZKE4Ou/5E9jM6tSjrZP6YXgS6RBm?= =?Windows-1252?Q?rTUuqaCENumGWae9nLlA0TAqn2VoDcHZF2KWu+HT+1ijJhZh5qv5Alxy?= =?Windows-1252?Q?jN4qmZejAshaZZLZG0blniZ+nJB8CNR1K1bZHbUK7iWwuMsIqBRVi0Va?= =?Windows-1252?Q?HYpgdVNbC+HjnuJ4MXl422vRNV+1lYXuxuGqI/aGRcOshYTWaW67Tqwp?= =?Windows-1252?Q?Kkt9N2iaCxDiCcJeTCQA0nll83BuJEN8jvXcGl0rU/AUSOrRjYkH6+qc?= =?Windows-1252?Q?31prLSzZkBAY1PRNAMWbrViQaZRtRyMJKIhdqrtw+UtfFuwP3f55dNM6?= =?Windows-1252?Q?aTQ5iPtuWWNyccBpQZZYImisVctmepBOkjLB82341KiClNlG6+MqDKwf?= =?Windows-1252?Q?tRYD/Bo6LfM73UCcaAgWTc7aqBh3lA9zcSA0HuVxj9CqlOAq2IE8Wt8F?= =?Windows-1252?Q?8MW1EGzMSiKdUJvY9sfJw78jnyANUSIGTiYcP+tb3TMS8mEYc5ZZ0Jij?= =?Windows-1252?Q?dy53DpeNZzTb59HZsW45idzrf2raRq9ZfceQ+XfcVavh70wkfPO9FO6j?= =?Windows-1252?Q?+NLeZkjNMRKg5zIIxAShl1Q8LRjT5i6eI0gR7+rdJpMRCojjS0TlUHB2?= =?Windows-1252?Q?01emvlHWicKSS5+yNNDCYPoGJExsWIfafcNjz8kmg=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1; MWHPR03MB2509; 6:WZ1/3IzQxDNPxAq5C8QF7pIEBJ1WrEycAMybU/Nr4Uxk+3xmjwI2N9l/oyoWLBOWq9AptImk/V5eG4qzLZAq/4adWr14o1rM50i2E3aEWws7LULtJLEtohtoJjxKdgaBI/D+gXTEyL7GbKnOQoRCLtaZaM7ksK1atCg+xH3NJUeLIg9YCLQ8Vaor35H07/uxq2sIwDJmgxhY00ZeOl8zcsqKvK8pVnQYadZArLwDze272j0Y1JG4bB2SlKNDC8DInXbwr+nLXF44LCiUT+7K80M3pV2KbMnWwmm8JYXvHyU=; 5:Qr7elqnVW9WFBc5YbC5nsB+x+sbAOTi7X/Zas4mHVxkNShCpsLs15w7uVV5oSbfuiuDJNo3jwipaX+2JUwI08J9zfdw6wG244pr8oocgryQOZ2dIKPCGm5rPcN4KIQBtf/Sa3wS5KmspZ2Sh/aGc2KcMZH0aTy/AQpeYa2K03q8=; 24:UqZO0mnD4xXIOswVulB4wo0fqvzuKzbsZe/sb2a2mCGGYB1/O8P0/zj4ZwQlda0u7I9zBDAMUDkeaePDf78pooHMcpaPdRpz6Pl6eEg0KzQ=; 7:PWu0pg4jK3hsprGGWTZ4R5TT43ro9JekVzTsJk4NRu8TJK61DbjTwnETqTggmuG2Q0AAHeo7cikzlWo206CFjtiGrI2WsA02UJeTwF8Ono0zO7jhZ9xnqcKvOHV+Gd+G9YQ7EqUn++GV1Eg+smM54bmI/2cskG2CCxoxR0F3fZd5rLreY8QBZ0LpShrxVt4VXqduq9oyiZWvai1lhkicjvPNcyRLJ6IRJI7gK1ofePjNjcsoQ94DAMftfL1KKFif SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jul 2016 10:35:50.5619 (UTC) X-MS-Exchange-CrossTenant-Id: 5afe0b00-7697-4969-b663-5eab37d5f47e X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=5afe0b00-7697-4969-b663-5eab37d5f47e; Ip=[192.88.168.50]; Helo=[tx30smr01.am.freescale.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR03MB2509 Subject: Re: [dpdk-dev] [PATCH v6 16/17] ethdev: convert to eal hotplug 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: Fri, 15 Jul 2016 10:35:54 -0000 On Thursday 14 July 2016 10:21 PM, Jan Viktorin wrote: > On Tue, 12 Jul 2016 11:31:21 +0530 > Shreyansh Jain wrote: > >> Remove bus logic from ethdev hotplug by using eal for this. >> >> Current api is preserved: >> - the last port that has been created is tracked to return it to the >> application when attaching, >> - the internal device name is reused when detaching. >> >> We can not get rid of ethdev hotplug yet since we still need some mechanism >> to inform applications of port creation/removal to substitute for ethdev >> hotplug api. >> >> dev_type field in struct rte_eth_dev and rte_eth_dev_allocate are kept as >> is, but this information is not needed anymore and is removed in the following >> commit. >> >> Signed-off-by: David Marchand >> Signed-off-by: Shreyansh Jain >> --- >> lib/librte_ether/rte_ethdev.c | 207 +++++++----------------------------------- >> 1 file changed, 33 insertions(+), 174 deletions(-) >> >> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c >> index a667012..8d14fd7 100644 >> --- a/lib/librte_ether/rte_ethdev.c >> +++ b/lib/librte_ether/rte_ethdev.c >> @@ -72,6 +72,7 @@ >> static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data"; >> struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS]; >> static struct rte_eth_dev_data *rte_eth_dev_data; >> +static uint8_t eth_dev_last_created_port; >> static uint8_t nb_ports; >> > > [...] > >> - >> /* attach the new device, then store port_id of the device */ >> int >> rte_eth_dev_attach(const char *devargs, uint8_t *port_id) >> { >> - struct rte_pci_addr addr; >> int ret = -1; >> + int current = eth_dev_last_created_port; >> + char *name = NULL; >> + char *args = NULL; >> >> if ((devargs == NULL) || (port_id == NULL)) { >> ret = -EINVAL; >> goto err; >> } >> >> - if (eal_parse_pci_DomBDF(devargs, &addr) == 0) { >> - ret = rte_eth_dev_attach_pdev(&addr, port_id); >> - if (ret < 0) >> - goto err; >> - } else { >> - ret = rte_eth_dev_attach_vdev(devargs, port_id); >> - if (ret < 0) >> - goto err; >> + /* parse devargs, then retrieve device name and args */ >> + if (rte_eal_parse_devargs_str(devargs, &name, &args)) >> + goto err; >> + >> + ret = rte_eal_dev_attach(name, args); >> + if (ret < 0) >> + goto err; >> + >> + /* no point looking at eth_dev_last_created_port if no port exists */ > > I am not sure about this comment. What is "no point"? > > Isn't this also a potential bug? (Like the one below.) How could it > happen there is no port after a successful attach? Yes, even searching through code path I couldn't find a positive case where control would reach here without nb_ports>0. Though, i am not sure if some rough application attempts to mix-up calls - and that, in my opinion, is not worth checking. Should I remove it? > >> + if (!nb_ports) { >> + ret = -1; >> + goto err; >> + } >> + /* if nothing happened, there is a bug here, since some driver told us >> + * it did attach a device, but did not create a port */ >> + if (current == eth_dev_last_created_port) { >> + ret = -1; >> + goto err; > > Should we log this? Or call some kind panic? I will place a log because applications should have a chance to ignore a device which cannot be attached for whatever reason. > >> } >> + *port_id = eth_dev_last_created_port; >> + ret = 0; >> >> - return 0; >> err: >> - RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n"); >> + free(name); >> + free(args); >> return ret; >> } >> >> @@ -590,7 +464,6 @@ err: >> int >> rte_eth_dev_detach(uint8_t port_id, char *name) >> { >> - struct rte_pci_addr addr; >> int ret = -1; >> >> if (name == NULL) { >> @@ -598,33 +471,19 @@ rte_eth_dev_detach(uint8_t port_id, char *name) >> goto err; >> } >> >> - /* check whether the driver supports detach feature, or not */ >> + /* FIXME: move this to eal, once device flags are relocated there */ >> if (rte_eth_dev_is_detachable(port_id)) >> goto err; >> >> - if (rte_eth_dev_get_device_type(port_id) == RTE_ETH_DEV_PCI) { >> - ret = rte_eth_dev_get_addr_by_port(port_id, &addr); >> - if (ret < 0) >> - goto err; >> - >> - ret = rte_eth_dev_detach_pdev(port_id, &addr); >> - if (ret < 0) >> - goto err; >> - >> - snprintf(name, RTE_ETH_NAME_MAX_LEN, >> - "%04x:%02x:%02x.%d", >> - addr.domain, addr.bus, >> - addr.devid, addr.function); >> - } else { >> - ret = rte_eth_dev_detach_vdev(port_id, name); >> - if (ret < 0) >> - goto err; >> - } >> + snprintf(name, sizeof(rte_eth_devices[port_id].data->name), >> + "%s", rte_eth_devices[port_id].data->name); >> + ret = rte_eal_dev_detach(name); >> + if (ret < 0) >> + goto err; >> >> return 0; >> >> err: >> - RTE_LOG(ERR, EAL, "Driver, cannot detach the device\n"); > > I'd be more specific about the failing device, we have its name. Agree. I will add 'name' to this. >> return ret; >> } >> >