> -----Original Message----- > From: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com> > Sent: Wednesday, May 26, 2021 2:02 PM > To: Van Haaren, Harry <harry.van.haa...@intel.com>; bugzi...@dpdk.org; > dev@dpdk.org; Rao, Nikhil <nikhil....@intel.com>; Jayatheerthan, Jay > <jay.jayatheert...@intel.com>; heng.w...@ericsson.com > Subject: RE: [dpdk-dev] [Bug 721] Wrong event pointer in rx adapter > > >> From: dev <dev-boun...@dpdk.org> On Behalf Of bugzi...@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
