From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id AAE6DA0543; Tue, 7 Jun 2022 08:44:10 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8E1C74021D; Tue, 7 Jun 2022 08:44:10 +0200 (CEST) Received: from wout3-smtp.messagingengine.com (wout3-smtp.messagingengine.com [64.147.123.19]) by mails.dpdk.org (Postfix) with ESMTP id 0E96040156; Tue, 7 Jun 2022 08:44:09 +0200 (CEST) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.west.internal (Postfix) with ESMTP id EF1F9320070D; Tue, 7 Jun 2022 02:44:06 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Tue, 07 Jun 2022 02:44:07 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm2; t=1654584246; x= 1654670646; bh=HRqkhsy/cUj6Nri8xju5BCPsG8DP0QW1Y2dGoh6pMio=; b=f rd+Pdvyo+BJKc3PK3z9S2LT9Telh1hJKiANF8QWoC+fuDpb6jgksr5K0kcyjDTnJ Wh/xGU9Dys19VHUeGMosjdE+YDO830ozuoTtzNa7DWU4vCyV8NNUfXNIf1bGK+ES ck7d55MLriDw5E+haMHdPcgp6iBe/9AnB66oI7+a7ErYqOZHtr99n/Gfr22Y3yRr 5n1DbGWebmjJUaadxjF41tgOXoMaxcqDDZKUJ3Y9sYMtmGRhTQgyiN7OVJPaszGt W/fI4W2c6wxGVpvXbH/wUJdP15phDBVXPbOsRotLkBlHKDRQvp1IwlnmT7xdoV5B 8hRqx24HEbMu2sfqjxHUw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1654584246; x= 1654670646; bh=HRqkhsy/cUj6Nri8xju5BCPsG8DP0QW1Y2dGoh6pMio=; b=O Cap9X/QZXFKEo8oQl++EYJrwa0rJH3sUu0w56Tfn253vkZAwgxjl+8zVtgiVkDU+ gdeBhZV9EXaxXf0DvQYZyliVU1d1afXhZwftJkWs2+4i0QV4BSizJ2q9mwYn2baE QsuhdOYg/5oRMBwqdyh1LgQw2HJqRpm55ovOG3vCZtP4DJ/QmnxUGAYjOig2J1oz rg9aTdHA05M+H5s1++SDbkJ4Iowm14+bjCMizNPJDN8Y54H/PA600/UDkMCi/SEP gb3nhyBeh/knKQDCXSeo/JZvctE0kRB5grTS+zLKoQ0Uf72k0xKhuLp643Jh6lZs NqSi+WUNXz0fsj5ASzhEg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedruddtgedguddufecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttdejnecuhfhrohhmpefvhhho mhgrshcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqne cuggftrfgrthhtvghrnhepgedttdeljeejgeffkeekkedtjeevtdehvedtkeeivdeuuedv ieduvdelveejueejnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilh hfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 7 Jun 2022 02:44:04 -0400 (EDT) From: Thomas Monjalon To: Ferruh Yigit , "Min Hu (Connor)" , Ferruh Yigit , Andrew Rybchenko , David Marchand , "lihuisong (C)" Cc: dev , dpdk stable , Bruce Richardson Subject: Re: [PATCH] ethdev: fix push new event Date: Tue, 07 Jun 2022 08:44:00 +0200 Message-ID: <6157663.iZASKD2KPV@thomas> In-Reply-To: <4c646ebf-0d2d-fe29-6e17-898540a7f3fb@huawei.com> References: <20220521065549.33451-1-humin29@huawei.com> <2511545.Lt9SDvczpP@thomas> <4c646ebf-0d2d-fe29-6e17-898540a7f3fb@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 07/06/2022 03:23, lihuisong (C): >=20 > =E5=9C=A8 2022/6/3 15:42, Thomas Monjalon =E5=86=99=E9=81=93: > > 02/06/2022 13:24, lihuisong (C): > >> =E5=9C=A8 2022/5/30 19:10, Ferruh Yigit =E5=86=99=E9=81=93: > >>> On 5/30/2022 9:28 AM, Thomas Monjalon wrote: > >>>> [CAUTION: External Email] > >>>> > >>>> 28/05/2022 10:53, lihuisong (C): > >>>>> =E5=9C=A8 2022/5/23 22:36, Thomas Monjalon =E5=86=99=E9=81=93: > >>>>>> 23/05/2022 11:51, David Marchand: > >>>>>>> On Sat, May 21, 2022 at 8:57 AM Min Hu > >>>>>>> (Connor) wrote: > >>>>>>>> From: Huisong Li > >>>>>>>> > >>>>>>>> The 'state' in struct rte_eth_dev may be used to update some > >>>>>>>> information > >>>>>>>> when app receive these events. For example, when app receives a > >>>>>>>> new event, > >>>>>>>> app may get the socket id of this port by calling > >>>>>>>> rte_eth_dev_socket_id to > >>>>>>>> setup the attached port. The 'state' is used in > >>>>>>>> rte_eth_dev_socket_id. > >>>>>>>> > >>>>>>>> If the state isn't modified to RTE_ETH_DEV_ATTACHED before > >>>>>>>> pushing the new > >>>>>>>> event, app will get the socket id failed. So this patch moves > >>>>>>>> pushing event > >>>>>>>> operation after the state updated. > >>>>>>>> > >>>>>>>> Fixes: 99a2dd955fba ("lib: remove librte_ prefix from directory > >>>>>>>> names") > >>>>>>> A patch moving code is unlikely to be at fault. > >>>>>>> > >>>>>>> > >>>>>>> Looking at the patch which moved those notifications in this poin= t of > >>>>>>> the code, the state update was pushed after the notification on > >>>>>>> purpose. > >>>>>>> See be8cd210379a ("ethdev: fix port probing notification") > >>>>>>> > >>>>>>> ethdev: fix port probing notification > >>>>>>> > >>>>>>> The new device was notified as soon as it was allocated. > >>>>>>> It leads to use a device which is not yet initialized. > >>>>>>> > >>>>>>> The notification must be published after the initialization > >>>>>>> is done > >>>>>>> by the PMD, but before the state is changed, in order to l= et > >>>>>>> notified entities taking ownership before general availabi= lity. > >>>>>>> > >>>>>>> > >>>>>>> Do we need an intermediate state during probing? > >>>>>> Possibly. Currently we have only 3 states: > >>>>>> RTE_ETH_DEV_UNUSED > >>>>>> RTE_ETH_DEV_ATTACHED > >>>>>> RTE_ETH_DEV_REMOVED > >>>>>> > >>>>>> We may add RTE_ETH_DEV_ALLOCATED just before calling > >>>>>> rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL); > >>>>>> Then we would need to check against RTE_ETH_DEV_ALLOCATED > >>>>>> in some ethdev functions. > >>>>>> > >>>>> Hi, Thomas, > >>>>> > >>>>> Do you mean that we need to modify some funcions like following? > >>>>> > >>>>> int rte_eth_dev_is_valid_port(uint16_t port_id) > >>>>> { > >>>>> if (port_id >=3D RTE_MAX_ETHPORTS || > >>>>> (rte_eth_devices[port_id].state !=3D *RTE_ETH_DEV_ALLOCA= TED*)) > >>>>> return 0; > >>> Won't this mark ATTACHED devices as invalid? > >> Yes, You are right. > >> > >>> If the state flow will be as UNUSED -> ALLOCATED -> ATTACHED, above > >>> check should be against 'ATTACHED' I think. > > It should validate both ALLOCATED and ATTACHED. > Actually, we can only pick one, because it is an enumeration. You can check it is either one state or the other. > >> If these check is against 'ATTACHED', it goes back to the issue this > >> patch mentioned. > >> > >> The failsafe PMD applications expect sending event before device state > >> set to 'ATTACHED'. > >> But other applications expect the device with 'ATTACHED' state before > >> send event. > >> They are in conflict with each other. So we can't solve this issue by > >> adding an > >> 'RTE_ETH_DEV_ALLOCATED' state. > > > > > > . >=20