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

[Van Haaren, Harry]

> From: dev <dev-bounces at dpdk.org> On Behalf Of bugzilla at dpdk.org
> Sent: Wednesday, May 26, 2021 9:24 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [Bug 721] Wrong event pointer in rx adapter
> 
> https://bugs.dpdk.org/show_bug.cgi?id=721
> 
>             Bug ID: 721
>            Summary: Wrong event pointer in rx adapter
>            Product: DPDK
>            Version: 20.11
>           Hardware: All
>                 OS: All
>             Status: UNCONFIRMED
>           Severity: major
>           Priority: Normal
>          Component: eventdev
>           Assignee: dev at dpdk.org
>           Reporter: heng.wang at ericsson.com
>   Target Milestone: ---
> 
> Problem:
> In the function rxa_buffer_mbufs the ev is not reset to the array's first
> element before passing to a user call back function.
> 
> Workaround:
> In the user callback, we must decrement ev by number of event before we can use
> it.
> 
> Fix:
> I think we should pass &buf->events[buf->count] to dev_info->cb_fn instead of
> ev.
> 
>  773         for (i = 0; i < num; i++) {
>  774                 m = mbufs[i];
>  775
>  776                 rss = do_rss ?
>  777                         rxa_do_softrss(m, rx_adapter->rss_key_be) :
>  778                         m->hash.rss;
>  779                 ev->event = event;
>  780                 ev->flow_id = (rss & ~flow_id_mask) |
>  781                                 (ev->flow_id & flow_id_mask);
>  782                 ev->mbuf = m;
>  783                 ev++;
>  784         }
>  785
>  786         if (dev_info->cb_fn) {
>  787
>  788                 dropped = 0;
>  789                 nb_cb = dev_info->cb_fn(eth_dev_id, rx_queue_id,
>  790                                         ETH_EVENT_BUFFER_SIZE, buf->count,
> ev,
>  791                                         num, dev_info->cb_arg, &dropped);
>  792                 if (unlikely(nb_cb > num))
>  793                         RTE_EDEV_LOG_ERR("Rx CB returned %d (> %d)
> events",
>  794                                 nb_cb, num);
>  795                 else
>  796                         num = nb_cb;
>  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?

Regards, -Harry