> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, October 13, 2017 3:59 AM
> To: Singh, Jasvinder <jasvinder.si...@intel.com>; dev@dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitre...@intel.com>; De Lara Guarch,
> Pablo <pablo.de.lara.gua...@intel.com>; Pei, Yulong
> <yulong....@intel.com>
> Subject: RE: [PATCH v7 3/3] app/test-pmd: add CLI for TM nodes and
> hierarchy commit
>
> > + /* Port status */
> > + if (port_is_started(port_id)) {
> > + printf(" Port %u not stopped (error)\n", port_id);
> > + return;
> > + }
> > +
> > + /* Node parameters */
> > + if (res->parent_node_id < 0)
> > + parent_node_id = UINT32_MAX;
> > + else
> > + parent_node_id = res->parent_node_id;
> > +
> > + memset(&np, 0, sizeof(struct rte_tm_node_params));
> > + np.shaper_profile_id = res->shaper_profile_id;
> > + np.n_shared_shapers = res->n_shared_shapers;
> > +
> > + if (np.n_shared_shapers == 1)
> > + np.shared_shaper_id[0] = res->shared_shaper_id;
> > + else
> > + np.shared_shaper_id = NULL;
> > +
>
> Does n_shared_shapers means number of shared_shapers? And now we
> only support 1?
Yes. It is number of shared shapers. The ixgbe and i40e doesn't support shared
shapers, while softnic support single shared shaper.
> When refer to the definition of struct rte_tm_node_params, the
> shared_shaper_id arry need to be allocated by user, but I didn't find the
> allocation here or even in patch 2/3.
>
> The same comments for below commands.
Will fix this.
> [......]
>
> > +/* *** Port TM Hierarchy Commit *** */ struct
> > +cmd_port_tm_hierarchy_commit_result {
> > + cmdline_fixed_string_t port;
> > + cmdline_fixed_string_t tm;
> > + cmdline_fixed_string_t hierarchy;
> > + cmdline_fixed_string_t commit;
> > + uint16_t port_id;
> > + uint32_t clean_on_fail;
> > +};
> > +
> > +cmdline_parse_token_string_t cmd_port_tm_hierarchy_commit_port =
> > + TOKEN_STRING_INITIALIZER(
> > + struct cmd_port_tm_hierarchy_commit_result, port, "port");
> > +cmdline_parse_token_string_t cmd_port_tm_hierarchy_commit_tm =
> > + TOKEN_STRING_INITIALIZER(
> > + struct cmd_port_tm_hierarchy_commit_result, tm, "tm");
> > +cmdline_parse_token_string_t
> cmd_port_tm_hierarchy_commit_hierarchy =
> > + TOKEN_STRING_INITIALIZER(
> > + struct cmd_port_tm_hierarchy_commit_result,
> > + hierarchy, "hierarchy");
> > +cmdline_parse_token_string_t cmd_port_tm_hierarchy_commit_commit
> =
> > + TOKEN_STRING_INITIALIZER(
> > + struct cmd_port_tm_hierarchy_commit_result, commit,
> > "commit");
> > +cmdline_parse_token_num_t cmd_port_tm_hierarchy_commit_port_id
> =
> > + TOKEN_NUM_INITIALIZER(
> > + struct cmd_port_tm_hierarchy_commit_result,
> > + port_id, UINT16);
> > +cmdline_parse_token_num_t
> cmd_port_tm_hierarchy_commit_clean_on_fail
> > =
> > + TOKEN_NUM_INITIALIZER(struct
> > cmd_port_tm_hierarchy_commit_result,
> > + clean_on_fail, UINT32);
> How about the define clean_on_fail to be a string like "(clean|no_clean)" or
> "clean_on_fail (yes|no)"?
Will make this string.
> And don't forget the doc update for all the new commands.
Yes, will update the doc accordingly. Thanks.
> Thanks
> Jingjing
