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 ! 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). 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