>On Fri, Nov 8, 2019 at 7:24 PM Ananyev, Konstantin
><konstantin.anan...@intel.com> wrote:
>>
>>
>>
>> > -----Original Message-----
>> > From: dev <dev-boun...@dpdk.org> On Behalf Of Ferruh Yigit
>> > Sent: Thursday, November 7, 2019 7:41 PM
>> > To: Jerin Jacob <jerinjac...@gmail.com>
>> > Cc: Pavan Nikhilesh <pbhagavat...@marvell.com>; Andrew
>Rybchenko <arybche...@solarflare.com>; jer...@marvell.com;
>> > tho...@monjalon.net; Lu, Wenzhuo <wenzhuo...@intel.com>;
>Wu, Jingjing <jingjing...@intel.com>; Iremonger, Bernard
>> > <bernard.iremon...@intel.com>; Mcnamara, John
><john.mcnam...@intel.com>; Kovacevic, Marko
><marko.kovace...@intel.com>;
>> > dev@dpdk.org
>> > Subject: Re: [dpdk-dev] [PATCH v16 8/8] app/testpmd: add
>command to set supported ptype mask
>> >
>> > 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
>forwa
>> > > need the offload, it can be enabled.
>>
>> Well, at least now user can see ptype in pkt dump with 'set verbose >
>0'
>> It's definitely looks loke a a behavior change.
>> Wonder why it can't be done visa-versa?
>> Keep current behavior as default one (all ptypes are un-masked) and
>> have special command to disable/enable ptypes.
>> as I understand such command was added by these series. correct?
>
>IMO, we are following the principle that by default all offloads are
>disabled and enable it
>based on the need to have optimal performance across the DPDK. This
>change will be
>on the same theme.
>
>Regarding the verbose > 0 cases, I think, we can call
>rte_eth_dev_set_ptypes() to all PTYPES on
>the setting verbose callback.
Agree verbose > 0 case we can do set_ptypes with
RTE_PTYPE_ALL_MASK. But what if the user wants to verify
rte_eth_dev_set_ptypes() call itself in verbose mode?.
Example:
set port 0 ptype_mask RTE_PTYPE_L3_MASK
set verbose 1
In this case user expects to see only L3 mask set. Wouldn’t it be confusing if
we set RTE_PTYPE_ALL_MASK under the hood when verbose level is increased?
Pavan.
