[dpdk-dev] [Bug 721] Wrong event pointer in rx adapter

[Van Haaren, Harry]

> -----Original Message-----
> From: Pavan Nikhilesh Bhagavatula <pbhagavatula at marvell.com>
> Sent: Wednesday, May 26, 2021 2:02 PM
> To: Van Haaren, Harry <harry.van.haaren at intel.com>; bugzilla at dpdk.org;
> dev at dpdk.org; Rao, Nikhil <nikhil.rao at intel.com>; Jayatheerthan, Jay
> <jay.jayatheerthan at intel.com>; heng.wang at ericsson.com
> Subject: RE: [dpdk-dev] [Bug 721] Wrong event pointer in rx adapter
> 
> >> From: dev <dev-bounces at dpdk.org> On Behalf Of bugzilla at dpdk.org
<snip original bug report for brevity>
> >>  797                 if (dropped)
> >>  798                         rx_adapter->stats.rx_dropped += dropped;
> >>  799         }
> >
> >+CC RX Adapter Maintainer, and Pavan as this code has been changed
> >recently.
> >
> >I've done a quick review of the above report, and believe it to be a
> >genuine bug,
> >however this code has been refactored in commit d7c428e557b as part
> >of series
> >to " eventdev: support Rx adapter event vector".
> >
> >The relevant diff here for reference:
> >Note "ev" pointer replaced with "&buf->events[buf->count]"
> >
> >                dropped = 0;
> >                nb_cb = dev_info->cb_fn(eth_dev_id, rx_queue_id,
> >-                                       ETH_EVENT_BUFFER_SIZE, buf->count, ev,
> >-                                       num, dev_info->cb_arg, &dropped);
> >+                                       ETH_EVENT_BUFFER_SIZE, buf->count,
> >+                                       &buf->events[buf->count], num,
> >+                                       dev_info->cb_arg, &dropped);
> >                if (unlikely(nb_cb > num))
> >
> >
> >Based on this investigation, although the code changed, the same bug
> >seems present,
> >as really &buf->events[0] should be passed to the callback?
> 
> The callback semantics are pretty ambiguous. There are two cases:
> 1. Is the callback allowed to modify the entire event buffer?
> Or
> 2. Is it only allowed to modify the events received in the current Rx cycle?
> 
> The current code only takes into account case 2 as handling case 1 requires
> additional modification of nb_cb etc..

Ah, I'm not actually familiar with the rx-adapter code at all, or the callback
semantics/design... I presumed it was the same as Ethdev RX/TX callbacks.

Jay, would you mind taking a look and identifying what the desired behavior is?

It would be good to fix this ASAP as it seems like a genuine issue, and the workaround
of "moving the pointer to where it should be in App code" is... a workaround :) 

> Cheers,
> Pavan.

Thanks for the input Pavan! Regards, -Harry