[dpdk-dev] [Bug 721] Wrong event pointer in rx adapter
Pavan Nikhilesh Bhagavatula
pbhagavatula at marvell.com
Wed May 26 15:02:09 CEST 2021
>> 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://urldefense.proofpoint.com/v2/url?u=https-
>3A__bugs.dpdk.org_show-5Fbug.cgi-3Fid-
>3D721&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB
>-fmvgGV3o-
>g_fjLhk5Pupi9ijohpc&m=P7yStZJEWQK5Q4wQOaFcDFtRT_BqBTTBJSfc-
>d8-qUs&s=AqT8tJKXHif2z3dq6jjWuj00vlMwfzWGGv9LICSVwjY&e=
>>
>> 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?
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..
>
>Regards, -Harry
Cheers,
Pavan.
More information about the dev
mailing list