Re: [dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk() and rte_pktmbuf_free_bulk()

["Wiles, Roger Keith" <keith.wiles@windriver.com>]

Hi Bruce,

Do I need to reject the for the new routines or just make sure the vector driver does not get updated to use those routines?

Thanks
++Keith

On Oct 6, 2014, at 3:56 AM, Richardson, Bruce <bruce.richardson@intel.com> wrote:

> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Keith Wiles
>> Sent: Sunday, October 05, 2014 12:10 AM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH 2/2] Adding the routines rte_pktmbuf_alloc_bulk()
>> and rte_pktmbuf_free_bulk()
>> 
>> Minor helper routines to mirror the mempool routines and remove the code
>> from applications. The ixgbe_rxtx_vec.c routine could be changed to use
>> the ret_pktmbuf_alloc_bulk() routine inplace of rte_mempool_get_bulk().
>> 
> 
> I believe such a change would cause a performance regression, as the extra init code in the alloc_bulk() function would take additional cycles and is not needed. The vector routines use the mempool function directly, so that there is no overhead of mbuf initialization, as the vector routines use their additional "knowledge" of what the mbufs will be used for to init them in a faster manner than can be done inside the mbuf library.
> 
> /Bruce
> 
>> Signed-off-by: Keith Wiles <keith.wiles@windriver.com>
>> ---
>> lib/librte_mbuf/rte_mbuf.h | 77
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 77 insertions(+)
>> 
>> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
>> index 1c6e115..f298621 100644
>> --- a/lib/librte_mbuf/rte_mbuf.h
>> +++ b/lib/librte_mbuf/rte_mbuf.h
>> @@ -546,6 +546,41 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf
>> *m)
>> }
>> 
>> /**
>> + * @internal Allocate a list of mbufs from mempool *mp*.
>> + * The use of that function is reserved for RTE internal needs.
>> + * Please use rte_pktmbuf_alloc_bulk().
>> + *
>> + * @param mp
>> + *   The mempool from which mbuf is allocated.
>> + * @param m_list
>> + *   The array to place the allocated rte_mbufs pointers.
>> + * @param cnt
>> + *   The number of mbufs to allocate
>> + * @return
>> + *   - 0 if the number of mbufs allocated was ok
>> + *   - <0 is an ERROR.
>> + */
>> +static inline int __rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp, struct
>> rte_mbuf *m_list[], int cnt)
>> +{
>> +     struct rte_mbuf *m;
>> +     int             ret;
>> +
>> +     ret = rte_mempool_get_bulk(mp, (void **)m_list, cnt);
>> +     if ( ret == 0 ) {
>> +             int             i;
>> +             for(i = 0; i < cnt; i++) {
>> +                     m = *m_list++;
>> +#ifdef RTE_MBUF_REFCNT
>> +                     rte_mbuf_refcnt_set(m, 1);
>> +#endif /* RTE_MBUF_REFCNT */
>> +                     rte_pktmbuf_reset(m);
>> +             }
>> +             ret = cnt;
>> +     }
>> +     return ret;
>> +}
>> +
>> +/**
>>  * Allocate a new mbuf from a mempool.
>>  *
>>  * This new mbuf contains one segment, which has a length of 0. The pointer
>> @@ -671,6 +706,32 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
>> }
>> 
>> /**
>> + * Allocate a list of mbufs from a mempool into a mbufs array.
>> + *
>> + * This mbuf list contains one segment per mbuf, which has a length of 0. The
>> pointer
>> + * to data is initialized to have some bytes of headroom in the buffer
>> + * (if buffer size allows).
>> + *
>> + * The routine is just a simple wrapper routine to reduce code in the application
>> and
>> + * provide a cleaner API for multiple mbuf requests.
>> + *
>> + * @param mp
>> + *   The mempool from which the mbuf is allocated.
>> + * @param m_list
>> + *   An array of mbuf pointers, cnt must be less then or equal to the size of the
>> list.
>> + * @param cnt
>> + *   Number of slots in the m_list array to fill.
>> + * @return
>> + *   - The number of valid mbufs pointers in the m_list array.
>> + *   - Zero if the request cnt could not be allocated.
>> + */
>> +static inline int __attribute__((always_inline))
>> +rte_pktmbuf_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf *m_list[],
>> int16_t cnt)
>> +{
>> +     return __rte_mbuf_raw_alloc_bulk(mp, m_list, cnt);
>> +}
>> +
>> +/**
>>  * Free a segment of a packet mbuf into its original mempool.
>>  *
>>  * Free an mbuf, without parsing other segments in case of chained
>> @@ -708,6 +769,22 @@ static inline void rte_pktmbuf_free(struct rte_mbuf
>> *m)
>>      }
>> }
>> 
>> +/**
>> + * Free a list of packet mbufs back into its original mempool.
>> + *
>> + * Free a list of mbufs by calling rte_pktmbuf_free() in a loop as a wrapper
>> function.
>> + *
>> + * @param m_list
>> + *   An array of rte_mbuf pointers to be freed.
>> + * @param npkts
>> + *   Number of packets to free in list.
>> + */
>> +static inline void rte_pktmbuf_free_bulk(struct rte_mbuf *m_list[], int16_t
>> npkts)
>> +{
>> +     while(npkts--)
>> +             rte_pktmbuf_free(*m_list++);
>> +}
>> +
>> #ifdef RTE_MBUF_REFCNT
>> 
>> /**
>> --
>> 2.1.0
> 

Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533