mempool name size incorrect?

[Andrew Rybchenko]

On 3/11/26 1:52 PM, Konstantin Ananyev wrote:
> 
> 
>>> On 3/10/26 5:10 PM, Morten Brørup wrote:
>>>> Isn't the RTE_MEMPOOL_NAMESIZE too short?
>>>>
>>>> Looking at the names sizes:
>>>>
>>>> RTE_MEMZONE_NAMESIZE = 32,
>>>> RTE_RING_NAMESIZE = RTE_MEMZONE_NAMESIZE - (sizeof("RG_")=4) + 1 =
>>> 29,
>>>> RTE_MEMPOOL_NAMESIZE = RTE_RING_NAMESIZE - (sizeof("MP_")=4) + 1 =
>> 26
>>>>
>>>> Referring to [1], I think it should be fixed as:
>>>> - #define RTE_MEMPOOL_NAMESIZE (RTE_RING_NAMESIZE - \
>>>> 			      sizeof(RTE_MEMPOOL_MZ_PREFIX) + 1)
>>>> + #define RTE_MEMPOOL_NAMESIZE (RTE_MEMZONE_NAMESIZE - \
>>>> 			      sizeof(RTE_MEMPOOL_MZ_PREFIX) + 1)
>>>>
>>>> There is no ring involved, so I guess it is some kind of copy-paste-
>>> search-replace error.
>>>>
>>>
>>> I guess ring is involved in fact since the default mempool driver is
>>> ring.
>>>
>>> See drivers/mempool/ring/rte_mempool_ring.c ring_alloc().
>>>
>>> Yes, it is not ideal, but at least it explains why RTE_RING_NAMESIZE
>>> is used.
>>
>> Thanks, that explains it. Bad layer violation...
>> Let's hope no future mempool driver adds anything longer than "RG_" to the
>> name of any memzone it creates.
>>
>> Looking into the associated string length checks, using a too long name will fail
>> with ENAMETOOLONG.
>> So, using a long mempool name might succeed with some mempool drivers and
>> fail with others. :-(
>>
>> I guess there's no simple fix for that.
>> And I was wrong to think that the RTE_MEMPOOL_NAMESIZE should be
>> increased from 26 to 29.
> 
> As a generic thought: might be it is time to make the length across these
> structs (mempool, ring, etc.) arbitrary?
> At our next big API breakage or so.

Why not, if it is really required

> 
>>>
>>>>
>>>> Looking at the rte_mempool structure [2]:
>>>> struct __rte_cache_aligned rte_mempool {
>>>> 	char name[RTE_MEMPOOL_NAMESIZE]; /**< Name of mempool. */
>>>> 	union {
>>>> 		void *pool_data;         /**< Ring or pool to store
>>> objects. */
>>>> 		uint64_t pool_id;        /**< External mempool identifier.
>>> */
>>>> 	};
>>>>
>>>>
>>>> Due to the 8-byte alignment of the pool_id field following the name
>>> field, fixing the length as suggested doesn't change the memory layout
>>> for 64 bit CPU architectures.
>>>> But it does for 32 bit CPU architectures, which will only 4-byte
>>> align the pool_id field.
>>>>
>>>> [1]: https://elixir.bootlin.com/dpdk/v26.03-
>>> rc1/source/lib/mempool/rte_mempool.h#L128
>>>> [2]: https://elixir.bootlin.com/dpdk/v26.03-
>>> rc1/source/lib/mempool/rte_mempool.h#L230
>>>>
>>>>
>>>> Another thing:
>>>> On 32 bit CPU architectures, the cache_size and local_cache fields in
>>> the rte_mempool structure are not in the same cache line.
>>>> But I guess we don't really care about 32 bit CPU architectures.
>>>>
>>>>
>>>> Venlig hilsen / Kind regards,
>>>> -Morten Brørup
>>>>
>