"Thread safety" in rte_flow

"Thread safety" in rte_flow

[David Marchand]

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.


- 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?


Thanks.

-- 
David Marchand

RE: "Thread safety" in rte_flow

[Ori Kam]

Hi David,

Best,
Ori

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, May 26, 2023 3:33 PM
> To: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> (EXTERNAL) <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@amd.com>;
> Andrew Rybchenko <andrew.rybchenko@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