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 AFE52A0561; Fri, 5 Mar 2021 10:58:41 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 99B9940691; Fri, 5 Mar 2021 10:58:41 +0100 (CET) Received: from szxga05-in.huawei.com (szxga05-in.huawei.com [45.249.212.191]) by mails.dpdk.org (Postfix) with ESMTP id 5F9D440147 for ; Fri, 5 Mar 2021 10:58:40 +0100 (CET) Received: from DGGEMS414-HUB.china.huawei.com (unknown [172.30.72.59]) by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4DsNS73XVnzjWSZ; Fri, 5 Mar 2021 17:56:55 +0800 (CST) Received: from [10.78.49.194] (10.78.49.194) by DGGEMS414-HUB.china.huawei.com (10.3.19.214) with Microsoft SMTP Server id 14.3.498.0; Fri, 5 Mar 2021 17:58:35 +0800 To: "Li, Xiaoyun" , "Yigit, Ferruh" CC: "dev@dpdk.org" , "linuxarm@openeuler.org" References: <1614929583-37727-1-git-send-email-oulijun@huawei.com> From: oulijun Message-ID: Date: Fri, 5 Mar 2021 17:58: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] app/testpmd: support Tx mbuf free on demand cmd 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/5 15:46, Li, Xiaoyun дµÀ: > Hi > Sorry, forgot to send this in last patchset. > >> -----Original Message----- >> From: Lijun Ou >> Sent: Friday, March 5, 2021 15:33 >> To: Yigit, Ferruh >> Cc: Li, Xiaoyun ; dev@dpdk.org; >> linuxarm@openeuler.org >> Subject: [PATCH] app/testpmd: support Tx mbuf free on demand cmd >> >> From: Chengwen Feng >> >> This patch support tx_done_cleanup command: >> tx_done_cleanup port (port_id) (queue_id) (free_cnt) >> >> User must make sure there are no concurrent access to the same Tx queue (like >> rte_eth_tx_burst, rte_eth_dev_tx_queue_stop and so on) when this command > Users ...... this command is executed. OK£¬ I will fix. >> executed. >> >> Signed-off-by: Chengwen Feng >> Signed-off-by: Lijun Ou >> --- >> app/test-pmd/cmdline.c | 91 +++++++++++++++++++++++++++++ >> doc/guides/rel_notes/release_21_05.rst | 2 + >> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 +++ >> 3 files changed, 100 insertions(+) >> >> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index >> 14110eb..832ae70 100644 >> --- a/app/test-pmd/cmdline.c >> +++ b/app/test-pmd/cmdline.c >> @@ -36,6 +36,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -675,6 +676,9 @@ static void cmd_help_long_parsed(void *parsed_result, >> "set port (port_id) ptype_mask (ptype_mask)\n" >> " set packet types classification for a specific >> port\n\n" >> >> + "tx_done_cleanup (port_id) (queue_id) (free_cnt)\n" >> + " Cleanup a tx queue's mbuf on a port\n\n" >> + >> "set port (port_id) queue-region region_id (value) " >> "queue_start_index (value) queue_num (value)\n" >> " Set a queue region on a port\n\n" >> @@ -16910,6 +16914,92 @@ cmdline_parse_inst_t cmd_showport_macs = { >> }, >> }; >> >> +/* *** tx_done_cleanup *** */ >> +struct cmd_tx_done_cleanup_result { >> + cmdline_fixed_string_t clean; >> + cmdline_fixed_string_t port; >> + uint16_t port_id; >> + uint16_t queue_id; >> + uint32_t free_cnt; >> +}; >> + >> +static void >> +cmd_tx_done_cleanup_parsed(void *parsed_result, >> + __rte_unused struct cmdline *cl, >> + __rte_unused void *data) >> +{ >> + struct cmd_tx_done_cleanup_result *res = parsed_result; >> + struct rte_eth_dev *dev; >> + uint16_t port_id = res->port_id; >> + uint16_t queue_id = res->queue_id; >> + uint32_t free_cnt = res->free_cnt; >> + int ret; >> + >> + if (!rte_eth_dev_is_valid_port(port_id)) { >> + printf("Invalid port_id %u\n", port_id); >> + return; >> + } >> + >> + dev = &rte_eth_devices[port_id]; >> + if (queue_id >= dev->data->nb_tx_queues) { >> + printf("Invalid TX queue_id %u\n", queue_id); > Tx? You just want to send a patch to use Rx/Tx. You should keep concurrency. Good idea. I will fix it. >> + return; >> + } >> + >> + if (dev->data->tx_queue_state[queue_id] != >> + RTE_ETH_QUEUE_STATE_STARTED) { >> + printf("TX queue_id %u not started!\n", queue_id); > Tx? > >> + return; >> + } >> + >> + /* >> + * rte_eth_tx_done_cleanup is a dataplane API, user must make sure >> + * there are no concurrent access to the same Tx queue (like >> + * rte_eth_tx_burst, rte_eth_dev_tx_queue_stop and so on) when this >> API >> + * called. >> + */ > And I have some concerns on this. Users cannot know about this unless they read your code. I don't think this will likely happen. > So you should document this in testpmd doc when you introduced this command. > > Or maybe you can have a way to stop that to happen. Maybe checking "test_done && engine != rxonly" before this? > In your comment, the tx forwarding shouldn't happen, right? Every fwd engine except rxonly will contain tx. > Well, it's still a rough way because tx may not happen even if it's io fwd. > > Maybe other people have better ideas. Otherwise, you should at least document it. > @Yigit, Ferruh What do you think? Is there a better way? I have add doc for noting. >> + ret = rte_eth_tx_done_cleanup(port_id, queue_id, free_cnt); >> + if (ret < 0) { >> + printf("Failed to cleanup mbuf for port %u TX queue %u " > Tx? > >> + "error desc: %s(%d)\n", >> + port_id, queue_id, strerror(-ret), ret); >> + return; >> + } >> + >> + printf("Cleanup port %u TX queue %u mbuf nums: %u\n", > Tx? > >> + port_id, queue_id, ret); >> +} >> + >> +cmdline_parse_token_string_t cmd_tx_done_cleanup_clean = >> + TOKEN_STRING_INITIALIZER(struct cmd_tx_done_cleanup_result, clean, >> + "tx_done_cleanup"); >> +cmdline_parse_token_string_t cmd_tx_done_cleanup_port = >> + TOKEN_STRING_INITIALIZER(struct cmd_tx_done_cleanup_result, port, >> + "port"); >> +cmdline_parse_token_num_t cmd_tx_done_cleanup_port_id = >> + TOKEN_NUM_INITIALIZER(struct cmd_tx_done_cleanup_result, port_id, >> + UINT16); >> +cmdline_parse_token_num_t cmd_tx_done_cleanup_queue_id = >> + TOKEN_NUM_INITIALIZER(struct cmd_tx_done_cleanup_result, >> queue_id, >> + UINT16); >> +cmdline_parse_token_num_t cmd_tx_done_cleanup_free_cnt = >> + TOKEN_NUM_INITIALIZER(struct cmd_tx_done_cleanup_result, >> free_cnt, >> + UINT32); >> + >> +cmdline_parse_inst_t cmd_tx_done_cleanup = { >> + .f = cmd_tx_done_cleanup_parsed, >> + .data = NULL, >> + .help_str = "tx_done_cleanup port ", >> + .tokens = { >> + (void *)&cmd_tx_done_cleanup_clean, >> + (void *)&cmd_tx_done_cleanup_port, >> + (void *)&cmd_tx_done_cleanup_port_id, >> + (void *)&cmd_tx_done_cleanup_queue_id, >> + (void *)&cmd_tx_done_cleanup_free_cnt, >> + NULL, >> + }, >> +}; >> + >> /* >> ***************************************************************** >> *************** */ >> >> /* list of instructions */ >> @@ -17035,6 +17125,7 @@ cmdline_parse_ctx_t main_ctx[] = { >> (cmdline_parse_inst_t *)&cmd_config_rss_reta, >> (cmdline_parse_inst_t *)&cmd_showport_reta, >> (cmdline_parse_inst_t *)&cmd_showport_macs, >> + (cmdline_parse_inst_t *)&cmd_tx_done_cleanup, >> (cmdline_parse_inst_t *)&cmd_config_burst, >> (cmdline_parse_inst_t *)&cmd_config_thresh, >> (cmdline_parse_inst_t *)&cmd_config_threshold, diff --git >> a/doc/guides/rel_notes/release_21_05.rst >> b/doc/guides/rel_notes/release_21_05.rst >> index 23f7f0b..8077573 100644 >> --- a/doc/guides/rel_notes/release_21_05.rst >> +++ b/doc/guides/rel_notes/release_21_05.rst >> @@ -69,6 +69,8 @@ New Features >> >> * Added command to display Rx queue used descriptor count. >> ``show port (port_id) rxq (queue_id) desc used count`` >> + * Added command to cleanup a Tx queue's mbuf on a port. >> + ``tx_done_cleanup port `` >> >> >> Removed Items >> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst >> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst >> index f59eb8a..39281f5 100644 >> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst >> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst >> @@ -272,6 +272,13 @@ and ready to be processed by the driver on a given RX >> queue:: >> >> testpmd> show port (port_id) rxq (queue_id) desc used count >> >> +cleanup txq mbufs >> +~~~~~~~~~~~~~~~~~~~~~~~~ >> + >> +Request the driver to free mbufs currently cached by the driver for a >> +given port's Tx queue:: >> + testpmd> tx_done_cleanup port (port_id) (queue_id) (free_cnt) >> + >> show config >> ~~~~~~~~~~~ >> >> -- >> 2.7.4 > . >