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 D67DAA0555 for ; Thu, 9 Jun 2022 13:20:47 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CB6AE40220; Thu, 9 Jun 2022 13:20:47 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 000A240220; Thu, 9 Jun 2022 13:20:45 +0200 (CEST) Received: from kwepemi500017.china.huawei.com (unknown [172.30.72.57]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4LJhST5z9lzjXBJ; Thu, 9 Jun 2022 19:19:21 +0800 (CST) Received: from [10.67.103.235] (10.67.103.235) by kwepemi500017.china.huawei.com (7.221.188.110) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Thu, 9 Jun 2022 19:20:43 +0800 Subject: Re: [PATCH v4] app/testpmd: fix slave device isn't released To: Ferruh Yigit , References: <20220503100217.46203-5-humin29@huawei.com> <20220607081038.23859-1-liudongdong3@huawei.com> <9a0fcfdf-6302-36f8-e26a-ff228cccfc74@xilinx.com> <714db392-457c-0570-779c-49513caeb944@xilinx.com> CC: , , , , , , Huisong Li , Min Hu From: Dongdong Liu Message-ID: <91f92b9a-8e8a-a411-b3df-8c615068eb79@huawei.com> Date: Thu, 9 Jun 2022 19:20:42 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <714db392-457c-0570-779c-49513caeb944@xilinx.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.103.235] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemi500017.china.huawei.com (7.221.188.110) X-CFilter-Loop: Reflected X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Hi Ferruh Many thanks for your review. On 2022/6/9 16:50, Ferruh Yigit wrote: > On 6/9/2022 8:50 AM, Dongdong Liu wrote: >> Hi Ferruh >> >> Many thanks for your review. >> On 2022/6/7 22:31, Ferruh Yigit wrote: >>> On 6/7/2022 9:10 AM, Dongdong Liu wrote: >>>> From: Huisong Li >>>> >>>> Currently, some eth devices are added to bond device, these devices >>>> are not >>>> released when the quit command is executed in testpmd. This patch adds >>>> the >>>> release operation for all active slaves under a bond device. >>>> >>>> Fixes: 0e545d3047fe ("app/testpmd: check stopping port is not in >>>> bonding") >>>> Cc: stable@dpdk.org >>>> >>>> Signed-off-by: Huisong Li >>>> Signed-off-by: Min Hu (Connor) >>>> Signed-off-by: Dongdong Liu >>>> --- >>>> app/test-pmd/cmdline.c | 1 + >>>> app/test-pmd/testpmd.c | 46 >>>> ++++++++++++++++++++++++++++++++++++++++++ >>>> app/test-pmd/testpmd.h | 2 ++ >>>> 3 files changed, 49 insertions(+) >>>> >>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c >>>> index fdd0cada3b..6c58b02650 100644 >>>> --- a/app/test-pmd/cmdline.c >>>> +++ b/app/test-pmd/cmdline.c >>>> @@ -8915,6 +8915,7 @@ static void cmd_quit_parsed(__rte_unused void >>>> *parsed_result, >>>> __rte_unused void *data) >>>> { >>>> cmdline_quit(cl); >>>> + cl_quit = 1; >>>> } >>>> static cmdline_parse_token_string_t cmd_quit_quit = >>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c >>>> index fc1b64b60d..2b16551a26 100644 >>>> --- a/app/test-pmd/testpmd.c >>>> +++ b/app/test-pmd/testpmd.c >>>> @@ -229,6 +229,7 @@ unsigned int xstats_display_num; /**< Size of >>>> extended statistics to show */ >>>> * option. Set flag to exit stats period loop after received >>>> SIGINT/SIGTERM. >>>> */ >>>> uint8_t f_quit; >>>> +uint8_t cl_quit; /* Quit testpmd from cmdline. */ >>>> /* >>>> * Max Rx frame size, set by '--max-pkt-len' parameter. >>>> @@ -3201,11 +3202,44 @@ remove_invalid_ports(void) >>>> nb_cfg_ports = nb_fwd_ports; >>>> } >>>> +static void >>>> +clear_bonding_slave_device(portid_t *slave_pids, uint16_t num_slaves) >>>> +{ >>>> +#ifdef RTE_NET_BOND >>>> + struct rte_port *port; >>>> + portid_t slave_pid; >>>> + uint16_t i; >>>> + >>>> + for (i = 0; i < num_slaves; i++) { >>>> + slave_pid = slave_pids[i]; >>>> + if (port_is_started(slave_pid) == 1) { >>>> + if (rte_eth_dev_stop(slave_pid) != 0) >>>> + fprintf(stderr, "rte_eth_dev_stop failed for port >>>> %u\n", >>>> + slave_pid); >>>> + >>>> + port = &ports[slave_pid]; >>>> + port->port_status = RTE_PORT_STOPPED; >>>> + } >>>> + >>>> + clear_port_slave_flag(slave_pid); >>>> + >>>> + /* Close slave device when testpmd quit or is killed. */ >>>> + if (cl_quit == 1 || f_quit == 1) >>>> + rte_eth_dev_close(slave_pid); >>>> + } >>>> +#else >>>> + RTE_SET_USED(slave_pids); >>>> + RTE_SET_USED(num_slaves); >>>> +#endif >>> >>> Do we need this #ifdef, everything is already available in compile time. >> >> Although it can be compiled ok without this #ifdef, >> I think we still need this #ifdef as this is bond related >> implementation. >> > > Code will compile and function as expected even if bonding PMD is > disable, right? If so why #ifdef is needed. Yes, will delete. Thanks, Dongdong > >> This patch does the similar #ifdef grouping done in 3/5 patch. >> >> Thanks, >> Dongdong >>> >>>> +} >>>> + >>>> void >>>> close_port(portid_t pid) >>>> { >>>> portid_t pi; >>>> struct rte_port *port; >>>> + portid_t slave_pids[RTE_MAX_ETHPORTS]; >>>> + int num_slaves = 0; >>>> if (port_id_is_invalid(pid, ENABLED_WARN)) >>>> return; >>>> @@ -3240,7 +3274,19 @@ close_port(portid_t pid) >>>> port_flow_flush(pi); >>>> port_flex_item_flush(pi); >>>> port_action_handle_flush(pi); >>>> +#ifdef RTE_NET_BOND >>>> + if (port->bond_flag == 1) >>>> + num_slaves = rte_eth_bond_slaves_get(pi, >>>> + slave_pids, RTE_MAX_ETHPORTS); >>>> +#endif >>>> rte_eth_dev_close(pi); >>>> + /* >>>> + * If this port is bonded device, all slaves under the >>>> + * device need to be removed or closed. >>>> + */ >>>> + if (port->bond_flag == 1 && num_slaves > 0) >>>> + clear_bonding_slave_device(slave_pids, >>>> + num_slaves); >>>> } >>>> free_xstats_display_info(pi); >>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h >>>> index 6693813dda..b44d5dd5ac 100644 >>>> --- a/app/test-pmd/testpmd.h >>>> +++ b/app/test-pmd/testpmd.h >>>> @@ -38,6 +38,8 @@ >>>> #define RTE_PORT_CLOSED (uint16_t)2 >>>> #define RTE_PORT_HANDLING (uint16_t)3 >>>> +extern uint8_t cl_quit; >>>> + >>>> /* >>>> * It is used to allocate the memory for hash key. >>>> * The hash key size is NIC dependent. >>> >>> . >>> > > . >