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 E2D41A0A0A; Wed, 24 Mar 2021 02:00:42 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 77A3D40151; Wed, 24 Mar 2021 02:00:42 +0100 (CET) Received: from szxga05-in.huawei.com (szxga05-in.huawei.com [45.249.212.191]) by mails.dpdk.org (Postfix) with ESMTP id 19DF54014F for ; Wed, 24 Mar 2021 02:00:40 +0100 (CET) Received: from DGGEMS404-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4F4qcJ4TMLzwQLH; Wed, 24 Mar 2021 08:58:40 +0800 (CST) Received: from [10.78.49.194] (10.78.49.194) by DGGEMS404-HUB.china.huawei.com (10.3.19.204) with Microsoft SMTP Server id 14.3.498.0; Wed, 24 Mar 2021 09:00:35 +0800 To: "Li, Xiaoyun" , "Yigit, Ferruh" CC: "dev@dpdk.org" , "linuxarm@openeuler.org" References: <1614939741-63927-1-git-send-email-oulijun@huawei.com> <1614939741-63927-3-git-send-email-oulijun@huawei.com> From: oulijun Message-ID: Date: Wed, 24 Mar 2021 09:00:35 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="gbk"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.78.49.194] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH 2/3] app/testpmd: remove forwarding config from parsing Rx and Tx 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 Sender: "dev" ÔÚ 2021/3/23 15:50, Li, Xiaoyun дµÀ: > Hi > >> -----Original Message----- >> From: Lijun Ou >> Sent: Friday, March 5, 2021 18:22 >> To: Yigit, Ferruh >> Cc: Li, Xiaoyun ; dev@dpdk.org; >> linuxarm@openeuler.org >> Subject: [PATCH 2/3] app/testpmd: remove forwarding config from parsing Rx >> and Tx >> >> From: Huisong Li >> > > The commit message should be more simple and avoids grammar mistakes. > All right, I will simply it. >> The "fwd_config_setup()" function does release and apply for memory of >> forwarding flows, and re-establish these streams when rxq/txq or rxd/txd is >> changed. The function is also called by "start_packet_forwarding()" when user >> executes "start" cmd. >> All changes for rxq/txq or rxd/txd can be updated uniformly when this command >> is executed. Therefore, it is a little redundant in the "cmd_config_rx_tx_parsed" >> function. > > It's not redundant. This command may configure number of rxq/txq. So the fwd streams map may change. > Then it's common to check the fwd streams after this command using "show config fwd". > If you remove this fwd stream update, users can't get the correct new fwd streams until they start the traffic. > But they may change a lot of things and want to check if the setting is correct before they start the traffic. > Yes, you are right. It's really unfriendly. >> >> In addition, the forwarding stream under one TC is configured based on number >> of queues allocated to TC. And number of queues allocated to TC is updated >> after calling "rte_eth_dev_configure" >> again. If the number of queues is reduced after configuring the DCB, and then, >> release and apply for stream memory, and reinitialize the forwarding stream >> under the DCB mode based on the old TC information. As a result, null pointer >> may be accessed. > > I think you should add "rte_eth_dev_configure " into dcb_fwd_config_setup() before rte_eth_dev_get_dcb_info(). > > And the commit message should be similar like the following: > Segment fault might happen after configuring queue number to less because dcb_fwd_config_setup setup dcb based on old dcb info. > And dcb info can only update after rte_eth_dev_configure(). > So this patch adds rte_eth_dev_configure() before rte_eth_dev_get_dcb_info() to get updated dcb info to fix this issue. > Thank you for your advice. But the above adjustments may still not work for some drivers. The mapping between queues and TCs in these drivers is updated in the dev_start stage. I have an idea. We can move fwd_config_setup() to start_port(), which is called by main() and after starting ports This not only solves the segment fault, but also does not have the problem you mentioned above. I test it and it is ok. What do you think, xiaoyun? >> >> Like: >> set nbcore 4 >> port stop all >> port config 0 dcb vt off 4 pfc on >> port start all >> port stop all >> port config all rxq 8 >> port config all txq 8 >> >> At the moment, a segmentation fault occurs. >> >> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings") >> Cc: stable@dpdk.org >> >> Signed-off-by: Huisong Li >> Signed-off-by: Lijun Ou >> --- >> V1->V2: >> - use stream instead of flow >> --- >> app/test-pmd/cmdline.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index >> 4df0c32..e316f5c 100644 >> --- a/app/test-pmd/cmdline.c >> +++ b/app/test-pmd/cmdline.c >> @@ -1837,8 +1837,6 @@ cmd_config_rx_tx_parsed(void *parsed_result, >> return; >> } >> >> - fwd_config_setup(); >> - >> init_port_config(); >> >> cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1); >> -- >> 2.7.4 > > . >