[dpdk-dev] [PATCH v9 1/8] ethdev: use locks to protect Rx/Tx callback lists

Ananyev, Konstantin konstantin.ananyev at intel.com
Wed Jun 15 14:40:16 CEST 2016


Hi Ivan,

> -----Original Message-----
> From: Ivan Boule [mailto:ivan.boule at 6wind.com]
> Sent: Wednesday, June 15, 2016 1:15 PM
> To: Thomas Monjalon; Ananyev, Konstantin
> Cc: Pattan, Reshma; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v9 1/8] ethdev: use locks to protect Rx/Tx callback lists
> 
> On 06/15/2016 10:48 AM, Thomas Monjalon wrote:
> 
> >>
> >>> I think the read access would need locking but we do not want it
> >>> in fast path.
> >>
> >> I don't think it would be needed.
> >> As I said - read/write interaction didn't change from what we have right now.
> >> But if you have some particular scenario in mind that you believe would cause
> >> a race condition - please speak up.
> >
> > If we add/remove a callback during a burst? Is it possible that the next
> > pointer would have a wrong value leading to a crash?
> > Maybe we need a comment to state that we should not alter burst
> > callbacks while running burst functions.
> >
> 
> Hi Reshma,
> 
> You claim that the "rte_eth_rx_cb_lock" does not need to be hold in the
> function "rte_eth_rx_burst()" while parsing the "post_rx_burst_cbs" list
> of RX callbacks associated with the polled RX queue to safely remove RX
> callback(s) in parallel.
> The problem is not [only] with the setting and the loading of "cb->next"
> that you assume to be atomic operations, which is certainly true on most
> CPUs.
> I see the 2 important following issues:
> 
> 1) the "rte_eth_rxtx_callback" data structure associated with a removed
> RX callback could still be accessed in the callback parsing loop of the
> function "rte_eth_rx_burst()" after having been freed in parallel.
> 
> BUT SUCH A BAD SITUATION WILL NOT CURRENTLY HAPPEN, THANKS TO THE NICE
> MEMORY LEAK BUG in the function "rte_eth_remove_rx_callback()"  that
> does not free the "rte_eth_rxtx_callback" data structure associated with
> the removed callback !

Yes, though it is documented behaviour, someone can probably
refer it as a feature, not a bug ;)

> 
> 2) As a consequence of 1), RX callbacks can be invoked/executed
> while/after being removed.
> If the application must free resources that it dynamically allocated to
> be used by the RX callback being removed, how to guarantee that the last
> invocation of that RX callback has been completed and that such a
> callback will never be invoked again, so that the resources can safely
> be freed?
> 
> This is an example of a well-known more generic object deletion problem
> which must arrange to guarantee that a deleted object is not used and
> not accessible for use anymore before being actually deleted (freed, for
> instance).

Yes, and as I wrote in other mail, IMO it needs to be addressed.
But again it is already existing problem in rte_ethdev,
and I think it shouldn't stop pdump integration.
Konstantin

> 
> Note that a lock cannot be used in the execution path of the
> rte_eth_rx_burst() function to address this issue, as locks MUST NEVER
> be introduced in the RX/TX path of the DPDK framework.
> 
> Of course, the same issues stand for TX callbacks.
> 
> Regards,
> Ivan
> 
> 
> 
> --
> Ivan Boule
> 6WIND Development Engineer


More information about the dev mailing list