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 F30D4A0542; Sat, 8 Oct 2022 06:06:17 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9630D40146; Sat, 8 Oct 2022 06:06:17 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 5737740042; Sat, 8 Oct 2022 06:06:16 +0200 (CEST) Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.54]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Mks3G2XpNzpVmd; Sat, 8 Oct 2022 12:03:06 +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.31; Sat, 8 Oct 2022 12:06:12 +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.31; Sat, 8 Oct 2022 12:06:12 +0800 Message-ID: <4d4304fb-8ebd-5ad4-acc3-3ba72846b6e5@huawei.com> Date: Sat, 8 Oct 2022 12:06:11 +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: dpdk stable , Bruce Richardson References: <20220521065549.33451-1-humin29@huawei.com> <6157663.iZASKD2KPV@thomas> <2870765.o0KrE1Onz3@thomas> From: "lihuisong (C)" In-Reply-To: <2870765.o0KrE1Onz3@thomas> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) 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/9/27 18:29, Thomas Monjalon 写道: > 11/06/2022 10:59, lihuisong (C): >> 在 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. > 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 >> processing above >> still doesn't satisfy the purpose of failsafe PMD when this sub-device >> push new event. > I don't understand why you think failsafe is not satisfied. >> I don't know if I'm missing something. Can you explain it, Ferruh and >> Thomas? > Please explain what you think is failing with failsafe. Please look the reply in new patchset. > > > > .