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 2238646071; Mon, 13 Jan 2025 12:23:31 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0547B402A7; Mon, 13 Jan 2025 12:23:31 +0100 (CET) Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by mails.dpdk.org (Postfix) with ESMTP id 2314040261 for ; Mon, 13 Jan 2025 12:23:28 +0100 (CET) Received: from mail.maildlp.com (unknown [172.19.163.252]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4YWqXp4jplz1W45j; Mon, 13 Jan 2025 19:19:38 +0800 (CST) Received: from dggemv704-chm.china.huawei.com (unknown [10.3.19.47]) by mail.maildlp.com (Postfix) with ESMTPS id EAE64180214; Mon, 13 Jan 2025 19:23:26 +0800 (CST) Received: from kwepemn100009.china.huawei.com (7.202.194.112) by dggemv704-chm.china.huawei.com (10.3.19.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Mon, 13 Jan 2025 19:23:26 +0800 Received: from [10.67.121.59] (10.67.121.59) by kwepemn100009.china.huawei.com (7.202.194.112) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Mon, 13 Jan 2025 19:23:25 +0800 Message-ID: <870781f8-951e-7575-0ee0-7b1ceb7833c0@huawei.com> Date: Mon, 13 Jan 2025 19:23:25 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH v1 2/2] ethdev: fix skip valid port in probing callback To: Thomas Monjalon CC: , , , "Ajit Khaparde" , Somnath Kotur , Praveen Shetty , Andrew Boyer , Dariusz Sosnowski , Viacheslav Ovsiienko , "Bing Zhao" , Ori Kam , Suanming Mou , Matan Azrad , Chaoyong He , Andrew Rybchenko , References: <20250113025521.32703-1-lihuisong@huawei.com> <1988729.PYKUYFuaPT@thomas> <0293d4d1-0df7-63e1-3dba-244729a78bb0@huawei.com> <9389517.CDJkKcVGEf@thomas> From: "lihuisong (C)" In-Reply-To: <9389517.CDJkKcVGEf@thomas> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.59] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To kwepemn100009.china.huawei.com (7.202.194.112) 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 在 2025/1/13 18:57, Thomas Monjalon 写道: > 13/01/2025 10:35, lihuisong (C): >> 在 2025/1/13 16:16, Thomas Monjalon 写道: >>> 13/01/2025 03:55, Huisong Li: >>>> The event callback in application may use the macro RTE_ETH_FOREACH_DEV to >>>> iterate over all enabled ports to do something(like, verifying the port id >>>> validity) when receive a probing event. If the ethdev state of a port is >>>> not RTE_ETH_DEV_UNUSED, this port will be considered as a valid port. >>>> >>>> However, this state is set to RTE_ETH_DEV_ATTACHED after pushing 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 the ethdev >>>> state to RTE_ETH_DEV_ALLOCATED before pushing probing event and set it to >>>> RTE_ETH_DEV_ATTACHED after definitely probed. And this port is valid if its >>>> device state is 'ALLOCATED' or 'ATTACHED'. >>> 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 issue is >> application want to check if the port id is valid. >> However, application just receive the new event from the device and the >> 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 need >> to check, right? > 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. If it's ok to you, I will add some comments for RTE_ETH_EVENT_NEW. > > We currently have this: > RTE_ETH_EVENT_NEW, /**< port is probed */ > > >> If so I think this series can be dropped. >>> It is the same as changing the state to RTE_ETH_DEV_ATTACHED >>> before calling the event callback. >>> >>> So this is a NACK. >>> >>> Why do you need drivers to check the state of a notified device? >>> If it is RTE_ETH_EVENT_NEW, you know that's a new device, >>> there is nothing else to check. >> It just modified the verification about RTE_ETH_DEV_UNUSED in the device >> driver. >> Driver not need to know the event. > Sorry I was not clear. > I said "drivers", but it should be "apps & drivers" because they can both > register to the event RTE_ETH_EVENT_NEW. > In some situations, it is convenient for a driver to listen to new ports > (it was done for failsafe driver). Yes. but it doesn't matter now.😁 > > > .