Re: [dpdk-dev] [PATCH] eventdev: fix Rx adapter stalls on event device backpressure

["Kundapura, Ganapati" <ganapati.kundapura@intel.com>]

Hi Mattias,

> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: 09 November 2021 13:58
> To: Kundapura, Ganapati <ganapati.kundapura@intel.com>;
> jerinj@marvell.com; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: Re: [PATCH] eventdev: fix Rx adapter stalls on event device
> backpressure
> 
> On 2021-11-09 07:26, Kundapura, Ganapati wrote:
> > Hi Mattias,
> >
> >> -----Original Message-----
> >> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >> Sent: 08 November 2021 19:14
> >> To: jerinj@marvell.com; Jayatheerthan, Jay
> >> <jay.jayatheerthan@intel.com>
> >> Cc: dev@dpdk.org; Kundapura, Ganapati
> <ganapati.kundapura@intel.com>;
> >> stable@dpdk.org
> >> Subject: Re: [PATCH] eventdev: fix Rx adapter stalls on event device
> >> backpressure
> >>
> >> On 2021-11-08 14:25, Mattias Rönnblom wrote:
> >>> In the Eventdev Ethernet RX Adapter, correctly handle the case where
> >>> the circular enqueue buffer head and tail index points to the same
> >>> element (i.e., the buffer is full) and the buffer has wrapped.
> >>>
> >>> This bug may be triggered in case there is backpressure from the
> >>> event device to the RX adapter.
> >>>
> >>> Fixes: 8113fd15e229 ("eventdev/eth_rx: make enqueue buffer
> >>> circular")
> >>> Cc: ganapati.kundapura@intel.com
> >>> Cc: stable@dpdk.org
> >>
> >> Disregard the stable cc. This bug does not appear in any released
> >> DPDK version (e.g., 21.08).
> >>
> >>
> >>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>> ---
> >>>    lib/eventdev/rte_event_eth_rx_adapter.c | 22 ++++++++++++++------
> --
> >>>    1 file changed, 14 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> >> b/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> index 56318b5a6f..809416d9b7 100644
> >>> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> @@ -777,19 +777,25 @@ rxa_flush_event_buffer(struct
> >> event_eth_rx_adapter *rx_adapter,
> >>>    		       struct eth_event_enqueue_buffer *buf,
> >>>    		       struct rte_event_eth_rx_adapter_stats *stats)
> >>>    {
> >>> -	uint16_t count = buf->last ? buf->last - buf->head : buf->count;
> >>> +	uint16_t count = buf->count;
> >>> +	uint16_t n = 0;
> >>>
> >>>    	if (!count)
> >>>    		return 0;
> >>>
> >>> -	uint16_t n = rte_event_enqueue_new_burst(rx_adapter-
> >>> eventdev_id,
> >>> -					rx_adapter->event_port_id,
> >>> -					&buf->events[buf->head],
> >>> -					count);
> >>> -	if (n != count)
> >>> -		stats->rx_enq_retry++;
> >>> +	if (buf->last)
> >>> +		count = buf->last - buf->head;
> >>> +
> >>> +	if (count) {
> >>> +		n = rte_event_enqueue_new_burst(rx_adapter-
> >>> eventdev_id,
> >>> +						rx_adapter->event_port_id,
> >>> +						&buf->events[buf->head],
> >>> +						count);
> >>> +		if (n != count)
> >>> +			stats->rx_enq_retry++;
> >>>
> >>> -	buf->head += n;
> >>> +		buf->head += n;
> >>> +	}
> >>>
> >>>    	if (buf->last && n == count) {
> >>>    		uint16_t n1;
> > When head = tail, count is the number of events in the event buffer
> > i.e count = buf->count and last = 0 Last is the marker used in case of roll
> over.
> > In case of tail roll over and head is not, events are processed from head to
> last and zero to tail.
> > Looks like change is same as the original.
> > Could you please clarify more on this change and also clarify if you were
> able to reproduce the backpressure issue?
> 
> 
> The enqueue buffer state I encountered was last != 0 and head == tail, and
> size != 0. In that case, the function returns early, since count == 0, even
> though there are events stored from 0 to tail. head, tail, last and size were all
> 192, from what I remember.
> 
Issue seems to be head is catching up with last not tail when the tail is rolled over.
When head = last, count is zero and returning without processing the events from 0 to tail.
Approved. You can add my ack

> 
> For reasons I didn't analyze, it only seem to occur when the event port's
> enqueue burst size was larger than 32 (the RX burst used against the RX
> adapter's ethdev queues), and there was backpressure from the event
> device.
>