Re: [dpdk-dev] [PATCH] parray: introduce internal API for dynamic arrays

["Morten Brørup" <mb@smartsharesystems.com>]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Friday, 18 June 2021 12.47
> 
> On 6/18/2021 10:14 AM, Morten Brørup wrote:
> > Another thing:
> >
> > I just noticed that struct rte_eth_dev_data has "void **rx_queues;"
> (and similarly for tx_queues).
> >
> > That should be "void *rx_queues[RTE_MAX_QUEUES_PER_PORT];", like in
> all the other ethdev structures.
> >
> 
> Why have a fixed size rx_queues array? It is already allocated
> dynamically in
> 'rte_eth_dev_configure()' based on actual Rx/Tx queue number.

For performance reasons, I guess.

> 
> We are already trying to get rid of compile time fixed array sizes, so
> I think
> better to keep it as it is.
> 
> Also this will increase the strcut size.
> 
> > The same structure even has "uint8_t
> rx_queue_state[RTE_MAX_QUEUES_PER_PORT];", so it's following two
> different conventions.
> >
> 
> I wonder if we should should switch these to dynamic allocation too.
> 

I agree that we should generally stick with one or the other, either fixed or dynamically allocated arrays, for consistency reasons. However, sometimes performance trumps consistency. :-)

If we change more per-queue arrays to dynamic allocation, perhaps it would be beneficial gathering these fields into one per-queue struct, so the struct rte_eth_dev_data only has one array instead of multiple arrays. It depends on how they are being used. (Also, maybe there should be one array in each direction, so it is two arrays, not just one.)