[dpdk-dev] [PATCH v2] rte_mbuf: mbuf bulk alloc/free functions added + unittest

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier MATZ
> Sent: Thursday, March 19, 2015 8:41 AM
> To: Neil Horman; vadim.suraev at gmail.com
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] rte_mbuf: mbuf bulk alloc/free functions added + unittest
> 
> Hi Neil,
> 
> On 03/18/2015 09:58 PM, Neil Horman wrote:
> >> +/**
> >> + * Free a bulk of mbufs into its original mempool.
> >> + * This function assumes:
> >> + * - refcnt equals 1
> >> + * - mbufs are direct
> >> + * - all mbufs must belong to the same mempool
> >> + *
> >> + * @param mbufs
> >> + *    Array of pointers to mbuf
> >> + * @param count
> >> + *    Array size
> >> + */
> >> +static inline void rte_pktmbuf_bulk_free(struct rte_mbuf **mbufs,
> >> +					 unsigned count)
> >> +{
> >> +	unsigned idx;
> >> +
> >> +	RTE_MBUF_ASSERT(count > 0);
> >> +
> >> +	for (idx = 0; idx < count; idx++) {
> >> +		RTE_MBUF_ASSERT(mbufs[idx]->pool == mbufs[0]->pool);
> >> +		RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 1);
> >> +		rte_mbuf_refcnt_set(mbufs[idx], 0);
> > This is really a misuse of the API.  The entire point of reference counting is
> > to know when an mbuf has no more references and can be freed.  By forcing all
> > the reference counts to zero here, you allow the refcnt infrastructure to be
> > circumvented, causing memory leaks.
> >
> > I think what you need to do here is enhance the underlying pktmbuf interface
> > such that an rte_mbuf structure has a destructor method association with it
> > which is called when its refcnt reaches zero.  That way the
> > rte_pktmbuf_bulk_free function can just decrement the refcnt on each
> > mbuf_structure, and the pool as a whole can be returned when the destructor
> > function discovers that all mbufs in that bulk pool are freed.
> 
> I don't really understand what's the problem here. The API explicitly
> describes the conditions for calling this functions: the segments are
> directs, they belong to the same mempool, and their refcnt is 1.
> 
> This function could be useful in a driver which knows that the mbuf
> it allocated matches this conditions. I think an application that
> only uses direct mbufs and one mempool could also use this function.

I also don't see anything wrong with that function.
As long, as user makes sure that all mbufs in the bulk satisfy the required conditions it should be ok, I think.
Of course, it's usage is limited, but I suppose the author has some scenario in mind, when introduced it.

Konstantin

> 
> Regards,
> Olivier