> +     /* 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?
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.

[......]

> +/* *** 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)"?

And don't forget the doc update for all the new commands.

Thanks
Jingjing

Reply via email to