From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id AB07BA0542;
	Sat,  8 Oct 2022 06:09:06 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 5191740146;
	Sat,  8 Oct 2022 06:09:06 +0200 (CEST)
Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187])
 by mails.dpdk.org (Postfix) with ESMTP id EE5CB40042
 for <dev@dpdk.org>; Sat,  8 Oct 2022 06:09:03 +0200 (CEST)
Received: from dggemv703-chm.china.huawei.com (unknown [172.30.72.56])
 by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Mks6W00WXzpVC7;
 Sat,  8 Oct 2022 12:05:54 +0800 (CST)
Received: from kwepemm600004.china.huawei.com (7.193.23.242) by
 dggemv703-chm.china.huawei.com (10.3.19.46) 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:09:01 +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:09:00 +0800
Message-ID: <53083e8e-b680-17d4-ce93-7c26bed14a57@huawei.com>
Date: Sat, 8 Oct 2022 12:09:00 +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 V2 3/6] ethdev: fix push new event
To: Thomas Monjalon <thomas@monjalon.net>
CC: <dev@dpdk.org>, <ferruh.yigit@xilinx.com>,
 <andrew.rybchenko@oktetlabs.ru>, <huangdaode@huawei.com>
References: <20220825024425.10534-1-lihuisong@huawei.com>
 <20220915124522.5407-1-lihuisong@huawei.com>
 <20220915124522.5407-4-lihuisong@huawei.com> <1710029.vCJZsxu672@thomas>
From: "lihuisong (C)" <lihuisong@huawei.com>
In-Reply-To: <1710029.vCJZsxu672@thomas>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
X-Originating-IP: [10.67.103.231]
X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) 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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org


在 2022/9/27 18:49, Thomas Monjalon 写道:
> 15/09/2022 14:45, 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")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   lib/ethdev/ethdev_driver.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
>> index a285f213f0..a6616f072b 100644
>> --- a/lib/ethdev/ethdev_driver.c
>> +++ b/lib/ethdev/ethdev_driver.c
>> @@ -206,9 +206,9 @@ rte_eth_dev_probing_finish(struct rte_eth_dev *dev)
>>   	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
>>   		eth_dev_fp_ops_setup(rte_eth_fp_ops + dev->data->port_id, dev);
>>   
>> -	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
>>   
>>   	dev->state = RTE_ETH_DEV_ATTACHED;
>> +	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
>>   }
> As explained in the first patch, I don't think it is a good solution.
> We should not allow the port to be used until the end of probing.
> When RTE_ETH_EVENT_NEW is sent, the device is allocated but
> not ready for use. If an entity like failsafe decides to take ownership
> of the port, then the application should not consider it at all.
> For these reasons, we should limit which operations can be done
> during RTE_ETH_EVENT_NEW processing.
> That's why I've proposed creating a new state RTE_ETH_DEV_ALLOCATED,
> not sure why you didn't follow this advice.
I want to put this problem to this case this patchset mentions, so as to
have a better discussion. From the first patch, I know what you mean.
But I still have some confusion:
When a device taken by failsafe PMD push new event, all event callback
will be called under this device. As you suggested, the device is a valid
port if its state is ALLOCATED or ATTACHED. Now, the macro 
RTE_ETH_FOREACH_DEV
uses the condition that device is valid(state is ALLOCATED or ATTACHED)
and NOT owned.

Even if we add the new ALLOCATED state, this device taken by failsafe is
also a valid port in new event callback in application, and is in 'NO 
OWNER'.
if event callback of application is called before one of failsafe PMD.
As a result, application still can operate this device directly.
Can we make sure that event callback of failsafe PMD is before one of 
application?
>
>
>
>
> .