> > > > > 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.
>
> I assume the names are allocated as part of the structures to make them part
> of
> the shared memory, to be searchable by secondary processes.
Yes sure, but we can put the actual name-buffer somewhere else:
let say for the ring at the very end (after the elements array) and
store only a pointer to it in the rte_ring header.
I.E. at ring_create we check name-length - allocate extra 'name-length'
bytes for it, initialize pointer to name with address of this extra bytes.
Probably the same can be done with the mempool, etc.
>
> I guess someone once thought 32 characters would be enough.
>
> Furthermore, these startup-only parts of the structures really should be
> clearly
> marked as such, to ensure that they remain in dedicated cache lines, not mixed
> with hot fields in the same structures.
> As I mentioned below, when building for 32 bit architectures, the cache_size
> field
> - which is accessed by the mempool fastpath functions - is in a cache line
> shared
> with the name, instead of the cache line with the local_cache field pointing
> to the
> local cache; i.e. causing two cache misses instead of one, when getting or
> putting
> objects from/to a mempool cache.
>
> I agree that this should be cleaned up at a future big API breakage.
>
> >
> > > >
> > > > >
> > > > > 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
> > > > >
