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

-----Original Message-----
From: Pavan Nikhilesh Bhagavatula <redacted>
Sent: Wednesday, May 26, 2021 2:02 PM
To: Van Haaren, Harry <redacted>; bugzilla@dpdk.org;
dev@dpdk.org; Rao, Nikhil [off-list ref]; Jayatheerthan, Jay
[off-list ref]; heng.wang@ericsson.com
Subject: RE: [dpdk-dev] [Bug 721] Wrong event pointer in rx adapter

quoted

quoted

From: dev <redacted> On Behalf Of bugzilla@dpdk.org

<snip original bug report for brevity>

quoted

quoted

 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