[EXT] Re: [PATCH v3 1/1] app/testpmd: control passing Rx metadata to PMD

[Hanumanth Reddy Pothula]

> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Saturday, December 3, 2022 1:11 AM
> To: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>; Hanumanth
> Reddy Pothula <hpothula at marvell.com>
> Cc: dev at dpdk.org; Aman Singh <aman.deep.singh at intel.com>; Yuying
> Zhang <yuying.zhang at intel.com>; dev at dpdk.org; Jerin Jacob
> Kollanukkaran <jerinj at marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram at marvell.com>; rasland at nvidia.com; orika at nvidia.com;
> viacheslavo at nvidia.com
> Subject: Re: [EXT] Re: [PATCH v3 1/1] app/testpmd: control passing Rx
> metadata to PMD
> 
> 02/12/2022 17:14, Hanumanth Reddy Pothula:
> > -----Original Message-----
> > > 27/10/2022 09:34, Thomas Monjalon:
> > > > 17/10/2022 10:32, Andrew Rybchenko:
> > > > > On 10/6/22 21:35, Hanumanth Pothula wrote:
> > > > > > Presently, Rx metadata is sent to PMD by default, leading to a
> > > > > > performance drop as processing for the same in rx path takes
> > > > > > extra cycles.
> > > > > >
> > > > > > Hence, introducing command line argument, 'nic-to-pmd-rx-
> metadata'
> > > > > > to control passing rx metadata to PMD. By default it’s disabled.
> > > > > >
> > > > > > Signed-off-by: Hanumanth Pothula <hpothula at marvell.com>
> > > > >
> > > > > Acked-by: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
> > > > >
> > > > > Applied to dpdk-next-net/main, thanks.
> > > >
> > > > I'm not sure this patch is really acceptable.
> > > > It is disabling Rx metadata by default just for benchmarking
> > > > reason because your driver is doing some processing even if
> > > > metadata is not
> > > required.
> > > >
> > > > From a user perspective, if a command requesting metadata is
> > > > entered, it won't work until we enable this new option on startup.
> > > > It looks terrible.
> > > >
> > > > Please tell me I misunderstood something.
> > >
> > > While pulling, I see that the name is not compliant with others.
> > > I think it should start with "enable-", use hyphens and be sorted.
> > >
> > > I'll drop it from the pull for now, we can have it in -rc3.
> > >
> >
> > @Thomas Monjalon I missed your comment, sorry for the delayed
> response.
> >
> > Sending Rx metadata to PMD is added recently, which breaking our driver
> performance.
> > Normally any feature added to testpmd will be disabled by default, to
> make sure it won't affect other code(PMD).
> > Hence adding new testpmd command line argument to disable this
> feature by default.
> 
> No, disabling by default doesn't mean you should enable with option.
> It should be enabled if required by a command.
> 
Yeah, got it, user can enable/disable the feature on the fly without restarting the application(testpmd).
Will introduce new command, like below, 
  'port config rx-nic-to-pmd-metadata on/off'

Please let me know your thoughts on this command implementation.
If it's fine, will start implementing the command. Suggest if there is any other better way to implement the same.