> On 10 Jul 2017, at 12:17, Krishna Garapati <balakrishna.garap...@linaro.org> > wrote: > > > > On 10 July 2017 at 10:13, Elo, Matias (Nokia - FI/Espoo) > <matias....@nokia.com> wrote: > > > + pkt_pool = mbuf_pool_create(pkt_dpdk->pool_name, > > + pool_entry->num, cache, > > + pool_entry->max_seg_len > > + > > + CONFIG_PACKET_HEADROOM, > > + pkt_dpdk); > > instead of passing the whole pkt_dpdk struct, can you just pass odp_pool_t > > & calculate just data_room in the dequeue_bulk ?. This way the pool > > configuration will be more clearer & less dependent on pkt_dpdk. > > > > The value of 'data_room' is constant, so it doesn't make sense to recalculate > it in the fast path every time pool_dequeue_bulk() is called. > mbuf_pool_create() is only used by the dpdk pktio, so the dependency is not a > problem. > I was actually referring to storing whole pkt_dpdk_t in to the > "rte_mempool_set_ops_byname". > I see that it would be clear if we just store odp_pool_t & store or > calculate the data_room part some other means. I agree that it's not good to > calculate data_room in the fast path. >
Using odp_pool_t as the only argument would indeed be clearer. The problems is that the only way to pass arguments to pool_dequeue_bulk() is though rte_mempool struct and storing the 'data_room' there would not be any better. I originally intended to keep the mbuf_pool_create() arguments as close as possible to the rte_pktmbuf_pool_create() function but one option would be to change mbuf_pool_create() to use pool_entry* as argument. -> static struct rte_mempool *mbuf_pool_create(const char *name, pool_t *pool_entry) --> rte_mempool.pool_data = odp_pool_t --> rte_mempool.pool_config = pool_entry * -Matias
