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

[Ivan Boule <ivan.boule@6wind.com>]

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