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 10E18A0553; Sat, 11 Jun 2022 11:00:00 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A384540222; Sat, 11 Jun 2022 10:59:59 +0200 (CEST) Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by mails.dpdk.org (Postfix) with ESMTP id 91C7540041; Sat, 11 Jun 2022 10:59:57 +0200 (CEST) Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.57]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4LKsDV1RQvz1K9LM; Sat, 11 Jun 2022 16:58:02 +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; Sat, 11 Jun 2022 16:59:54 +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; Sat, 11 Jun 2022 16:59:54 +0800 Message-ID: Date: Sat, 11 Jun 2022 16:59:36 +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> <2511545.Lt9SDvczpP@thomas> <4c646ebf-0d2d-fe29-6e17-898540a7f3fb@huawei.com> <6157663.iZASKD2KPV@thomas> From: "lihuisong (C)" In-Reply-To: <6157663.iZASKD2KPV@thomas> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) 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/7 14:44, Thomas Monjalon 写道: > 07/06/2022 03:23, lihuisong (C): >> 在 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. > 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 == RTE_ETH_DEV_ALLOCATED ||              rte_eth_devices[port_id].state == RTE_ETH_DEV_ATTACHED))         port_id++;     if (port_id >= RTE_MAX_ETHPORTS)         return RTE_MAX_ETHPORTS;     return port_id; } like this, right? If so, adding 'ALLOCATED' and setting to 'ALLOCATED' is the same with setting to 'ATTACHED' before sending new event. They both meet the requirements mentioned in this patch that the device is a valid port when applications receive a new event. However, if device is taken by failsafe PMD as sub-device, the processing above still doesn't satisfy the purpose of failsafe PMD when this sub-device push new event. I don't know if I'm missing something. Can you explain it, Ferruh and Thomas? >>>> 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. >>> >>> . > > > > > .