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 EB3ABA034F; Wed, 10 Nov 2021 15:35:05 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 76A414068B; Wed, 10 Nov 2021 15:35:04 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mails.dpdk.org (Postfix) with ESMTP id E1E2240683 for ; Wed, 10 Nov 2021 15:35:02 +0100 (CET) X-IronPort-AV: E=McAfee;i="6200,9189,10163"; a="212704575" X-IronPort-AV: E=Sophos;i="5.87,223,1631602800"; d="scan'208";a="212704575" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Nov 2021 06:35:02 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.87,223,1631602800"; d="scan'208";a="602221617" Received: from fmsmsx606.amr.corp.intel.com ([10.18.126.86]) by orsmga004.jf.intel.com with ESMTP; 10 Nov 2021 06:35:01 -0800 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx606.amr.corp.intel.com (10.18.126.86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Wed, 10 Nov 2021 06:35:01 -0800 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Wed, 10 Nov 2021 06:35:00 -0800 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12 via Frontend Transport; Wed, 10 Nov 2021 06:35:00 -0800 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.108) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.12; Wed, 10 Nov 2021 06:35:00 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Q2I8zuJGxGCvm3novt+6fak2Dlj7o+0jsQOfyXrzzQgG8+OjYh2etosQtDWMSk8Dg7TRQPVIQj4uKlFZxTYnMlGCfHqNGPTBnmwlBUKl0xTouL1O47FTOtir8QGk+O6QWdkSP9AQZAsAjZgxwr6rgYnR62fazLt9QVu0MNo2DsS9JT/s/UpzDmgEh7w2nAAbmgPyhM+lf6epL7iDXdD/3AeQYtXnpUxR+2/cNbNRzlRgEafFdiL4elZH+ILiCImIDYvLfaimVpZbZO1ooSwmFFKAVbIG/jW+2YEPhl1yi7ld/xZLAsg+MJ2MfLSovdP7/qjjuUjs7AoNykI9j75lSA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=9SaNtZnzdI1KULQEoYooMbZs05zjBuTfcdwZhQKDHuw=; b=W9WGO1gbUtKKoXcT+IU72hF5RCUrf4SMQiOzZ432gJpqT0YOKBnjcVC00mAz16zZayCOSQ2555VOAHYsUWvNk2XO2MJ5sQMIqhdrgLqEKOOwufwBg7AeOKs67lVEESCUfh6rxgxx4imwskh9Txd9E2kgNdS8dtsjWOSOXPcdz6igozSYSYF6VGF+H0YUvsVtEPi4AfCFuXjsR0904RTYfLgiMilpOeGkNhjFMd5+9V6cKfTr63I0SscRa8AAd0vx2G0r306o7C0MEaXgSFsufD1jH0DBibWBvIcIydhFSULOi3t5bhMIwzPQv6RD3S/PCz8K4D0QCtLNmlRGUkOLeQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=9SaNtZnzdI1KULQEoYooMbZs05zjBuTfcdwZhQKDHuw=; b=QYy9w/nj6uPAw/6XXZbxzTpS0xagBohsg5e7pbNg1K8JcgA47N+UyjjaIOHgFT5hPU5n//R7dLp60jWLdZbI0ZvNPrhAbzyuVyhkUKG7nQ3PJjNEkyyzBbf2+o7CuMFHZIyz1dt3suvcNYdwZdX1EqiY2zM7qFZp1vhOrKMNIv8= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) by PH0PR11MB4871.namprd11.prod.outlook.com (2603:10b6:510:30::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4690.16; Wed, 10 Nov 2021 14:34:59 +0000 Received: from PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::bd7d:29be:3342:632c]) by PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::bd7d:29be:3342:632c%6]) with mapi id 15.20.4669.017; Wed, 10 Nov 2021 14:34:59 +0000 Message-ID: <16525cf6-bf10-e637-3f03-f4d415ff6bb1@intel.com> Date: Wed, 10 Nov 2021 14:34:52 +0000 Subject: Re: [PATCH 2/2] ethdev: fix the race condition for fp ops reset Content-Language: en-US To: "Ananyev, Konstantin" , Thomas Monjalon , Bing Zhao CC: "andrew.rybchenko@oktetlabs.ru" , "dev@dpdk.org" References: <20211022211407.315068-1-bingz@nvidia.com> <20211022211407.315068-2-bingz@nvidia.com> <3885639.MmVk1qbWW8@thomas> From: Ferruh Yigit X-User: ferruhy In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P123CA0032.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:151::19) To PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) MIME-Version: 1.0 Received: from [192.168.0.206] (37.228.236.146) by LO4P123CA0032.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:151::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4690.18 via Frontend Transport; Wed, 10 Nov 2021 14:34:57 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: ed175926-7e76-457c-dd66-08d9a45745ab X-MS-TrafficTypeDiagnostic: PH0PR11MB4871: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:8882; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: YBWcMI1etxjnDFVbL++pXNFoAy3L0PGcKy4rUmh0W24Xb7wKwVHe4zQr8z1Go/OEkEP5I5LIXk0aQq2M7sZo12NENbGkDZah8VwDEq4oVpUwcfIbd4L0RU9JjT2yU4LuKlxBWeDlewPCqKvMZLUeMI0J4YegsHxJ/6WlK2P53KCUAuhVtwWrETNxC0/wjIsMkAOUdQuvZEOBFh/xteLJgb/K3AAwuE0O2xSdSDsrNRORGgcKAuRTHZZC3Mw6B0+mG0agE3bfyO6V8XcVGkYdZDYUo1zRfiKXlw7ZCVKz0uUB7aIAFdkJwYKpilqU64YrvqGV9ViyJ3anSpmBGsaCFnSX5UYjVKqslt7exxJr9fqMR0YfEPUwBsjT9Ha9NzmUtJepVtBhkLmHY3mRKHv8xntvbA/jTC8Qbx8oP6yD7RpG9Pin3SoljRzF6m+vOTiB0QlBf31DuQyjeE1j3umn5byVIcFncWRf93Ipaj/1thAwQ5Ppd6t8eqE0URUP46TdE22rNqlHs4Wv/JADzb7FVmQRQ/arHrNW3O5jveiqHlSVIptDPeZqSWp/20FOvSpD4eFgl5C/taKHr4LmNftHMNu5xEvDqks29Qy9Oo+hbFkAujBphEaBIvshPt1C0fpb+a/X7dgDmYF03t1jcA3uss35XehZPKzt8prqHoAnSz7AkXvVPZz2UtBpDTr6BIfi9r0L8NGYdz6h+hWn/g4qHQ== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PH0PR11MB5000.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(366004)(86362001)(36756003)(2616005)(4326008)(5660300002)(2906002)(44832011)(110136005)(8676002)(54906003)(316002)(16576012)(956004)(82960400001)(186003)(31696002)(8936002)(31686004)(508600001)(83380400001)(66476007)(66556008)(26005)(53546011)(38100700002)(6486002)(66946007)(6666004)(45980500001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eE5lZ2ZyUmI4Y2E2QmtWQlUyVnVUOW8yRG83b1ZSb3VXMzAzaURMczdIV0s5?= =?utf-8?B?UFVPNFZuUjNIQXRUZSsyZmtUY1hKbWpKVUx2OXhQRlAxbEdDZldlYkNtWVNx?= =?utf-8?B?RGJSbm5MZ2lPR09ScnpuVFc3NTl1S0hYVm40eWRkMjRoZjFaMGMzWEc0amln?= =?utf-8?B?K0lZSnBmWG15V1RsM0NNV3pKTFpubTlQN3ZVS1NrVmNoUTRBbnFtUmpkWmRR?= =?utf-8?B?WG8wckJpVnNrTGZTT0g2ZmlmNnplQjRMZUZGeHUwQjcya2ZOOERWbXBJTmQ4?= =?utf-8?B?ckZiYmdUNXhISERCYUZ0dm9jcU1ocnVwQ0xnZUZ1WUNwQ1lBYkozVC9jZ2Er?= =?utf-8?B?TXovbFFudkh2ODR2VXNkVzM2MThIRXViZnFFVkVKUThqQkVtT3hoOXVKblEx?= =?utf-8?B?NGNSWnk0YmhKZWpzNjN2ZURDNkJoS3R2c0t4WU9YeHVIVFA5ODRZbS95N056?= =?utf-8?B?eXpPdzdPSGs3SU1IdCs1R2RqcmY3MktramxYTjRHaG9FNmdPbkdjcW1KaC91?= =?utf-8?B?d1RtR3FQM1pQaWl3THdxcjhyYWFiTkNwUW94RTljRUIrcXZzMVhtdlIzb2Rm?= =?utf-8?B?dDFST2ZpeFhyY3dDZDlLVFVwbVJIM1V4dFd6VnZtdUhCWHltNkxPUG5QajYw?= =?utf-8?B?dmp6am5lY3NKWSsyTS9kTElOU25lOC9oaUliOS9ENWxPYnU1S2dYZDh4d0lD?= =?utf-8?B?alBQbWpRL1lHOGk1ZW9TK1ZmMnBrcUVYVjRkYy9ZN2VTSnBaT2tPdnFBdjZw?= =?utf-8?B?K3lDN2J6SFkrL1NTeE84OVVSb3hQUkU3MVMxazdMNHp4aTFYdjI0eDBtS1o3?= =?utf-8?B?dFRhUUJvRHZpdUJEWEhxMUwyekxabnRrQWJiakRRMzlEVWdidFVoN0s1U3JU?= =?utf-8?B?dVF2elZ4SXY3dERxUHBUdjlyRU4yM2JtYVBheWhVUHl5VHhYd3BHMVlrWVRL?= =?utf-8?B?ZGFKQy9IM0xVL0cwb2Z6K2g1RWQ0aDcxbHJOOVVnWElTNEMwYkZpc3JyMnZk?= =?utf-8?B?TlE3ckk0MlBGL0FTay8rVHk5SEw5UTFRbVdDUXpGSTVYWVVTVFVTVVU2akJ1?= =?utf-8?B?c1FGdVhQSml2T1BpamRURCtpbS83NkQwS0FjOTZScmtKRXBFV1RPWjRkazRU?= =?utf-8?B?bk8xeG1Ka1VzY0UreHFGQkhyclVoZFg0bVJibGs1emJGbTRjYytERlVOdEpB?= =?utf-8?B?VXNGNEMvbVVacmZac01LR0hhSStvdWdFSzNqZzhzVjhXN1NneTY3RHBvQ3ZT?= =?utf-8?B?cDJZSTBSM213eW5VVlczSVR2Y0JMbk9Oc3JhOG83MGNyMWVFelJYY0Q2NkN4?= =?utf-8?B?OFBSUGFKZlpyaU1oWm51cU4xNzBHYk83aDFwYUVnbWlaRlo4Ny9uakc4WmdW?= =?utf-8?B?c1gvWTBpZCsrbVpmcTRDU2NFQm8vMFRxT2d3M2VMWWltYXZDTTIyUXI2T2FF?= =?utf-8?B?TG1mSXIxdDBWckVSbHUyT2prcHljMTlnR1dhaTFLTTcvdHNhanFFcTNkMEt0?= =?utf-8?B?bTFnODJHbGtzRHhCV0F2L0hwNlZrRWFZOFVTNzAwVkZBNWVqV0FWNTZUR3Yv?= =?utf-8?B?ckFuT2RRdS9lVzdwUDVzdnRrblJIZVJZcW9RTTdOcEdYSVBkeGpCSVNWR0tE?= =?utf-8?B?WUt0WXE5VHZzUUltK3dRWjJPcWpPdmpId2lDS0VaRWR2b0l4VmlLc0Y3RUc3?= =?utf-8?B?WkxMT0g0dVdyNkZINm96Ylo3dGxROXlUaTZlTXlBUHgwbE9YK3ljQjVrR1ZB?= =?utf-8?B?dEdNMEJQd0w2c1FSZW80MlF2aURycnJZNyswOE5tdVBuUFVEV2lOQVpCQUJR?= =?utf-8?B?cmZGYy9HOFFPeHRIQlJnaUdWRE94dFBQeStBT3phSk8xOFlrT3Y1ckJRRDgr?= =?utf-8?B?a0pwb1V0UmpMaTI1MjRnL2c5VXJkVHBiREZuRTNNZnM2MEduVzJiaDM3ZW9r?= =?utf-8?B?NmhmNmF3UXFSTE9PUWtWUEFMNkxCWWh2WEFLN2IwS3ZTSXA3ejAvOTF4Vkly?= =?utf-8?B?dGZqMThmSXRrUVJTTThpb0ZFVHdLaldKSzBhUDhkam1YZWNIWERNUG9IVnM3?= =?utf-8?B?dm45VitXVjVTNHpxak11VGFNYWRiYTV4MVpQdVRsOVIwcTN6YW9sZFR6c21J?= =?utf-8?B?SmllZ1FNRVdFTTdRbVBxQUlrWmFSVWxmbU1wR1dCNU12eGV4aU8vc1F2eHJy?= =?utf-8?Q?JpVizFgOrt2ySA6aKJ1w7Bc=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: ed175926-7e76-457c-dd66-08d9a45745ab X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5000.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Nov 2021 14:34:59.0245 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: le3kBUJhOomnKWRyw3JxDQmEA3EdTxwxToIp8xJTryuaZvTD1gByQBQT4a5keHGyzr7T3dfsMw9pWGcjoPO53A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4871 X-OriginatorOrg: intel.com 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 On 10/23/2021 12:39 PM, Ananyev, Konstantin wrote: > > >> 22/10/2021 23:14, Bing Zhao: >>> In the function "eth_dev_fp_ops_reset", a structure assignment >>> operation is used to reset one queue's callback functions, etc., but >>> it is not thread safe. >>> >>> The structure assignment is not atomic, a lot of instructions will >>> be generated. Right now, since not all the fields are needed, the >>> fields in the "dummy_ops" which is not set explicitly will be 0s >>> based on the specification and compiler behavior. In order to make >>> "fpo" has the same content with "dummy_ops", some clearing to 0 >>> operation is needed. >>> >>> By checking the object instructions (e.g. with GCC 4.8.5) >>> 0x0000000000a58317 <+35>: mov %rsi,%rdi >>> 0x0000000000a5831a <+38>: mov %rdx,%rcx >>> => 0x0000000000a5831d <+41>: rep stos %rax,%es:(%rdi) >>> 0x0000000000a58320 <+44>: mov -0x38(%rsp),%rax >>> 0x0000000000a58325 <+49>: lea -0xe0(%rip),%rdx >>> // # 0xa5824c >>> >>> It shows that "rep stos" will clear the "fpo" structure before >>> assigning new values. >>> >>> In the other thread, if some data path Tx / Rx functions are still >>> running, there is a risk to get 0 instead of the correct dummy >>> content. >>> 1. qd = p->rxq.data[queue_id] >>> 2. (void **)&p->rxq.clbk[queue_id] >>> "data" and "clbk" may be observed with NULL (0) in other threads. >>> Even it is temporary, the accessing to a NULL pointer will cause a >>> crash. Using "memcpy" could get rid of this. >>> >>> Fixes: c87d435a4d79 ("ethdev: copy fast-path API into separate structure") >>> Cc: konstantin.ananyev@intel.com >>> >>> Signed-off-by: Bing Zhao >>> --- >>> --- a/lib/ethdev/ethdev_private.c >>> +++ b/lib/ethdev/ethdev_private.c >>> @@ -206,7 +206,7 @@ eth_dev_fp_ops_reset(struct rte_eth_fp_ops *fpo) >>> .txq = {.data = dummy_data, .clbk = dummy_data,}, >>> }; >>> >>> - *fpo = dummy_ops; >>> + rte_memcpy(fpo, &dummy_ops, sizeof(struct rte_eth_fp_ops)); >> >> That's not trivial. >> Please add a comment to briefly explain that memcpy avoids zeroing of a simple assignment. >> > > I think that patch is based on two totally wrong assumptions: > 1) ethdev data-path and control-path API is MT-safe. > With current design it is not. > When calling rx/tx_burst it is caller responsibility to make sure that given port is > already properly configured and started. Also it is user responsibility to guarantee > that none other thread doing dev_stop for the same port simultaneously. > And visa-versa when calling dev_stop(), it is user responsibility to ensure that > none other thread doing rx/tx_burst for given port simultaneously. > If your app doesn't follow these principles, then it is a bug that needs to be fixed. > 2) rte_memcpy() provides some sort of atomicity and it is safe to use it on its own > in MT environment. That's totally wrong. > In both cases compiler has total freedom to perform copy in any order it likes > (let say it can first read whole source data in some temporary buffer (SIMD register), > and then right it in one go, or it can do the same trick with 'rep stos' as above). > Moreover CPU itself can reorder instructions. > So if you need this copy to be atomic you need to use some sort of > sync primitives along with it (mutex, rwlock, rcu, etc.). > But as I said above right now ethdev API is not MT-safe, so it is not required. > > To summarise - there is no point to mae these changes, > and patch comment is wrong and misleading. Can we mark this patch as rejected now? Patch seems trying to cover a wrong application usage, and it should be addressed in the application level.