Re: [dpdk-dev] [PATCH] mempool: introduce flag to indicate hw mempool

[Olivier MATZ <olivier.matz@6wind.com>]

On Tue, 04 Apr 2017 08:58:40 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2017-04-04 11:05, Hemant Agrawal:
> > Hi Olivier,
> > 
> > On 4/3/2017 8:49 PM, Olivier Matz wrote:  
> > > Hi Hemant,
> > >
> > > On Mon, 3 Apr 2017 14:42:09 +0530, Hemant Agrawal
> > > <hemant.agrawal@nxp.com> wrote:  
> > >> Hardware pools need to distinguish between buffers allocated
> > >> using software or hardware backed pools.
> > >>
> > >> Some HW NICs may choose to autonomously free the pickets during
> > >> transmit if the packet is from HW pool. While they should not do
> > >> it for software backed pools.
> > >>
> > >> Such flag would also help when multiple pools are being handled
> > >> by a PMD, saving costly compare operations for any internal
> > >> marker.
> > >>
> > >> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> > >> ---
> > >>  lib/librte_mempool/rte_mempool.h | 5 +++++
> > >>  1 file changed, 5 insertions(+)
> > >>
> > >> diff --git a/lib/librte_mempool/rte_mempool.h
> > >> b/lib/librte_mempool/rte_mempool.h index 991feaa..91dbd21 100644
> > >> --- a/lib/librte_mempool/rte_mempool.h
> > >> +++ b/lib/librte_mempool/rte_mempool.h
> > >> @@ -263,6 +263,11 @@ struct rte_mempool {
> > >>  #define MEMPOOL_F_SC_GET         0x0008 /**< Default get is
> > >> "single-consumer".*/ #define MEMPOOL_F_POOL_CREATED
> > >> 0x0010 /**< Internal: pool is created. */ #define
> > >> MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically
> > >> contiguous objs. */ +#define MEMPOOL_F_HW_POOL        (1 <<
> > >> ((sizeof(int) * 8) - 1)) /**< Internal:
> > >> +	* Hardware offloaded pool. This information may be used
> > >> by the
> > >> +	* NIC or other hw. Some NICs autonomously free the HW
> > >> backed pool packets. */ +
> > >> +/**< Don't need physically contiguous objs. */
> > >>
> > >>  /**
> > >>   * @internal When debug is enabled, store some statistics.  
> > >
> > >
> > > One thing is still not clear to me: in your driver, you check
> > > this flag:
> > > - if it is unset, you reallocate a packet from your hw pool, you
> > > copy some metadata, and you send it to the hw.
> > > - if it is set, you assume that you can call mempool_to_bpid(mp)
> > > and directly send it to the hw.
> > >
> > > I think this is not correct. The test you want to do in your
> > > driver is: "is it the pool that I registered for my hardware"?
> > > It is not:
> > > "is it a hardware managed pool?".
> > > I think what you are doing here prevents to use 2 hardware
> > > mempools at the same time, because they would all have this flag,
> > > and mempool_to_bpid() would probably crash.
> > >  
> > 
> > No, I am only trying to differentiate between hw and software pool 
> > packets. I don't see a possiblity of having two different
> > orthogonal hw mempool types working in the system. At any point of
> > time when you are running DPDK on a particular type of hardware,
> > you will only have *one* type of hardware backed pools in your
> > implementation.  The number of mempool instances may be many but
> > all will able to work with mempool_to_bpid().  
> 
> No you could have different HW mempools on one system.
> Please imagine PCI NICs which provide a mempool.
> (other argument: never say never ;)
> 
> > The application may send packet allocated from a *ring* pool
> > instead of using "hw" pool.
> > 
> > So, it is sufficient to just check if the pool is offloaded or not.
> > HW can take care of all the supported pools.
> >   
> > > Instead, can't you just compare the mempool pointer to a value
> > > stored internally in the driver?  
> > 
> > There can be more than one instance of mempool, the driver is
> > capable of supporting multiple hw offloaded mempools. Each dpaa2
> > PMD port may have different mempool instance registered.
> > 
> > So, pointer comparison is not practical unless I start storing the 
> > mempool driver pointer.  
> 
> Is it difficult to store this pointer?
> 

Another idea which looks even better: what about comparing
mempool->ops_index to a value stored in the driver at init?

I think it describes exactly what you want: the mempool type is *your*
hardware mempool type.