Re: [dpdk-dev] [PATCH] app/testpmd: do not enable Rx offloads by default

[Shahaf Shuler <shahafs@mellanox.com>]

Friday, January 26, 2018 10:36 AM, Thomas Monjalon:
> 26/01/2018 09:06, Lu, Wenzhuo:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > 26/01/2018 08:30, Lu, Wenzhuo:
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > 25/01/2018 02:11, Lu, Wenzhuo:
> > > > > Why these offload flags are silently enabled?
> > > >
> > > > I don't think it's silently. It's a global configuration. In this
> > > > case, testpmd is
> > > the user, it does request it explicitly. If it's not so explicit,
> > > maybe we can consider moving all the configuration to a specific
> configure file.
> > > > Talking about why it's enabled by default. Hard to figure out the history.
> > > To my opinion, the original designer wants to simulate the common case.
> > >
> > > Please do not justify a design mistake by history.
> > >
> > > This is a test tool, so we don't care about the common case.
> > > A test tool should not try to guess the best configuration.
> > > Only the user should decide the configuration to apply, and the
> > > default should be empty, as the API is.
> >
> > I see the divergence. You think testpmd is a tool, so it should be white
> paper as the tool users may suppose it is.
> > I think testpmd is an example to let the APP developers know how to use
> DPDK. So any pre-configuration is acceptable.

If so, then what this patch shows that it is not such a good example to application writers. 
Application should not set offloads that are not supported by the PMD. Pre-configuration is OK, but adjustment is a must according to the PMD caps. 
I think that many PMD have a wrong behavior due to it. They are forced to silently drop un-supported offload configuration (to enable testpmd to run on top of them).
This will end up, for example, in application which expects to have the VLAN tag in the mbuf (because the configuration passed successfully), while in fact it will be always in the packet headers. 

I didn't change it on my series because I wanted to convert the application "as is" from the old to the new offloads API,  however I do think a change is needed here.

 
> 
> Ah ah ah :)
> No
> Examples are in examples/ directory.