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 0423FA0C41; Fri, 16 Apr 2021 10:12:18 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DF3BF141B68; Fri, 16 Apr 2021 10:12:17 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mails.dpdk.org (Postfix) with ESMTP id DE91A141B65 for ; Fri, 16 Apr 2021 10:12:15 +0200 (CEST) IronPort-SDR: ZhIyu54ljiJtRANZlogJVYCtqNSU4Oe8GRbFvcIROgjl3uyfhXxfmDlqCTqpbN9ejWlaUq3UbC i+WJG7x+7rMw== X-IronPort-AV: E=McAfee;i="6200,9189,9955"; a="174501177" X-IronPort-AV: E=Sophos;i="5.82,226,1613462400"; d="scan'208";a="174501177" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Apr 2021 01:12:14 -0700 IronPort-SDR: z5PKDqrTmGc6M8K4yj1N7QloTo/lV6Jf0y888EZwM7OKtyRbBieECUgRXeyW3mEHskSPwvp/9i 4ByoQut1Tz1Q== X-IronPort-AV: E=Sophos;i="5.82,226,1613462400"; d="scan'208";a="425508256" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.207.150]) ([10.213.207.150]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Apr 2021 01:12:12 -0700 To: fengchengwen , 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> <544287b8-b430-18b1-8baf-2028d22ef0df@huawei.com> From: Ferruh Yigit X-User: ferruhy Message-ID: Date: Fri, 16 Apr 2021 09:12:09 +0100 MIME-Version: 1.0 In-Reply-To: <544287b8-b430-18b1-8baf-2028d22ef0df@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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" On 4/16/2021 2:41 AM, fengchengwen wrote: > 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. > Thanks Chengwen for testing, appreciated. > > 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 >>> >> >> >> . >