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 80A2442643; Tue, 26 Sep 2023 15:59:30 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 13552402AA; Tue, 26 Sep 2023 15:59:30 +0200 (CEST) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id 1916D4027D for ; Tue, 26 Sep 2023 15:59:28 +0200 (CEST) Received: from kwepemi500020.china.huawei.com (unknown [172.30.72.53]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4Rw1V84x2GzMlj7; Tue, 26 Sep 2023 21:55:44 +0800 (CST) Received: from [10.67.121.175] (10.67.121.175) by kwepemi500020.china.huawei.com (7.221.188.8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Tue, 26 Sep 2023 21:59:26 +0800 Message-ID: Date: Tue, 26 Sep 2023 21:59:25 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH 00/36] fix Rx and Tx queue state To: David Marchand CC: Ferruh Yigit , , , Thomas Monjalon References: <20230908112901.1169869-1-haijie1@huawei.com> <6c030892-5e27-4537-9262-c733914d95df@amd.com> From: Jie Hai In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.175] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To kwepemi500020.china.huawei.com (7.221.188.8) 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 On 2023/9/22 14:41, David Marchand wrote: > Hello, > > On Fri, Sep 22, 2023 at 4:41 AM Jie Hai wrote: >> On 2023/9/19 0:54, Ferruh Yigit wrote: >>> On 9/8/2023 12:50 PM, David Marchand wrote: >>>> On Fri, Sep 8, 2023 at 1:32 PM Jie Hai wrote: >>>>> >>>>> The DPDK framework reports the queue state, which is stored in >>>>> dev->data->tx_queue_state and dev->data->rx_queue_state. The >>>>> state is maintained by the driver. Users may determine whether >>>>> a queue participates in packet forwarding based on the state, >>>>> for example, >>>> >>>> The driver is maintaining this state in dev_start / dev_stop and per >>>> queue start/stop handlers. >>>> >>>>> >>>>> [1] 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding" >>>>> [2] 141a520b35f7 ("app/testpmd: fix primary process not polling all queues") >>>>> >>>>> Therefore, the drivers need to modify the queue state in time >>>>> according to the actual situation, especially when dev_start >>>>> and dev_stop are called. see [3] for more information. >>>>> >>>>> [3] https://inbox.dpdk.org/dev/20230721160422.3848154-1-ferruh.yigit@amd.com/ >>>>> >>>>> This patchset also resubmit the patch [2] and makes some fixes on the patch. >>>> >>>> I just had a quick look at some patches and I wonder if a better fix >>>> would be at the ethdev level, rather than fixing all drivers. >>>> >>>> >>> >>> I came here to make the same comment, >>> >>> Jie, I forgot if we discuss this already but, >>> >>> does it work if we update queue state in 'rte_eth_dev_start()' & >>> 'rte_eth_dev_stop()' when 'dev_start' & 'dev_stop' dev_ops succeeds? >>> >>> This solves the case driver forgets to update the queue state. >>> >>> >> Hi, Furrh and David, >> >> It's OK for dev_stop, but not enough for dev_start. >> Some drivers also support deferred_start. >> Therefore, not all queues are in the start state after dev_start. >> >> If we want to get that correct queue state at the framework level, I >> offer the following options: >> >> step 1. A capability(e.g. RTE_ETH_DEV_CAPA_DEFERRED_START) is added to >> the framework, indicating whether the driver supports deferred_start. >> The capability should be reported by the driver and user can get it by >> rte_eth_dev_info_get(). >> step 2. All drivers that support deferred_start should report configuration >> information about deferred_start through >> rte_eth_rx_queue_info_get |rte_eth_tx_queue_info_get. >> step 3.The framework updates the queue status based on the support and >> configuration of deferred_start. > > rte_eth_dev_start must only update the queue state if > rx_deferred_start is unset (see struct > rte_eth_rxconf::rx_deferred_start). > And the queue state needs to be updated in ethdev > rte_eth_dev_rx_queue_start/stop. > > So I don't see where we need a new capability. > Hi, David, Thanks and you are right. The driver reports whether the queue is configured with deferred_start through rxq_info_get & rxq_info_get ops. The framework can obtain queue configuration through rte_eth_rx_queue_info_get & rte_eth_tx_queue_info_get. According to your suggestion, the prerequisite is that all drivers that support deferred_start support rxq_info_get & rxq_info_get ops reporting it. Currently, not all drivers support reporting the deferred_start configuration. Therefore, we need to modify some driver. I'll send V2 after finishing. Best, regards Jie Hai >