On Fri, Nov 8, 2019 at 1:11 AM Ferruh Yigit <ferruh.yi...@intel.com> wrote: > > On 11/7/2019 6:55 PM, Jerin Jacob wrote: > > > > > > On Fri, 8 Nov, 2019, 12:10 am Ferruh Yigit, <ferruh.yi...@intel.com > > <mailto:ferruh.yi...@intel.com>> wrote: > > > > On 11/6/2019 7:18 PM, pbhagavat...@marvell.com > > <mailto:pbhagavat...@marvell.com> wrote: > > > From: Pavan Nikhilesh <pbhagavat...@marvell.com > > <mailto:pbhagavat...@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.
+1 for operational updates.