On Fri, Feb 16, 2018 at 01:10:39PM +0000, Singh, Jasvinder wrote: > > > > -----Original Message----- > > From: Tomasz Duszynski [mailto:t...@semihalf.com] > > Sent: Thursday, February 1, 2018 9:37 AM > > To: dev@dpdk.org > > Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Wu, Jingjing > > <jingjing...@intel.com>; Singh, Jasvinder <jasvinder.si...@intel.com>; > > Tomasz Duszynski <t...@semihalf.com> > > Subject: [PATCH 2/2] app/testpmd: add command to resume a TM node > > > > Traffic manager provides an API for resuming an arbitrary node in a > > hierarchy. > > > > This commit adds support for calling this API from testpmd. > > > > Signed-off-by: Tomasz Duszynski <t...@semihalf.com> > > --- > > app/test-pmd/cmdline.c | 4 ++ > > app/test-pmd/cmdline_tm.c | 70 > > +++++++++++++++++++++++++++++ > > app/test-pmd/cmdline_tm.h | 1 + > > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 5 +++ > > 4 files changed, 80 insertions(+) > > > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index > > 6bbd606..f9827f6 100644 > > --- a/app/test-pmd/cmdline.c > > +++ b/app/test-pmd/cmdline.c > > @@ -800,6 +800,9 @@ static void cmd_help_long_parsed(void > > *parsed_result, > > "suspend port tm node (port_id) (node_id)" > > " Suspend tm node.\n\n" > > > > + "resume port tm node (port_id) (node_id)" > > + " Resume tm node.\n\n" > > + > > "port tm hierarchy commit (port_id) > > (clean_on_fail)\n" > > " Commit tm hierarchy.\n\n" > > > > @@ -16251,6 +16254,7 @@ cmdline_parse_ctx_t main_ctx[] = { > > (cmdline_parse_inst_t *)&cmd_del_port_tm_node, > > (cmdline_parse_inst_t *)&cmd_set_port_tm_node_parent, > > (cmdline_parse_inst_t *)&cmd_suspend_port_tm_node, > > + (cmdline_parse_inst_t *)&cmd_resume_port_tm_node, > > (cmdline_parse_inst_t *)&cmd_port_tm_hierarchy_commit, > > NULL, > > }; > > diff --git a/app/test-pmd/cmdline_tm.c b/app/test-pmd/cmdline_tm.c index > > c9a18dd..807e724 100644 > > --- a/app/test-pmd/cmdline_tm.c > > +++ b/app/test-pmd/cmdline_tm.c > > @@ -2036,6 +2036,76 @@ cmdline_parse_inst_t > > cmd_suspend_port_tm_node = { > > }, > > }; > > > > +/* *** Resume Port TM Node *** */ > > +struct cmd_resume_port_tm_node_result { > > + cmdline_fixed_string_t resume; > > + cmdline_fixed_string_t port; > > + cmdline_fixed_string_t tm; > > + cmdline_fixed_string_t node; > > + uint16_t port_id; > > + uint32_t node_id; > > +}; > > + > > +cmdline_parse_token_string_t cmd_resume_port_tm_node_resume = > > + TOKEN_STRING_INITIALIZER( > > + struct cmd_resume_port_tm_node_result, resume, > > "resume"); > > +cmdline_parse_token_string_t cmd_resume_port_tm_node_port = > > + TOKEN_STRING_INITIALIZER( > > + struct cmd_resume_port_tm_node_result, port, "port"); > > +cmdline_parse_token_string_t cmd_resume_port_tm_node_tm = > > + TOKEN_STRING_INITIALIZER( > > + struct cmd_resume_port_tm_node_result, tm, "tm"); > > +cmdline_parse_token_string_t cmd_resume_port_tm_node_node = > > + TOKEN_STRING_INITIALIZER( > > + struct cmd_resume_port_tm_node_result, node, "node"); > > +cmdline_parse_token_num_t cmd_resume_port_tm_node_port_id = > > + TOKEN_NUM_INITIALIZER( > > + struct cmd_resume_port_tm_node_result, port_id, UINT16); > > +cmdline_parse_token_num_t cmd_resume_port_tm_node_node_id = > > + TOKEN_NUM_INITIALIZER( > > + struct cmd_resume_port_tm_node_result, node_id, UINT32); > > + > > +static void cmd_resume_port_tm_node_parsed(void *parsed_result, > > + __attribute__((unused)) struct cmdline *cl, > > + __attribute__((unused)) void *data) > > +{ > > + struct cmd_resume_port_tm_node_result *res = parsed_result; > > + struct rte_tm_error error; > > + uint32_t node_id = res->node_id; > > + portid_t port_id = res->port_id; > > + int ret; > > + > > + if (port_id_is_invalid(port_id, ENABLED_WARN)) > > + return; > > + > > + /* Port status */ > > + if (!port_is_started(port_id)) { > > + printf(" Port %u not started (error)\n", port_id); > > + return; > > + } > > I suggest to remove the CLI layer restriction to check the port status for > node suspend/resume. > The device can check at the driver layer whether it is ok suspend/resume node > in started/stopped state depending upon the allowed configuration.
Fair enough. Thanks for the review. > > Besides this, > Reviewed-by: Jasvinder Singh <jasvinder.si...@inel.com> -- - Tomasz Duszyński
