On 23.02.2016 08:56, Xie, Huawei wrote: > On 2/22/2016 6:16 PM, Thomas Monjalon wrote: >> 2016-02-22 02:07, Xie, Huawei: >>> On 2/19/2016 5:05 PM, Ilya Maximets wrote: >>>> On 19.02.2016 11:36, Xie, Huawei wrote: >>>>> On 2/19/2016 3:10 PM, Yuanhan Liu wrote: >>>>>> On Fri, Feb 19, 2016 at 09:32:43AM +0300, Ilya Maximets wrote: >>>>>>> Signed-off-by: Ilya Maximets <i.maximets at samsung.com> >>>>>>> --- >>>>>>> doc/guides/prog_guide/thread_safety_dpdk_functions.rst | 1 + >>>>>>> 1 file changed, 1 insertion(+) >>>>>>> >>>>>>> diff --git a/doc/guides/prog_guide/thread_safety_dpdk_functions.rst >>>>>>> b/doc/guides/prog_guide/thread_safety_dpdk_functions.rst >>>>>>> index 403e5fc..13a6c89 100644 >>>>>>> --- a/doc/guides/prog_guide/thread_safety_dpdk_functions.rst >>>>>>> +++ b/doc/guides/prog_guide/thread_safety_dpdk_functions.rst >>>>>>> The mempool library is based on the DPDK lockless ring library and >>>>>>> therefore is also multi-thread safe. >>>>>>> +rte_vhost_enqueue_burst() is also thread safe because based on >>>>>>> lockless ring-buffer algorithm like the ring library. >>>>>> FYI, Huawei meant to make rte_vhost_enqueue_burst() not be thread-safe, >>>>>> to aligh with the usage of rte_eth_tx_burst(). >>>>>> >>>>>> --yliu >>>>> I have a patch to remove the lockless enqueue. Unless there is strong >>>>> reason, i prefer vhost PMD to behave like other PMDs, with no internal >>>>> lockless algorithm. In future, for people who really need it, we could >>>>> have dynamic/static switch to enable it. >>> Thomas, what is your opinion on this and my patch removing lockless enqueue? >> The thread safety behaviour is part of the API specification. >> If we want to enable/disable such behaviour, it must be done with an API >> function. But it would introduce a conditional statement in the fast path. >> That's why the priority must be to keep a simple and consistent behaviour >> and try to build around. An API complexity may be considered only if there >> is a real (measured) gain. > > Let us put the gain aside temporarily. I would do the measurement. > Vhost is wrapped as a PMD in Tetsuya's patch. And also in DPDK OVS's > case, it is wrapped as a vport like all other physical ports. The DPDK > app/OVS will treat all ports equally.
That is not true. Currently vhost in Open vSwitch implemented as a separate netdev class. So, to use concurrency of vhost we just need to remove 2 lines (rte_spinlock_lock and rte_spinlock_unlock) from function __netdev_dpdk_vhost_send(). This will not change behaviour of other types of ports. > It will add complexity if the app > needs to know that some supports concurrency while some not. Since all > other PMDs doesn't support thread safety, it doesn't make sense for > vhost PMD to support that. I believe the APP will not use that behavior. >>From the API's point of view, if we previously implemented it wrongly, > we need to fix it as early as possible.
