From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2F3E6A0C3F; Fri, 16 Apr 2021 03:41:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A30AE1624D0; Fri, 16 Apr 2021 03:41:32 +0200 (CEST) Received: from szxga06-in.huawei.com (szxga06-in.huawei.com [45.249.212.32]) by mails.dpdk.org (Postfix) with ESMTP id 5889240140 for ; Fri, 16 Apr 2021 03:41:30 +0200 (CEST) Received: from DGGEMS404-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga06-in.huawei.com (SkyGuard) with ESMTP id 4FLzQs04xqzkkWQ; Fri, 16 Apr 2021 09:39:33 +0800 (CST) Received: from [127.0.0.1] (10.40.190.165) by DGGEMS404-HUB.china.huawei.com (10.3.19.204) with Microsoft SMTP Server id 14.3.498.0; Fri, 16 Apr 2021 09:41:23 +0800 To: Ferruh Yigit , Suanming Mou , Ori Kam , Andrew Rybchenko , Thomas Monjalon CC: "dev@dpdk.org" , Stephen Hemminger References: <20210315192722.35490-1-stephen@networkplumber.org> <20210315192722.35490-2-stephen@networkplumber.org> <7106da73-95a1-30ae-f949-87ecca05b24d@intel.com> From: fengchengwen Message-ID: <544287b8-b430-18b1-8baf-2028d22ef0df@huawei.com> Date: Fri, 16 Apr 2021 09:41:22 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <7106da73-95a1-30ae-f949-87ecca05b24d@intel.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.40.190.165] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH 1/2] ethdev: make flow API primary/secondary process safe X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" We make a test on this patch, test result show that it works fine. Below is the detail: HW: Kunpeng920 ARM Platform which is ARMv8 NIC: Kunpeng920 SOC NIC OS: Linux centos-C3 5.12.0-rc4+ DPDK: 21.02 DRV: hns3 Start three process: ./testpmd -w 0000:bd:00.0 -l 67-68 --proc-type=auto -- -i --num-procs=3 --proc-id=0 ./testpmd -w 0000:bd:00.0 -l 67-68 --proc-type=auto -- -i --num-procs=3 --proc-id=1 ./testpmd -w 0000:bd:00.0 -l 69-70 --proc-type=auto -- -i --num-procs=3 --proc-id=2 Every process execute following steps: 1. create one fdir rule, eg: flow create 0 ingress pattern eth / ipv4 src is 192.168.110.26 / end actions queue index 0 / end note: each process has different rules, so they will create success 2. create one rss rule note: all process create the same rules, so they may create fail 3. flush all rules 4. goto step 1, loop again note: there are +10ms delay after step 1/2/3 because we run VBS script to inject command on windows. The test lasted 12 hours, and there are no process crashes. On 2021/4/14 21:06, Ferruh Yigit wrote: > On 3/16/2021 11:48 PM, Suanming Mou wrote: >> Hi Stephen, >> >>> -----Original Message----- >>> From: Stephen Hemminger >>> Sent: Tuesday, March 16, 2021 3:27 AM >>> To: dev@dpdk.org >>> Cc: Stephen Hemminger ; Suanming Mou >>> >>> 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. > > > Stephen, > Are you aware of any downside setting 'PTHREAD_PROCESS_SHARED' attribute to the mutex? Any possible performance implications? > > Ori, > Since mlx is heavily using the flow API, is it possible to test this patch? If there is no negative impact, I think we can get this patch, what do you think? > >>> >>> Bugzilla ID: 662 >>> Signed-off-by: Stephen Hemminger >>> Fixes: 80d1a9aff7f6 ("ethdev: make flow API thread safe") >>> Cc: suanmingm@nvidia.com >>> --- >>>   lib/librte_ethdev/rte_ethdev.c | 6 +++++- >>>   1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index >>> 6f514c388b4e..d1024df408a5 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.c >>> +++ b/lib/librte_ethdev/rte_ethdev.c >>> @@ -470,6 +470,7 @@ rte_eth_dev_allocate(const char *name)  { >>>       uint16_t port_id; >>>       struct rte_eth_dev *eth_dev = NULL; >>> +    pthread_mutexattr_t attr; >>>       size_t name_len; >>> >>>       name_len = strnlen(name, RTE_ETH_NAME_MAX_LEN); @@ -506,7 >>> +507,10 @@ rte_eth_dev_allocate(const char *name) >>>       strlcpy(eth_dev->data->name, name, sizeof(eth_dev->data->name)); >>>       eth_dev->data->port_id = port_id; >>>       eth_dev->data->mtu = RTE_ETHER_MTU; >>> -    pthread_mutex_init(ð_dev->data->flow_ops_mutex, NULL); >>> + >>> +    pthread_mutexattr_init(&attr); >>> +    pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED); >>> >>> +    pthread_mutex_init(ð_dev->data->flow_ops_mutex, &attr); >>> >>>   unlock: >>>       rte_spinlock_unlock(ð_dev_shared_data->ownership_lock); >>> -- >>> 2.30.2 >> > > > .