Re: [dpdk-dev] [PATCH 1/2] ethdev: make flow API primary/secondary process safe

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 15 Apr 2021 02:55:17 +0000
Suanming Mou <suanmingm@nvidia.com> wrote:

> Hi,
> 
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > Sent: Wednesday, April 14, 2021 9:07 PM
> > To: Suanming Mou <suanmingm@nvidia.com>; Ori Kam <orika@nvidia.com>;
> > Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; NBU-Contact-Thomas
> > Monjalon <thomas@monjalon.net>
> > Cc: dev@dpdk.org; Stephen Hemminger <stephen@networkplumber.org>
> > Subject: Re: [dpdk-dev] [PATCH 1/2] ethdev: make flow API primary/secondary
> > process safe
> > 
> > On 3/16/2021 11:48 PM, Suanming Mou wrote:  
> > > Hi Stephen,
> > >  
> > >> -----Original Message-----
> > >> From: Stephen Hemminger <stephen@networkplumber.org>
> > >> Sent: Tuesday, March 16, 2021 3:27 AM
> > >> To: dev@dpdk.org
> > >> Cc: Stephen Hemminger <stephen@networkplumber.org>; Suanming Mou
> > >> <suanmingm@nvidia.com>
> > >> Subject: [PATCH 1/2] ethdev: make flow API primary/secondary process
> > >> safe
> > >>
> > >> Posix mutex are not by default safe for protecting for usage from
> > >> multiple processes. The flow ops mutex could be used by both primary
> > >> and secondary processes.  
> > >
> > > Process safe is something more widely scope. I assume it should be another  
> > feature but not a bugfix for thread-safe?  
> > > And the fag RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE we have added is just  
> > thread safe.  


> > >  
> > 
> > Hi Suanming,
> > 
> > I think 'RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE' flag and what this patch
> > address are different issues.
> > 
> > 'RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE' is to add/remove synchronization
> > support for flow APIs, that is for thread safety as flag name suggests.
> > 
> > This patch is to solve the problem for multi process, where commit log describes
> > as posix mutex is not safe for multiple process.  
> 
> So for PMDs which not set the RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE capability bit, they will have the process level protection in multi-process.
> For PMDs which set the RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE capability bit, this change does not help with these PMDs. If the PMD with RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE capability bit  internally does not support multi-process, they may still suffer crash etc. (If I understand correctly, mlx PMD level now should support multi-process, but better to have the confirmation from maintainers with much deeper level).
> I assume this patch solves the posix mutex for multi-process only, hard to say the flow API primary/secondary process safe after that patch.

Maybe we need a FLOW_OPS_PROCESS_SAFE flag? and declare all existing drivers as not process safe?