Hi Pavin,
<snip>
> >> > Subject: [dpdk-dev] [PATCH v16 8/8] app/testpmd: add command
> >to set
> >> > supported ptype mask
> >> >
> >> > From: Pavan Nikhilesh <pbhagavat...@marvell.com>
> >> >
> >> > Add command to set supported ptype mask.
> >> > Usage:
> >> > set port <port_id> ptype_mask <ptype_mask>
> >> >
> >> > Disable ptype parsing by default.
> >> >
> >> > Signed-off-by: Pavan Nikhilesh <pbhagavat...@marvell.com>
> >> > ---
> >> > app/test-pmd/cmdline.c | 83 +++++++++++++++++++++
> >> > app/test-pmd/testpmd.c | 5 ++
> >> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 ++
> >> > 3 files changed, 95 insertions(+)
> >> >
> >> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> >index
> >> > 49c45a3f0..7af2c58c3 100644
> >> > --- a/app/test-pmd/cmdline.c
> >> > +++ b/app/test-pmd/cmdline.c
> >>
> >> The help text should be updated to describe the new command at
> >line 240 in
> >> cmdline.c
> >>
> >> > @@ -668,6 +668,9 @@ static void cmd_help_long_parsed(void
> >> > *parsed_result,
> >> > "ptype mapping update (port_id) (hw_ptype)
> >> (sw_ptype)\n"
> >> > " Update a ptype mapping item on a port\n\n"
> >> >
> >> > + "set port (port_id) ptype_mask
> >(ptype_mask)\n"
> >> > + " set packet types classification for a
> >> > specific
> >> > 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"
> >> > @@ -18916,6 +18919,85 @@ cmdline_parse_inst_t
> >> > cmd_show_port_supported_ptypes = {
> >> > },
> >> > };
> >> >
> >> > +/* Common result structure for set port ptypes */ struct
> >> > +cmd_set_port_ptypes_result {
> >> > + cmdline_fixed_string_t set;
> >> > + cmdline_fixed_string_t port;
> >> > + portid_t port_id;
> >> > + cmdline_fixed_string_t ptype_mask;
> >> > + uint32_t mask;
> >> > +};
> >> > +
> >> > +/* Common CLI fields for set port ptypes */
> >> > +cmdline_parse_token_string_t cmd_set_port_ptypes_set =
> >> > + TOKEN_STRING_INITIALIZER
> >> > + (struct cmd_set_port_ptypes_result,
> >> > + set, "set");
> >> > +cmdline_parse_token_string_t cmd_set_port_ptypes_port =
> >> > + TOKEN_STRING_INITIALIZER
> >> > + (struct cmd_set_port_ptypes_result,
> >> > + port, "port");
> >> > +cmdline_parse_token_num_t cmd_set_port_ptypes_port_id =
> >> > + TOKEN_NUM_INITIALIZER
> >> > + (struct cmd_set_port_ptypes_result,
> >> > + port_id, UINT16);
> >> > +cmdline_parse_token_string_t cmd_set_port_ptypes_mask_str =
> >> > + TOKEN_STRING_INITIALIZER
> >> > + (struct cmd_set_port_ptypes_result,
> >> > + ptype_mask, "ptype_mask");
> >> > +cmdline_parse_token_num_t cmd_set_port_ptypes_mask_u32 =
> >> > + TOKEN_NUM_INITIALIZER
> >> > + (struct cmd_set_port_ptypes_result,
> >> > + mask, UINT32);
> >> > +
> >> > +static void
> >> > +cmd_set_port_ptypes_parsed(
> >> > + void *parsed_result,
> >> > + __attribute__((unused)) struct cmdline *cl,
> >> > + __attribute__((unused)) void *data) {
> >> > + struct cmd_set_port_ptypes_result *res = parsed_result;
> >> > +#define PTYPE_NAMESIZE 256
> >> > + char ptype_name[PTYPE_NAMESIZE];
> >> > + uint16_t port_id = res->port_id;
> >> > + uint32_t ptype_mask = res->mask;
> >> > + int ret, i;
> >> > +
> >> > + ret = rte_eth_dev_get_supported_ptypes(port_id,
> >ptype_mask,
> >> > NULL, 0);
> >
> >The last 2 parameters to the above function do not look correct, here
> >is the function declaration:
> >int rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t
> >ptype_mask, uint32_t *ptypes, int num);
> >
> >ptypes should be a pointer to an array to hold the ptypes, and num
> >should be the size of the array.
>
> We can use the same API to get the number of ptypes supported to initialize
> the array below.
>
> @see examples/l3fwd/l3fwd_lpm.c +424
Yes, you are correct, sorry for the noise.
> >> > + if (ret <= 0) {
> >> > + printf("Port %d doesn't support any ptypes.\n",
> >port_id);
> >> > + return;
> >> > + }
> >> > +
> >> > + uint32_t ptypes[ret];
> >
> >The above declaration can then be moved to the top of the function with
> >the other declarations.
>
>
> I intentionally placed it here as the array size depends on ret and there is
> no
> readily available macro for max number of packet types.
Yes, this is correct too
<snip>
Regards,
Bernard.
