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 02EF942575; Tue, 12 Sep 2023 04:39:42 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id ADBB1402AC; Tue, 12 Sep 2023 04:39:41 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 32925402A3 for ; Tue, 12 Sep 2023 04:39:38 +0200 (CEST) Received: from kwepemi500020.china.huawei.com (unknown [172.30.72.55]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4Rl7495ZkzzNnPQ; Tue, 12 Sep 2023 10:35:53 +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, 12 Sep 2023 10:39:35 +0800 Message-ID: <30464b98-22ad-14b0-a897-eb8badb07f28@huawei.com> Date: Tue, 12 Sep 2023 10:39:34 +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 30/36] net/sfc: fix Rx and Tx queue state To: Andrew Rybchenko , , Thomas Monjalon , Lijun Ou , Chengwen Feng , Konstantin Ananyev CC: , Ferruh Yigit References: <20230908112901.1169869-1-haijie1@huawei.com> <20230908112901.1169869-31-haijie1@huawei.com> <7cf9adb7-4fdb-7d02-4634-0201ed54b50c@oktetlabs.ru> From: Jie Hai In-Reply-To: <7cf9adb7-4fdb-7d02-4634-0201ed54b50c@oktetlabs.ru> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.175] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) 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/8 20:01, Andrew Rybchenko wrote: > On 9/8/23 14:28, 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. >> Therefore, the driver needs to modify the queue state in time >> according to the actual situation. >> >> Fixes: 9ad9ff476cac ("ethdev: add queue state in queried queue >> information") >> Cc: stable@dpdk.org >> >> Signed-off-by: Jie Hai >> --- >>   drivers/net/sfc/sfc_repr.c | 12 ++++++++++++ >>   1 file changed, 12 insertions(+) >> >> diff --git a/drivers/net/sfc/sfc_repr.c b/drivers/net/sfc/sfc_repr.c >> index 6c7727d56980..278e37477530 100644 >> --- a/drivers/net/sfc/sfc_repr.c >> +++ b/drivers/net/sfc/sfc_repr.c >> @@ -263,6 +263,7 @@ static int >>   sfc_repr_dev_start(struct rte_eth_dev *dev) >>   { >>       struct sfc_repr *sr = sfc_repr_by_eth_dev(dev); >> +    uint16_t i; >>       int ret; >>       sfcr_info(sr, "entry"); >> @@ -274,6 +275,11 @@ sfc_repr_dev_start(struct rte_eth_dev *dev) >>       if (ret != 0) >>           goto fail_start; >> +    for (i = 0; i < dev->data->nb_rx_queues; i++) >> +        dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED; >> +    for (i = 0; i < dev->data->nb_tx_queues; i++) >> +        dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED; > > May be I miss something, but the driver does when queue is > really started in sfc_rx_qstart() and sfc_tx_qstart(). > Also the patch is wrong in general in the case of deferred > start queues. > > Same for stop. Hi, Andrew Rybchenko, Thanks for your review. There are two ops implementations, one in file sfc_ethdev.c(eg. sfc_dev_start) and the other in file sfc_repr.c (eg. sfc_repr_dev_start). This patch is for the second one, which does not support deferred start, please see 'sfc_repr_rx_qcheck_conf' and 'sfc_repr_tx_qcheck_conf'. The function process of ‘sfc_repr_dev_start’and ‘sfc_repr_dev_stop’ is as follows: sfc_repr_dev_start -->sfc_repr_start -->sfc_repr_proxy_start_repr -->sfc_repr_proxy_start -->sfc_repr_proxy_rxq_start -->sfc_rx_qstart -- RTE_ETH_QUEUE_STATE_STARTED -->sfc_repr_proxy_txq_start sfc_repr_dev_stop -->sfc_repr_stop -->sfc_repr_proxy_stop_repr -->sfc_repr_proxy_stop -->sfc_repr_proxy_rxq_stop -->sfc_rx_qstop -- RTE_ETH_QUEUE_STATE_STOPPED -->sfc_repr_proxy_txq_stop Since the Rx has been modified, Maybe I should modify the TX queue status separately in 'sfc_repr_proxy_txq_start/stop'. -- Best regards, Jie Hai > >> + >>       sfcr_info(sr, "done"); >>       return 0; >> @@ -338,6 +344,7 @@ static int >>   sfc_repr_dev_stop(struct rte_eth_dev *dev) >>   { >>       struct sfc_repr *sr = sfc_repr_by_eth_dev(dev); >> +    uint16_t i; >>       int ret; >>       sfcr_info(sr, "entry"); >> @@ -352,6 +359,11 @@ sfc_repr_dev_stop(struct rte_eth_dev *dev) >>       sfc_repr_unlock(sr); >> +    for (i = 0; i < dev->data->nb_rx_queues; i++) >> +        dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED; >> +    for (i = 0; i < dev->data->nb_tx_queues; i++) >> +        dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED; >> + >>       sfcr_info(sr, "done"); >>       return 0; > > .