Re: Question about store_return() in ~/dpdk/lib/librte_distributor/rte_distributor.c

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Oct 07, 2015 at 06:07:23PM +0900, 최익성 wrote:
> Dear DPDK experts.
>  
> Thank you very much for your excellent efforts and contributions.
>  
> I have a question about store_return() in ~/dpdk/lib/librte_distributor/rte_distributor.c
>  
> The store_return() function adds a oldbuf packet to d->returns.mbuf[] queue.
>  
> If the queue is full and the oldbuf packet is NULL, the queue seems to lost a packet in the queue without modifying ret_start/ret_count values.
>  
> Is the last position of the queue always empty?
>  
> Would you check it? 
>  
>  
> #define RTE_DISTRIB_MAX_RETURNS 128
> #define RTE_DISTRIB_RETURNS_MASK (RTE_DISTRIB_MAX_RETURNS - 1)
>  
> /* stores a packet returned from a worker inside the returns array */
> static inline void
> store_return(uintptr_t oldbuf, struct rte_distributor *d,
>                 unsigned *ret_start, unsigned *ret_count)
> {
>         /* store returns in a circular buffer - code is branch-free. buffer 끝에 oldbuf를 추가함 */
>         d->returns.mbufs[(*ret_start + *ret_count) & RTE_DISTRIB_RETURNS_MASK]
>                         = (void *)oldbuf;
>         *ret_start += (*ret_count == RTE_DISTRIB_RETURNS_MASK) & !!(oldbuf);
>         *ret_count += (*ret_count != RTE_DISTRIB_RETURNS_MASK) & !!(oldbuf);
> }
>  
> If d->returns.mbufs[] queue is full, oldbuf replaces the first cell of the queue (new packet overwrites the last packet in the queue).
>  
> ret_start is preserved if the queue is not full (count!= MAX_VALUE(RTE_DISTRIB_RETURNS_MASK))
> if ret_start is MAX value and oldbuf is not NULL, ret_start is incremented by 1.
>  
> ret_count is incremented by 1 if it is not MAX value and oldbuf is not NULL).
> if ret_count is MAX value, ret_count is preserved.
>  
> The mbufs queue is written by oldbuf(NULL) even though when the queue is full and the oldbuf packet is NULL (no packet insertion). 
> It may lost a cell in the queue.
> If the last position of the queue is always empty, it may not be a problem.
>  
> Would you check it?
>  
> Thank you very much.
>  
> Sincerely Yours,
>  
> Ick-Sung Choi.
> 

Hi,

yes, it can overwrite entries in that array.

When designing the library, there were two possible use cases taken into account:
1. The user did not want to return the packets to the distributor but pass them
elsewhere from the workers when done. In this case, the overwriting did not matter.
2. The second case is where ordering is to be preserved, and here the user does
want to pass them back to the distributor. To accomodate this, the buffer size
is made considerably bigger than the default burst size, so that anyone who
polls the ring after each burst (or even after each second burst) should again
have no issues and avoid problems with overwriting the entry.

Regards,
/Bruce