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 CA176A00C2; Tue, 27 Sep 2022 12:29:41 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B967841133; Tue, 27 Sep 2022 12:29:41 +0200 (CEST) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by mails.dpdk.org (Postfix) with ESMTP id 15BC5410D0; Tue, 27 Sep 2022 12:29:40 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id C51BD5C0084; Tue, 27 Sep 2022 06:29:37 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Tue, 27 Sep 2022 06:29:37 -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=1664274577; x= 1664360977; bh=4TVi2GaQbBRW23dabCkRSggHscz3AKf6lxmodczCW78=; b=e KsrMYWEOKu54goyITWru/WntXCUJjiMDisofEBIpmAfQsTkqNQE4TtiVFFW+REDQ t6/1hrcaSlE+sYfpfLyQwmJ0K/+dHlkjSCXcIT1OKCAwmPwo891mgOH8Zh52OO6H I/Q/rs3lY2QLFNbiNmSO41xf0X3rQS/SUxmNEf8qXopKLOmIQSYz3onyswLjhBge JxWFNFKrugSJG3rMKj5W0kXFD9E6G//K9keypR7F0FP+cpAF++AKoBwJ53R6fZ86 3iOVWap287gEwkR8iU5c0n7eiKf+tkoZmfyM8NfUktndz+pYTNvX1+ew79qbyXFN lXU8K66y1vjJXt4/UQ+IQ== 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=1664274577; x= 1664360977; bh=4TVi2GaQbBRW23dabCkRSggHscz3AKf6lxmodczCW78=; b=E XeaviO/2iArPF40F6ITxsKbi8CpRRtEZ9lcqLWEqcJ3xYwWJ+xLYMol7YNP9qdTC IdY5YOP6lxoJr1gS+92Y2Hk7NKcvvaEB86Pw1F2jfZk3A74gvhf2UGc1Ws4BAcdY xlcr03sKA0Xde1oNIXIgnMaSpnrMUEEyzpvIh5jYOQN3Ox2v8FflYs6Kap4vw0Mu e01OhHuBMsMW4s5BI9+V2BmO3qw6YVCnS+mCvwO6oOvFE6zsghyboWPp8n+TfPQb WxIH/bWUTW0Zj++7HpcS1AVYv6tiQ9VS8faxLwXs9OAtFS3zT4EH73EN1GpS/Iac zSWHOqX4RYLELCFqSDSng== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeegiedgtdegucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvvefufffkjghfggfgtgesthhqredttddtjeenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpeegtddtleejjeegffekkeektdejvedtheevtdekiedvueeuvdei uddvleevjeeujeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 27 Sep 2022 06:29:35 -0400 (EDT) From: Thomas Monjalon To: Ferruh Yigit , "Min Hu (Connor)" , Ferruh Yigit , Andrew Rybchenko , David Marchand , dev@dpdk.org, "lihuisong (C)" Cc: dev , dpdk stable , Bruce Richardson Subject: Re: [PATCH] ethdev: fix push new event Date: Tue, 27 Sep 2022 12:29:34 +0200 Message-ID: <2870765.o0KrE1Onz3@thomas> In-Reply-To: References: <20220521065549.33451-1-humin29@huawei.com> <6157663.iZASKD2KPV@thomas> 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 11/06/2022 10:59, lihuisong (C): > =E5=9C=A8 2022/6/7 14:44, Thomas Monjalon =E5=86=99=E9=81=93: > > 07/06/2022 03:23, lihuisong (C): > >> =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 po= int 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 initializa= tion > >>>>>>>>> is done > >>>>>>>>> by the PMD, but before the state is changed, in order t= o let > >>>>>>>>> notified entities taking ownership before general avail= ability. > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> 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_ALL= OCATED*)) > >>>>>>> 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. > uint16_t > rte_eth_find_next(uint16_t port_id) > { > while (port_id < RTE_MAX_ETHPORTS && > !(rte_eth_devices[port_id].state =3D=3D RTE_ETH_DEV_ALLOCATED= || > rte_eth_devices[port_id].state =3D=3D RTE_ETH_DEV_ATTACHED)) > port_id++; >=20 > if (port_id >=3D RTE_MAX_ETHPORTS) > return RTE_MAX_ETHPORTS; >=20 > return port_id; > } > like this, right? If so, adding 'ALLOCATED' and setting to 'ALLOCATED'=20 > is the same with > setting to 'ATTACHED' before sending new event. > They both meet the requirements mentioned in this patch that the device=20 > is a valid port > when applications receive a new event. Yes, when receiving the event, the port would valid in state ALLOCATED. Then we can set as ATTACHED when definitely initialized, after the notifications. > However, if device is taken by failsafe PMD as sub-device, the=20 > processing above > still doesn't satisfy the purpose of failsafe PMD when this sub-device=20 > push new event. I don't understand why you think failsafe is not satisfied. >=20 > I don't know if I'm missing something. Can you explain it, Ferruh and=20 > Thomas? Please explain what you think is failing with failsafe.