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 8DF9DA0545; Sat, 28 May 2022 10:53:35 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7FAEB410FA; Sat, 28 May 2022 10:53:35 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 806C240E50; Sat, 28 May 2022 10:53:33 +0200 (CEST) Received: from kwepemi100013.china.huawei.com (unknown [172.30.72.53]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4L9FkB6tx3zRhTf; Sat, 28 May 2022 16:50:26 +0800 (CST) Received: from kwepemm600004.china.huawei.com (7.193.23.242) by kwepemi100013.china.huawei.com (7.221.188.136) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Sat, 28 May 2022 16:53:29 +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, 28 May 2022 16:53:29 +0800 Content-Type: multipart/alternative; boundary="------------dLl6ZHiTIm5M0psLQCep2pnE" Message-ID: <3a28b653-b83a-6cb5-bf5a-a04c144ba497@huawei.com> Date: Sat, 28 May 2022 16:53:28 +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 , "Min Hu (Connor)" , Ferruh Yigit , Andrew Rybchenko , David Marchand CC: dev , dpdk stable , Bruce Richardson References: <20220521065549.33451-1-humin29@huawei.com> <2199956.o7ts2hSHzF@thomas> From: "lihuisong (C)" In-Reply-To: <2199956.o7ts2hSHzF@thomas> X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) 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 --------------dLl6ZHiTIm5M0psLQCep2pnE Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit 在 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;     else         return 1; } 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*)         port_id++;     if (port_id >= RTE_MAX_ETHPORTS)         return RTE_MAX_ETHPORTS;     return port_id; } > > . --------------dLl6ZHiTIm5M0psLQCep2pnE Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: 8bit


在 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) <humin29@huawei.com> wrote:

From: Huisong Li <lihuisong@huawei.com>

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;
    else
        return 1;
}

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)
        port_id++;

    if (port_id >= RTE_MAX_ETHPORTS)
        return RTE_MAX_ETHPORTS;

    return port_id;
}

.
--------------dLl6ZHiTIm5M0psLQCep2pnE--