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 E7FCD46054;
Mon, 13 Jan 2025 13:06:12 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id D0F6B40B97;
Mon, 13 Jan 2025 13:06:12 +0100 (CET)
Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188])
by mails.dpdk.org (Postfix) with ESMTP id B0C92402A7
for <dev@dpdk.org>; Mon, 13 Jan 2025 13:06:10 +0100 (CET)
Received: from mail.maildlp.com (unknown [172.19.163.48])
by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4YWrV91R2pzbk2s;
Mon, 13 Jan 2025 20:02:25 +0800 (CST)
Received: from dggemv704-chm.china.huawei.com (unknown [10.3.19.47])
by mail.maildlp.com (Postfix) with ESMTPS id A791E180064;
Mon, 13 Jan 2025 20:05:32 +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 20:05:32 +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 20:05:31 +0800
Message-ID: <d4da83ca-5adc-0f5d-7368-40b77ca5b272@huawei.com>
Date: Mon, 13 Jan 2025 20:05:30 +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
From: "lihuisong (C)" <lihuisong@huawei.com>
To: Thomas Monjalon <thomas@monjalon.net>
CC: <dev@dpdk.org>, <stephen@networkplumber.org>, <ferruh.yigit@amd.com>,
"Ajit Khaparde" <ajit.khaparde@broadcom.com>, Somnath Kotur
<somnath.kotur@broadcom.com>, Praveen Shetty <praveen.shetty@intel.com>,
Andrew Boyer <andrew.boyer@amd.com>, Dariusz Sosnowski
<dsosnowski@nvidia.com>, Viacheslav Ovsiienko <viacheslavo@nvidia.com>, "Bing
Zhao" <bingz@nvidia.com>, Ori Kam <orika@nvidia.com>, Suanming Mou
<suanmingm@nvidia.com>, Matan Azrad <matan@nvidia.com>, Chaoyong He
<chaoyong.he@corigine.com>, Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
<fengchengwen@huawei.com>
References: <20250113025521.32703-1-lihuisong@huawei.com>
<1988729.PYKUYFuaPT@thomas> <0293d4d1-0df7-63e1-3dba-244729a78bb0@huawei.com>
<9389517.CDJkKcVGEf@thomas> <870781f8-951e-7575-0ee0-7b1ceb7833c0@huawei.com>
In-Reply-To: <870781f8-951e-7575-0ee0-7b1ceb7833c0@huawei.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
X-Originating-IP: [10.67.121.59]
X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) 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 <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
Hi Thomas,
在 2025/1/13 19:23, lihuisong (C) 写道:
>
> 在 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.
>
Now I remember why I have to put this patch into the patchset [1] that
testpmd support multiple process attach and detach port.
Becase patch 4/5 in this series depands on this patch.
The setup_attached_port() have to move to eth_event_callback() in
testpmd to update something.
And the setup_attached_port() would indirectyly check if this port is
valid by rte_eth_dev_is_valid_port().
Their caller stack is as follows:
eth_event_callback
-->setup_attached_port
-->rte_eth_dev_socket_id
-->rte_eth_dev_is_valid_port
From the testpmd's modification, that is to say, it is possible for
appllication to call some APIs like rte_eth_dev_socket_id() and
indirectyly check if this port id is valid in event new callback.
So should we add this patch? I think there are many like these API in
ethdev layer. I'm confused a bit now.
[1] https://mails.dpdk.org/archives/dev/2024-January/286026.html
/Huisong
>>
>> 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.😁
>>
>>
>> .