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 5275446066; Mon, 13 Jan 2025 13:30:55 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DEC9540BA2; Mon, 13 Jan 2025 13:30:54 +0100 (CET) Received: from fhigh-b2-smtp.messagingengine.com (fhigh-b2-smtp.messagingengine.com [202.12.124.153]) by mails.dpdk.org (Postfix) with ESMTP id EFD9A40A80 for ; Mon, 13 Jan 2025 13:30:52 +0100 (CET) Received: from phl-compute-10.internal (phl-compute-10.phl.internal [10.202.2.50]) by mailfhigh.stl.internal (Postfix) with ESMTP id D428E25401C3; Mon, 13 Jan 2025 07:30:51 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-10.internal (MEProxy); Mon, 13 Jan 2025 07:30:52 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1736771451; x=1736857851; bh=lhOhXGZI3zNA/yQC6JaoNnqGoDZI/xCoLj+U3pJcHIE=; b= oiT5AI8qSspQmSudVrWTXWFiUHfEl7KC8EiGbUaeYKWhPfzLDhsJzIRFhDAY+8Sd GmLYnyMgjcHvRMylT+/7TSLbnpGr5HdudnpaBRWTUEKVaXhC7NMkSXC3m2pUeZFM Gh4q1/xM0SPYNfNwz5ai6OKgiMP08kdgYQA9ewBS0yv+Udzg79QYYfm3Iku7ahp8 +GPNqXuuDqHi0/xQ1p+ALk1cRwfH1lmdjsEx7pInqaiLWrgK0cCT3G1R7EFXjyTc octZLJNnhaNMqrhrmSr64PsP9Pz3L/h7J25zmXYvlTscJWxoZtRWLtE5L/7Wl3Dq xrhbNOqe4KdLRAQ5DfDWAw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1736771451; x= 1736857851; bh=lhOhXGZI3zNA/yQC6JaoNnqGoDZI/xCoLj+U3pJcHIE=; b=r WOaR2etXz7IbZQLFDzxt5IguLplKFzTUP4kpoDNjGbE7qaCHeetTSlHPUhuv9eiF Nyt+YaPpikR2rsM+BBSrZOFlQStd42SiSgvgmWUjsLsBV2GlGprTHd28qGwBHXar xkvqVozJdTZpc7PtcOeOUfEbmLBXJFqoFI2Fhd5tOg4sEpVZynr9ixXHLE44mt96 a6aAsc2mVatfcKzCdBPFDLlWglNjW2iq/yoLwSeUNjcFyVnpd3V7112e6mbJP5vi uHepXsx/C8iMeKeWAuxd9v8KswPjXuLM/Lak6Zjwz7abEO3PEsSKUrK1RLDOU3G3 sIrqIElTYduE/mvNwHJ8Q== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefuddrudehgedggedtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhephffvvefufffkjghfggfgtgesthhqredttddtjeen ucfhrhhomhepvfhhohhmrghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrg hlohhnrdhnvghtqeenucggtffrrghtthgvrhhnpeegtddtleejjeegffekkeektdejvedt heevtdekiedvueeuvdeiuddvleevjeeujeenucevlhhushhtvghrufhiiigvpedtnecurf grrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtpdhn sggprhgtphhtthhopedujedpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoheplhhihh huihhsohhngheshhhurgifvghirdgtohhmpdhrtghpthhtohepuggvvhesughpughkrdho rhhgpdhrtghpthhtohepshhtvghphhgvnhesnhgvthifohhrkhhplhhumhgsvghrrdhorh hgpdhrtghpthhtohepfhgvrhhruhhhrdihihhgihhtsegrmhgurdgtohhmpdhrtghpthht oheprghjihhtrdhkhhgrphgrrhguvgessghrohgruggtohhmrdgtohhmpdhrtghpthhtoh epshhomhhnrghthhdrkhhothhurhessghrohgruggtohhmrdgtohhmpdhrtghpthhtohep phhrrghvvggvnhdrshhhvghtthihsehinhhtvghlrdgtohhmpdhrtghpthhtoheprghnug hrvgifrdgsohihvghrsegrmhgurdgtohhmpdhrtghpthhtohepughsohhsnhhofihskhhi sehnvhhiughirgdrtghomh X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 13 Jan 2025 07:30:48 -0500 (EST) From: Thomas Monjalon To: "lihuisong (C)" Cc: dev@dpdk.org, stephen@networkplumber.org, ferruh.yigit@amd.com, Ajit Khaparde , Somnath Kotur , Praveen Shetty , Andrew Boyer , Dariusz Sosnowski , Viacheslav Ovsiienko , Bing Zhao , Ori Kam , Suanming Mou , Matan Azrad , Chaoyong He , Andrew Rybchenko , fengchengwen@huawei.com Subject: Re: [PATCH v1 2/2] ethdev: fix skip valid port in probing callback Date: Mon, 13 Jan 2025 13:30:47 +0100 Message-ID: <8515179.NyiUUSuA9g@thomas> In-Reply-To: References: <20250113025521.32703-1-lihuisong@huawei.com> <870781f8-951e-7575-0ee0-7b1ceb7833c0@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 13/01/2025 13:05, lihuisong (C): > =E5=9C=A8 2025/1/13 19:23, lihuisong (C) =E5=86=99=E9=81=93: > > =E5=9C=A8 2025/1/13 18:57, Thomas Monjalon =E5=86=99=E9=81=93: > >> 13/01/2025 10:35, lihuisong (C): > >>> =E5=9C=A8 2025/1/13 16:16, Thomas Monjalon =E5=86=99=E9=81=93: > >>>> 13/01/2025 03:55, Huisong Li: > >>>>> The event callback in application may use the macro=20 > >>>>> RTE_ETH_FOREACH_DEV to > >>>>> iterate over all enabled ports to do something(like, verifying the= =20 > >>>>> port id > >>>>> validity) when receive a probing event. If the ethdev state of a=20 > >>>>> port is > >>>>> not RTE_ETH_DEV_UNUSED, this port will be considered as a valid por= t. > >>>>> > >>>>> However, this state is set to RTE_ETH_DEV_ATTACHED after pushing=20 > >>>>> probing > >>>>> event. It means that probing callback will skip this port. But this > >>>>> assignment can not move to front of probing notification. See > >>>>> commit be8cd210379a ("ethdev: fix port probing notification") > >>>>> > >>>>> So this patch has to add a new state, RTE_ETH_DEV_ALLOCATED. Set=20 > >>>>> the ethdev > >>>>> state to RTE_ETH_DEV_ALLOCATED before pushing probing event and=20 > >>>>> set it to > >>>>> RTE_ETH_DEV_ATTACHED after definitely probed. And this port is=20 > >>>>> valid if its > >>>>> device state is 'ALLOCATED' or 'ATTACHED'. > >>>>=20 > >>>> If you do that, changing the definition of eth_dev_find_free_port() > >>>> you allow the application using a port before probing is finished. > >>> Yes, it's not reasonable. > >>> > >>> Thinking your comment twice, I feel that the root cause of this=20 > >>> issue is > >>> application want to check if the port id is valid. > >>> However, application just receive the new event from the device and t= he > >>> port id of this device must be valid when report new event. > >>> So application can think the received new event is valid and don't ne= ed > >>> to check, right? > >>=20 > >> Yes > >> Do you think it should be highlighted in the API doc? > > Security detection is common and always good for application. > > So I think it's better to highlight that in doc. > > > Now I remember why I have to put this patch into the patchset [1] that=20 > testpmd support multiple process attach and detach port. > Becase patch 4/5 in this series depands on this patch. > The setup_attached_port() have to move to eth_event_callback() in=20 > testpmd to update something. > And the setup_attached_port() would indirectyly check if this port is=20 > valid by rte_eth_dev_is_valid_port(). > Their caller stack is as follows: > eth_event_callback > -->setup_attached_port > -->rte_eth_dev_socket_id > -->rte_eth_dev_is_valid_port >=20 > From the testpmd's modification, that is to say, it is possible for=20 > appllication to call some APIs like rte_eth_dev_socket_id() and=20 > indirectyly check if this port id is valid in event new callback. > So should we add this patch? I think there are many like these API in=20 > ethdev layer. I'm confused a bit now. Yes rte_eth_dev_is_valid_port() is used in many API functions, so that's a valid concern. I would say we should not call much of these functions in the "new port" event callback. But the case of rte_eth_dev_socket_id() is concerning. I suggest to update rte_eth_dev_socket_id() to make it work with a newly allocated port. I suppose we can use the function eth_dev_is_allocated().