31/01/2023 17:17, Jerin Jacob: > On Fri, Jan 27, 2023 at 8:31 PM Thomas Monjalon <tho...@monjalon.net> wrote: > > > > 27/01/2023 11:42, Nithin Kumar Dabilpuram: > > > From: Thomas Monjalon <tho...@monjalon.net> > > > > 27/01/2023 06:02, Nithin Kumar Dabilpuram: > > > > > From: Thomas Monjalon <tho...@monjalon.net> > > > > > > Ferruh is proposing to have a command "port config <port_id> ..." > > > > > > to configure the flags to negotiate. > > > > > > Are you OK with this approach? > > > > > > > > > > Yes, we are fine to have such command to enable and disable the > > > > > feature > > > > > with default being it disabled if supported by PMD. > > > > > Is default being disabled fine if the feature is supported by a PMD ? > > > > > > > > I think the default should be enabled for ease of use. > > > > > > Since testpmd is used extensively for benchmarking purposes, we thought > > > it should have minimum features > > > enabled by default. The default testpmd doesn't have any Rx/Tx offloads > > > enabled(except for FAST FREE), default > > > fwd mode being "iofwd" and the Rx metadata is only referenced when > > > dumping packets. > > > > > > > > > > Do we have similar features disables by default? > > > > I mean do we know features in testpmd which require a "double > > > > enablement" > > > > like one configuration command + one rte_flow rule? > > > > > > Spec itself is that way i.e "RTE_FLOW_RULE + RX_METADATA_NEGOTIATE(once)" > > > > > > Isn't it enough if > > > > > > #1 We have enough print when rte_flow is being create without negotiation > > > done and ask user to enable rx metadata using > > > "port config <port_id>..." > > > #2 Provide testpmd app arg to enable Rx metadata(for example " > > > --rx-metadata") like other features to avoid calling another > > > command before rte flow create. > > > > I'm not sure what is best. > > I will let others discuss this part. > > IMO, enabling something always defeat the purpose to negotiate. IMO, > someone needs to negotiate > for a feature if the feature is needed. I think, the double enablement > is part of the spec itself. In case, The PMD > drivers won't like double enablement, no need to implement the PMD > callback. That way, there is no change in the existing flow. > > The reason why cnxk driver thought of leveraging negotiate() feature > so that it helps for optimization. e.s.p > function template for multiprocess case as we know what features > needed in fastpath upfront. > > If there still concerns with patch we can take up this to TB decide > one way or another to make forward progress. Let us know.
Ferruh, Andrew, Ori, Ivan, what is your opinion? Let's progress with this patch to make it in -rc1.
