Re: [dpdk-dev] [PATCH v2 5/6] common_linuxapp: Added CONFIG_RTE_ETHDEV_LRO_SUPPORT option

[Vladislav Zolotarov <vladz@cloudius-systems.com>]

On Mar 5, 2015 9:14 PM, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:
>
> 2015-03-05 16:18, Vlad Zolotarov:
> >
> > On 03/05/15 16:01, Thomas Monjalon wrote:
> > > 2015-03-05 15:39, Vlad Zolotarov:
> > >> On 03/05/15 15:19, Thomas Monjalon wrote:
> > >>> 2015-03-05 13:28, Vlad Zolotarov:
> > >>>> Enables LRO support in PMDs.
> > >>>>
> > >>>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
> > >>>> ---
> > >>>>    config/common_linuxapp | 1 +
> > >>>>    1 file changed, 1 insertion(+)
> > >>>>
> > >>>> diff --git a/config/common_linuxapp b/config/common_linuxapp
> > >>>> index 97f1c9e..5b98595 100644
> > >>>> --- a/config/common_linuxapp
> > >>>> +++ b/config/common_linuxapp
> > >>>> @@ -137,6 +137,7 @@ CONFIG_RTE_MAX_ETHPORTS=32
> > >>>>    CONFIG_RTE_LIBRTE_IEEE1588=n
> > >>>>    CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> > >>>>    CONFIG_RTE_ETHDEV_RXTX_CALLBACKS=y
> > >>>> +CONFIG_RTE_ETHDEV_LRO_SUPPORT=y
> > >>> Sorry I don't really follow this ixgbe discussion but I wonder why
you
> > >>> would add a compile time option for this feature.
> > >> The only reason is to be able to detect that the feature is present
in
> > >> the DPDK version u r compiling against because of the API change.
> > >> Currently, this can't be done using the DPDK version thus we may
either
> > > Why you cannot use version? In development phase?
> > > When released, you'll be able to test >= 2.1.
> >
> > Of course! When the version bumps, the #ifdef I've mentioned above may
> > be replaced with the version check.
> >
> > >
> > >> do a try-compilation and if it fails define some application-level
macro
> > >> disabling
> > >> the feature usage or we may define a macro in the library level
> > >> (together with tons of other such macros like those in the patch
snippet
> > >> above).
> > > I'd prefer a request rte_eth_dev_info_get() to advertise that the
feature
> > > is available with the device and driver.
> > > Please let's try to remove config options and #ifdefs.
> >
> > This is exactly what my patch does. But that's not ending there - there
> > is a new feature bit added in rte_eth_rxmode (enable_lro) and in order
> > to compile the application has to know somehow if this bit is present or
> > not. How do u propose to do this now?
>
> I think it would be better to define something like RTE_HAS_LRO in
rte_ethdev.h.

Ok. So i'll change it in v4.

>
> > Of course, I can put such macro in my own tree but then I'll have to
> > rebase all the time and inform other developers that will have to work
> > against my tree (and not upstream as it's supposed to be) - to update.
> > This sounds like a hassle thus I added this macro to resolve this issue
> > until the version is bumped.
> >
> > >
> > >>> What is the benefit of disabling it?
> > >> No benefit whatsoever.
>
>