Re: [dpdk-dev] [PATCH v3 1/3] net/sfc: fix power of 2 round up when align has smaller type

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 7/24/2019 7:41 PM, Andrew Rybchenko wrote:
> On 7/24/19 7:57 PM, Ferruh Yigit wrote:
>> On 7/24/2019 2:16 PM, Andrew Rybchenko wrote:
>>> Substitute driver-defined P2ROUNDUP() h with EFX_P2ROUNDUP()
>>> defined in libefx.
>>>
>>> Cast value and alignment to one specified type to guarantee result
>>> correctness.
>>>
>>> Fixes: e1b944598579 ("net/sfc: build libefx")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> <...>
>>
>>> @@ -29,6 +29,10 @@ extern "C" {
>>>   /* The macro expands divider twice */
>>>   #define	EFX_DIV_ROUND_UP(_n, _d)		(((_n) + (_d) - 1) / (_d))
>>>   
>>> +/* Round value up to the nearest power of two. */
>>> +#define	EFX_P2ROUNDUP(_type, _value, _align)	\
>>> +	(-(-(_type)(_value) & -(_type)(_align)))
>>> +
>> I trust you it does what it says J
>>
>> Just a high level comment, should we have some kind of tools/utilities file in
>> one of the libraries so everyone can use/share them?
> 
> It is the base driver code and it is used in the base driver, so it can't
> rely on any DPDK library. There are RTE_ALIGN_FLOOR/CEIL in
> lib/librte_eal/common/include/rte_common.h.
> 
> Before the patch the macro was defined in efsys.h (i.e. driver specific
> and could use defines available for the driver), but in fact it was
> duplicated in too many drivers and we decided to have it in libefx
> (base driver in DPDK case) itself.
> 

Yes there are lots of small functionalities duplicated in various drivers, that
is why I thought perhaps we can unify them.

And the base driver concern is valid for many drivers I think, since those code
are mostly from a shared delivery, using DPDK specific macros in them will cause
more maintenance cost.