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 4B22BA0544; Tue, 7 Jun 2022 03:23:47 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EFB3240156; Tue, 7 Jun 2022 03:23:46 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 430E4400EF; Tue, 7 Jun 2022 03:23:45 +0200 (CEST) Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.55]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4LHCGR58n3zRhjS; Tue, 7 Jun 2022 09:20:31 +0800 (CST) Received: from kwepemm600004.china.huawei.com (7.193.23.242) by dggemv704-chm.china.huawei.com (10.3.19.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 7 Jun 2022 09:23:42 +0800 Received: from [10.67.103.231] (10.67.103.231) by kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 7 Jun 2022 09:23:42 +0800 Message-ID: <4c646ebf-0d2d-fe29-6e17-898540a7f3fb@huawei.com> Date: Tue, 7 Jun 2022 09:23:41 +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] ethdev: fix push new event To: Thomas Monjalon , Ferruh Yigit , "Min Hu (Connor)" , "Ferruh Yigit" , Andrew Rybchenko , David Marchand CC: dev , dpdk stable , Bruce Richardson References: <20220521065549.33451-1-humin29@huawei.com> <11bac61b-87c4-ce18-2744-ae5bb4d45253@xilinx.com> <2511545.Lt9SDvczpP@thomas> From: "lihuisong (C)" In-Reply-To: <2511545.Lt9SDvczpP@thomas> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemm600004.china.huawei.com (7.193.23.242) X-CFilter-Loop: Reflected 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 在 2022/6/3 15:42, Thomas Monjalon 写道: > 02/06/2022 13:24, lihuisong (C): >> 在 2022/5/30 19:10, Ferruh Yigit 写道: >>> On 5/30/2022 9:28 AM, Thomas Monjalon wrote: >>>> [CAUTION: External Email] >>>> >>>> 28/05/2022 10:53, lihuisong (C): >>>>> 在 2022/5/23 22:36, Thomas Monjalon 写道: >>>>>> 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 point 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 let >>>>>>> notified entities taking ownership before general availability. >>>>>>> >>>>>>> >>>>>>> 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 >= RTE_MAX_ETHPORTS || >>>>> (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*)) >>>>> 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. > > >> 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. > > > .