Hi David, Best, Ori
> -----Original Message----- > From: David Marchand <david.march...@redhat.com> > Sent: Friday, May 26, 2023 3:33 PM > To: Ori Kam <or...@nvidia.com>; NBU-Contact-Thomas Monjalon > (EXTERNAL) <tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@amd.com>; > Andrew Rybchenko <andrew.rybche...@oktetlabs.ru> > Cc: dev <dev@dpdk.org> > Subject: "Thread safety" in rte_flow > > Hello Ori, ethdev maintainers, > > I am a bit puzzled at the RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE checks in > rte_flow.c. > > - The rte_flow.h does not hint at what is being protected. > > All I can see is a somewhat vague, in lib/ethdev/rte_ethdev.h: > /** PMD supports thread-safe flow operations */ > #define RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE RTE_BIT32(0) > > It would be great to have a more detailed description of what this > thread safety means. > Some history, When the rte_flow was first created it was part of the control path. This meant that functions should be called from a single thread or locked by the application. As more and more applications started to add rules from the datapath and we saw that such global locks are too expensive, we added the THREAD_SAFE flag that allows a PMD to declare that those functions are thread safe and no locking by application is needed. To create unified API from application point of view we moved the lock to the rte_flow functions. This means that all such functions are no thread safe, but if PMD declare itself as thread_safe the locking is done on the PMD. The idea is that the PMD is able to lock only part of the long running function and not all the function. To handle the notion that applications are adding rules in the datapath we introduced the queue based API (template/async API) in this API there shouldn't be any locks and it is the application responsibility to call each function with the queue that corresponds to the working thread. I hope answer your questions. > > - I don't think many functions of the rte_flow API look at this flag. > It seems it is never checked in newly added symbols (this is an > impression, I did not enter the details). > > Could you have a look? > Will have a look, but from my above explanation, the only functions that should have it are the functions that don't use queues or are assumed to be called from one thread only (control function) > > Thanks. > > -- > David Marchand Best, Ori
