>On 02/12/2016 03:57 PM, Thomas Monjalon wrote: >> 2016-02-12 13:23, Panu Matilainen: >>> On 02/10/2016 11:18 PM, Keith Wiles wrote: >>>> static inline void *rte_mempool_get_priv(struct rte_mempool *mp) >>>> { >>>> +#ifdef RTE_NEXT_ABI >>>> + return (char *)mp + >>>> + MEMPOOL_HEADER_SIZE(mp, mp->pg_num, mp->cache_size); >>>> +#else >>>> return (char *)mp + MEMPOOL_HEADER_SIZE(mp, mp->pg_num); >>>> +#endif /* RTE_NEXT_ABI */ >>>> } >>> >>> This is not RTE_NEXT_ABI material IMO, the added ifdef clutter is just >>> too much. >> >> The changes are restricted to the mempool files. >> I think it is not so much. However I wonder how much the feature is important >> to justify the use of NEXT_ABI. > >Well yes, to be precise: for the benefit of this patch, the ifdef >clutter seems too much. > >Its not as if every change is expected to go through a NEXT_ABI phase, >based on http://dpdk.org/ml/archives/dev/2016-February/032866.html there >might be some confusion regarding that.
I think the NEXT_ABI is reasonable in this case as it does change a structure everyone uses and the ifdef clutter is caused by having to remove old ifdefs, which is a good thing for DPDK. The NEXT_ABI ifdefs only exist for one release and then they will disappear, which I think is more then reasonable. > >> >>> I'd suggest adding a deprecation notice for the change now and after >>> 16.04 is released, just resend the patch without messing with RTE_NEXT_ABI. >> >> When adding a deprecation notice, it is really better to provide a reference >> to the code change. >> So if you give up with NEXT_ABI, please add a link to this code change in >> the new commit message. Thanks >> > >Nod. > > - Panu - > Regards, Keith