Re: [PATCH] mempool: introduce flag to indicate hw mempool

[Thomas Monjalon <thomas.monjalon@6wind.com>]

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?