Hi Sorry, forgot to send this in last patchset. > -----Original Message----- > From: Lijun Ou <ouli...@huawei.com> > Sent: Friday, March 5, 2021 15:33 > To: Yigit, Ferruh <ferruh.yi...@intel.com> > Cc: Li, Xiaoyun <xiaoyun...@intel.com>; dev@dpdk.org; > linux...@openeuler.org > Subject: [PATCH] app/testpmd: support Tx mbuf free on demand cmd > > From: Chengwen Feng <fengcheng...@huawei.com> > > 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. > executed. > > Signed-off-by: Chengwen Feng <fengcheng...@huawei.com> > Signed-off-by: Lijun Ou <ouli...@huawei.com> > --- > 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 <rte_pci.h> > #include <rte_ether.h> > #include <rte_ethdev.h> > +#include <rte_ethdev_driver.h> > #include <rte_string_fns.h> > #include <rte_devargs.h> > #include <rte_flow.h> > @@ -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. > + 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? > + 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 <port_id> <queue_id> <free_cnt>", > + .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 <port_id> <queue_id> <free_cnt>`` > > > 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