Hi Mattias,
> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnb...@ericsson.com>
> Sent: 09 November 2021 13:58
> To: Kundapura, Ganapati <ganapati.kundap...@intel.com>;
> jer...@marvell.com; Jayatheerthan, Jay <jay.jayatheert...@intel.com>
> Cc: dev@dpdk.org; sta...@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.ronnb...@ericsson.com>
> >> Sent: 08 November 2021 19:14
> >> To: jer...@marvell.com; Jayatheerthan, Jay
> >> <jay.jayatheert...@intel.com>
> >> Cc: dev@dpdk.org; Kundapura, Ganapati
> <ganapati.kundap...@intel.com>;
> >> sta...@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.kundap...@intel.com
> >>> Cc: sta...@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.ronnb...@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.
>
