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