On Wed, Feb 1, 2023 at 5:06 PM Ferruh Yigit <ferruh.yi...@amd.com> wrote: > > On 2/1/2023 11:15 AM, Jerin Jacob wrote: > > On Wed, Feb 1, 2023 at 4:35 PM Thomas Monjalon <tho...@monjalon.net> wrote: > >> > >> 01/02/2023 11:58, Andrew Rybchenko: > >>> On 2/1/23 13:48, Jerin Jacob wrote: > >>>> On Wed, Feb 1, 2023 at 2:59 PM Andrew Rybchenko > >>>> <andrew.rybche...@oktetlabs.ru> wrote: > >>>>> Frankly speaking I don't understand why default value is so > >>>>> important if we have a way to change it. Reasons should be > >>>>> really strong to change existing defaults. > >>>> > >>>> The only reason is, typically testpmd will be used performance > >>>> benchmarking as an industry standard. It is difficult to tell/educate > >>>> the QA or customers > >>>> that, "BTW if you need to get better performance add more flag to > >>>> testpmd command line". > >> > >> I disagree. > >> When you do performance benchmark, you tune settings accordingly. > > > > IMO, We tune the system resources like queue depth not the disabling > > features for raw performance. > > queue depth etc people know to tune so it is obvious. What is not > > obvious is, testpmd only > > negotiated some features by default.I am not using that feature, hence > > I need to explicitly > > disable it. > > > > When 'rte_eth_rx_metadata_negotiate()' API is NOT used at all, and I > believe that is the case for almost all applications since API is a > relatively new one, PMD default behavior should be to enable Rx metadata > flow rules, in case user requests them later. > > So, enabling all in application is same with not calling the API at all. > > In this perspective, disabling Rx metadata is additional > optimization/tuning that application can do if it is sure that Rx > metadata flow rules won't be used at all. > And API is more meaningful when it is used to disable Rx metadata. > > I think it is reasonable to enable all Rx metadata by default in testpmd > with a capability to disable it when wanted. > > OR > > May be we don't call 'rte_eth_rx_metadata_negotiate()' API by default in > testpmd, it is only called when it is requested explicitly from user, > enable or disable.
Second option looks good to me. When 1) user request for action which is needed negotiate(), AND 2) rte_eth_rx_metadata_negotiate() != ENOSUP then, testpmd print a warning that need to enable rte_eth_rx_metadata_negotiate(). > > > >> > >>>> To make that worst, only some PMD needs to give the additional > >>>> parameter to get better number. > >>>> And also, testpmd usage will be treated as application modeling. > >>>> > >>>> Since this feature only used on sfc and cnxk driver, What is the > >>>> situation with sfc driver? > >>>> Keeping it as negotiated and not use the feature, will impact the per > >>>> core performance of sfc or > >>>> is it just PCI bandwidth thing which really dont show any difference in > >>>> testpmd? > >>> > >>> Yes, sfc could run faster if no Rx metadata are negotiated. So, > >>> it is better to negotiate nothing by default. But it is always > >>> painful to change defaults. You need to explain that now you > >>> need to negotiate Rx metadata to use mark, flag and tunnel offloads. > >>> Yes, it will be required on sfc and cnxk only. > >>> As an sfc maintainer I don't mind to change testpmd defaults. > >> > >> If we change testpmd defaults to "do nothing", > >> then we should disable MBUF_FAST_FREE as well. > > > > if you see MBUF_FAST_FREE, it does nothing. Actually, > > !MBUF_FAST_FREE is doing more work. > > > > > >> > >> >
