[dpdk-dev] [PATCH v16 8/8] app/testpmd: add command to set supported ptype mask

[Ferruh Yigit]

On 11/7/2019 6:55 PM, Jerin Jacob wrote:
> 
> 
> On Fri, 8 Nov, 2019, 12:10 am Ferruh Yigit, <ferruh.yigit at intel.com
> <mailto:ferruh.yigit at intel.com>> wrote:
> 
>     On 11/6/2019 7:18 PM, pbhagavatula at marvell.com
>     <mailto:pbhagavatula at marvell.com> wrote:
>     > From: Pavan Nikhilesh <pbhagavatula at marvell.com
>     <mailto:pbhagavatula at marvell.com>>
>     >
>     > Add command to set supported ptype mask.
>     > Usage:
>     >       set port <port_id> ptype_mask >  /* ***************
>     > 
>     >  /* list of instructions */
>     > @@ -19155,6 +19237,7 @@ cmdline_parse_ctx_t main_ctx[] = {
>     >       (cmdline_parse_inst_t *)&cmd_show_vf_stats,
>     >       (cmdline_parse_inst_t *)&cmd_clear_vf_stats,
>     >       (cmdline_parse_inst_t *)&cmd_show_port_supported_ptypes,
>     > +     (cmdline_parse_inst_t *)&cmd_set_port_ptypes,
>     >       (cmdline_parse_inst_t *)&cmd_ptype_mapping_get,
>     >       (cmdline_parse_inst_t *)&cmd_ptype_mapping_replace,
>     >       (cmdline_parse_inst_t *)&cmd_ptype_mapping_reset,
>     > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>     > index 5ba974162..812aebf35 100644
>     > --- a/app/test-pmd/testpmd.c
>     > +++ b/app/test-pmd/testpmd.c
>     > @@ -2024,6 +2024,7 @@ start_port(portid_t pid)
>     >       queueid_t qi;
>     >       struct rte_port *port;
>     >       struct rte_ether_addr mac_addr;
>     > +     static uint8_t clr_ptypes = 1;
>     > 
>     >       if (port_id_is_invalid(pid, ENABLED_WARN))
>     >               return 0;
>     > @@ -2153,6 +2154,10 @@ start_port(portid_t pid)
>     >                       }
>     >               }
>     >               configure_rxtx_dump_callbacks(verbose_level);
>     > +             if (clr_ptypes) {
>     > +                     clr_ptypes = 0;
>     > +                     rte_eth_dev_set_ptypes(pi, RTE_PTYPE_UNKNOWN, NULL, 0);
>     > +             }
> 
>     I am not sure about this command, we have now capability to set/disable ptypes
>     on demand, why disabling them by default?
> 
> 
> As forward engines are not using the ptype offload. If a specific forward mode
> need the offload, it can be enabled.
> 

OK, I am still not sure but I understand your reasoning.

But this is a behavior change and it may caught people, what about following
more operational updates:
- Separate this into its own patch, this is different than adding new command
- move "clr_ptypes" next to other static global variables, to make this
selection more obvious, and those global variables tend to have some comments,
we can add comment to this one as well.
- print a log when "rte_eth_dev_set_ptypes()" returns success, to say packets
types parsing is disabled, to help user to understand what is happening. Because
currently there is no other way to get the current configured ptypes from PMD.