[dpdk-dev] API feature check _HAS_

[dpdk-dev] API feature check _HAS_

[Thomas Monjalon]

When introducing LRO, Vlad has defined the macro RTE_ETHDEV_HAS_LRO_SUPPORT:
http://dpdk.org/browse/dpdk/commit/lib/librte_ether/rte_ethdev.h?id=8eecb329

It allows to use the feature without version check (before the release or
after a backport).
Do you think it is useful?
Should we define other macros RTE_[API]_HAS_[FEATURE] for each new feature
or API change?
It's time to fix it before releasing the 2.2 version.

Re: [dpdk-dev] API feature check _HAS_

[Vlad Zolotarov]

On 11/26/15 22:35, Thomas Monjalon wrote:
> When introducing LRO, Vlad has defined the macro RTE_ETHDEV_HAS_LRO_SUPPORT:
> http://dpdk.org/browse/dpdk/commit/lib/librte_ether/rte_ethdev.h?id=8eecb329
>
> It allows to use the feature without version check (before the release or
> after a backport).
> Do you think it is useful?
> Should we define other macros RTE_[API]_HAS_[FEATURE] for each new feature
> or API change?

The main purpose of the above macro was to identify the presence of the 
new field in the rte_eth_rxmode during the
period of time when there was no other way to know it. Once this may be 
concluded based on the release version I see no
reason to keep it.

> It's time to fix it before releasing the 2.2 version.

Re: [dpdk-dev] API feature check _HAS_

[Gleb Natapov]

On Sun, Nov 29, 2015 at 11:07:44AM +0200, Vlad Zolotarov wrote:
> 
> 
> On 11/26/15 22:35, Thomas Monjalon wrote:
> >When introducing LRO, Vlad has defined the macro RTE_ETHDEV_HAS_LRO_SUPPORT:
> >http://dpdk.org/browse/dpdk/commit/lib/librte_ether/rte_ethdev.h?id=8eecb329
> >
> >It allows to use the feature without version check (before the release or
> >after a backport).
> >Do you think it is useful?
> >Should we define other macros RTE_[API]_HAS_[FEATURE] for each new feature
> >or API change?
> 
> The main purpose of the above macro was to identify the presence of the new
> field in the rte_eth_rxmode during the
> period of time when there was no other way to know it. Once this may be
> concluded based on the release version I see no
> reason to keep it.
> 
Concluding things based on release version does not work so well for
back ports.

> >It's time to fix it before releasing the 2.2 version.
> 

--
			Gleb.

Re: [dpdk-dev] API feature check _HAS_

[Vlad Zolotarov]

On 11/29/15 11:10, Gleb Natapov wrote:
> On Sun, Nov 29, 2015 at 11:07:44AM +0200, Vlad Zolotarov wrote:
>>
>> On 11/26/15 22:35, Thomas Monjalon wrote:
>>> When introducing LRO, Vlad has defined the macro RTE_ETHDEV_HAS_LRO_SUPPORT:
>>> http://dpdk.org/browse/dpdk/commit/lib/librte_ether/rte_ethdev.h?id=8eecb329
>>>
>>> It allows to use the feature without version check (before the release or
>>> after a backport).
>>> Do you think it is useful?
>>> Should we define other macros RTE_[API]_HAS_[FEATURE] for each new feature
>>> or API change?
>> The main purpose of the above macro was to identify the presence of the new
>> field in the rte_eth_rxmode during the
>> period of time when there was no other way to know it. Once this may be
>> concluded based on the release version I see no
>> reason to keep it.
>>
> Concluding things based on release version does not work so well for
> back ports.

In that case the existing applications won't be able to enjoy the 
feature with the older releases with the backport - that's true.
Having this flag has it's benefits (e.g. the corresponding ifdefs are 
much more readable), however to be consistent we'd rather define this 
type of flags
for other features too like Thomas wrote above. I'm not against this 
approach too...

>
>>> It's time to fix it before releasing the 2.2 version.
> --
> 			Gleb.