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 D573FA0C4E; Sat, 23 Oct 2021 13:39:09 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5FE0F4069D; Sat, 23 Oct 2021 13:39:09 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id 86FD340041 for ; Sat, 23 Oct 2021 13:39:07 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10145"; a="229307655" X-IronPort-AV: E=Sophos;i="5.87,175,1631602800"; d="scan'208";a="229307655" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Oct 2021 04:39:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.87,175,1631602800"; d="scan'208";a="553584943" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by fmsmga004.fm.intel.com with ESMTP; 23 Oct 2021 04:39:05 -0700 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Sat, 23 Oct 2021 04:39:05 -0700 Received: from fmsmsx608.amr.corp.intel.com (10.18.126.88) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Sat, 23 Oct 2021 04:39:05 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx608.amr.corp.intel.com (10.18.126.88) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12 via Frontend Transport; Sat, 23 Oct 2021 04:39:05 -0700 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.171) 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; Sat, 23 Oct 2021 04:39:05 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Rc6m1eVLkgHLICxeB0Kawl1bIwULqOyIc3iZTm9azld/N+oYMxQA8A654IE9IIKh2ELAqP8kXd+osL4gFm5UAmDHiYWArPXcFnOiNdK1fxwI4+Au325Ves7gLYNt/GOKM+oBaMuHSKLrHL187+UddXdiFPT92Z8dn5aPdWL4I5Zx02vMoQHtidUyPBAVs/snJ+kKxj3CAGzX6sPwYvmTbPeNGpsaobYC3Mx3yqfwqSiAiC8nTHUJH5lIgdzMkZupOJvDI4FkIRxmtFBPrAXHcnZdnbZd2pNiDqBFRcW1cEXm9bRwPP0eVEhky7tGpzWNDiW6Mvec7oiTSSL/6iMvhg== 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=Gq4e0d9roKVxLQl67+WZbZ9maSwjw+JQKe4Gv6Wvn0s=; b=BEtgGnWWG4pc6oxgtx9cvJgJglLksPx0Owfs2cOi/o8NJ55uL36h2EaYUmpEo85y65FTuewn7NF4oZlDJju/+NWxiMpMbf7BvB/Q+KwBNX31s8OSKiq9XjJNu4nSSBTgqqE+rujKCWJNrxDQ1lU1CFo2d9t4zRjVM7/AWWu0HL1cKfFERhGm9QxdbBT0iKw3AVMkAUj778YnG2BXGFWZkT3Lx9Varx9S3GjsWVWfvYKNdvALA3VQ227LQtBzX0eC2SsO4phGqPnklZyuRdQPHmDfrEnMWTthmJyPk1tv4y8UbWmQFvJ42W9icJWIKlJoRtxlvK+eFEUNWeCltq3gKg== 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=Gq4e0d9roKVxLQl67+WZbZ9maSwjw+JQKe4Gv6Wvn0s=; b=r/zticHz4yef4ybPOjkjv4RCT2htWgsSyacbl6rzyGWaPCsHuJZOJIzsjmZ7G+cBsEBRTWPzoAxSkas9ai40xlgHpX4+g2Z2EWcJPuExYB2lT2f7zOwz+Y2sZLJ5aXfrkfg7/EUe86UZtXq+y22daA1QANWVigoH7vyuU7vbwx8= Received: from DM6PR11MB4491.namprd11.prod.outlook.com (52.132.251.211) by DM6PR11MB2732.namprd11.prod.outlook.com (20.176.95.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4628.20; Sat, 23 Oct 2021 11:39:04 +0000 Received: from DM6PR11MB4491.namprd11.prod.outlook.com ([fe80::2c0c:5383:f814:3b4e]) by DM6PR11MB4491.namprd11.prod.outlook.com ([fe80::2c0c:5383:f814:3b4e%6]) with mapi id 15.20.4628.020; Sat, 23 Oct 2021 11:39:03 +0000 From: "Ananyev, Konstantin" To: Thomas Monjalon , Bing Zhao CC: "Yigit, Ferruh" , "andrew.rybchenko@oktetlabs.ru" , "dev@dpdk.org" Thread-Topic: [PATCH 2/2] ethdev: fix the race condition for fp ops reset Thread-Index: AQHXx4nXzjvPlyQFGEeDnRI50TeQ7qvgQpuAgAArTQA= Date: Sat, 23 Oct 2021 11:39:03 +0000 Message-ID: References: <20211022211407.315068-1-bingz@nvidia.com> <20211022211407.315068-2-bingz@nvidia.com> <3885639.MmVk1qbWW8@thomas> In-Reply-To: <3885639.MmVk1qbWW8@thomas> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.6.200.16 authentication-results: monjalon.net; dkim=none (message not signed) header.d=none;monjalon.net; dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: ad46d35d-5e02-4d3f-eed9-08d99619b6f3 x-ms-traffictypediagnostic: DM6PR11MB2732: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8273; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: R6BQUI3BFDWmAJbwfKk2iGQe68jwKhF7iRt6I4ilms1kejntkZ963g1FgpJXtUJykpmYOxiBYMNHnoURu6Sp+qgnYyusQJwcjErwUepTExmqdHQuPU27Bs/c1S5hEmyVaCA6BqPpcUcJwF+Hdkz/VOsBUBYso5h0+tehAcGwGDuTumS38Cmf0/ji+fzyZsC0v4ZiEs3Z6Oh7r8qI61ycJKl/wfwk+tjyai2//6ibmO3VB4r0AkLrcT/0d9jNB8T4YqOK6xEnb37xim6h71+f0+ZYSemQtD6GC0jaEeXgsien6mxK9L5Np17V/2vAEADA0vYAo3LcaIqc2JrjWDxqzwqaAROGuTppl3SdDm6bNWssHomcqUbIfD/oXxBGo3/CRfiHTHtX9gmYp1n8iokMIgLPQEGthOP2z6lucr3X9ex5s9noEGPEqDeWr00GimZVRj/e0c4R4A6oFxptJNwuNxx8DxDx7/Q73W9xh00XiCsGsQ6mNcgFIrhOQtnVQoMuyPt0d5v0LdpxS9VoGclnr0bRgzZKvIl4TGSroGjGW07ZkyNZ+zI2KnKtKzOPUxm3zona0CsRAaGUUUY0xFqGRnpWGaYrbxOlT/oynsit/E2a/O/9WmUf8xLMH61F6vTnnn2YLJS6Qc2u6aksawTm5BxfyqGb0RveUh939jl2cP4b+vwhG9M/nmmzJoHOHRRms8CEd+VkFpy05W+6NwQRSA== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR11MB4491.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(366004)(122000001)(38100700002)(2906002)(186003)(82960400001)(26005)(8676002)(5660300002)(33656002)(38070700005)(316002)(66446008)(7696005)(66476007)(86362001)(66556008)(64756008)(8936002)(71200400001)(6506007)(9686003)(55016002)(508600001)(66946007)(110136005)(54906003)(76116006)(4326008)(52536014)(55236004)(83380400001); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?tOusvbheNldp6FEzxyx5jZ31ggw/5m30ePAEvE0+mgtW1mtBv0pfINWsiSlS?= =?us-ascii?Q?Nz9hjgWjzQ5e7/sd7o/5jVLElRWQMX+m7jr/xll7wpbX5WUf5c3Nt8QzMndR?= =?us-ascii?Q?X0J6FwsoC9IXZltfUZAvMxzrLCN7KFSVd0Flo0LYV/l9koL2s1LcefynBaM9?= =?us-ascii?Q?8NEwZ9EvoXhBRyYzgPxdXUhEDxzA4sVu0fIBBq2DkA7YkF1FsnZ2cOrF8UCn?= =?us-ascii?Q?7TPv09Gak7kyXrCINVt5YOU1rwgUinoWmIWiEmIXqecm5jN/d3JWKeOMzzBn?= =?us-ascii?Q?OOUHR3qw8SzqAKWrST3L7AeQS/PZHGMcmwtq3c59rHRwaCZ84Fb5mxDy/TUD?= =?us-ascii?Q?Q/kNcyk1FEZGipfCTIJDBRlCwWePQM5Ub+kr/ERzMK+8w+Zctw/s/as47VvS?= =?us-ascii?Q?DaKLGjqYh07COzZsNADvSe7wwgDYf2aYiTex/67PNQc5BP9Z1ZTx23jbTKHU?= =?us-ascii?Q?tqrx2vhg+6LEIqmcn4Tv25pnouFzNezlzea6p/LOMK6JJJjpmYFAifXcBGwm?= =?us-ascii?Q?uZt1ZkDeLTMy2n832YevkY644yEeyF4USB3QkSq0t8tDWtT1QbRgB0HQmuDs?= =?us-ascii?Q?IfDZfC6MFA+HlAEzwpgPgcq8GlRSdZpiC6WLZSgVR57qQ9hBO6trHwx6A5NM?= =?us-ascii?Q?Y2PZDvVsM9eXpzVx6159bNQDYOV4PjsK+jvpGEFliHPz2O2BKMKv3QUBx9XO?= =?us-ascii?Q?XKwIQMxPt4kbEybBS4TkYN0Bu8BjLjq4CQHRJuzb6Oz+6RRa+tfBLDCCzawf?= =?us-ascii?Q?3vUHrra7B6cisZjRoiz8bT37XxHadki71h6BYwID7HRwh3Pwp/smW7q0SCNy?= =?us-ascii?Q?uVhHJ3sfDHJ6yoghrh6Ea6jJWHzJW/EB4Z1u89veIT6RtMJsO20MVl9SAhfT?= =?us-ascii?Q?tOrPnmllZCsCBhhWDFdbgqmDMKpcsCRVcLXDP8Z/gcnozHBnNv77yL38BZij?= =?us-ascii?Q?fBk1TopOOiMzI1GcK7OgvWEnRM8sIxJ4mjACkbU70IHxFBU8FtAwpjyhmLMX?= =?us-ascii?Q?RtuCVIzZXNusHdY02yv5w5Cqv04bG3QHkZejtLB0W3JsLLw+Y5aiDRsRYqFU?= =?us-ascii?Q?mll+6q9Bmup9wd3uFyBPq581bwE39BXHov3MYeOeXGnKMggFdgeiwC67X/Mt?= =?us-ascii?Q?P8V9xccqtyiCWDVJ/jrPl1HhziVWwrXDP1qaC1jW/4+a/pa4P1wa3BjQdEzc?= =?us-ascii?Q?p7lyUe+EyJ6hgyy6dziOm44FpjL03hio8JUu5yK6p4AJafJd0+gUgmLUpSBy?= =?us-ascii?Q?9XVdw1RfX8miODeMLaD2GG6+J4OiwAGPrjzZjsEFc6hrRULjxg3uTmAJhirB?= =?us-ascii?Q?uXIA4nxWpgkjN+w6TJMH+NAiycxr9Vv6cDVrIgyLO/9ITHJ8OUVWO+jmwxD7?= =?us-ascii?Q?YfWU0VMsmbDgKwFdJz2lag1hjeC76dBT4n15WnMkEiUBfLhL6b8PLDqFJQJ0?= =?us-ascii?Q?eDpKdgwok1eEyCs3mkSmbbuq1sLqr4bOWzXL8c4HiDHKISujFhqjK6ZBT6m1?= =?us-ascii?Q?z1VJsfvJua4MfBJbNr38ZGhLrOtZE4w+RssL/M0bp5/vG5K6MedwWL62mRwm?= =?us-ascii?Q?l54EI8IqfdCIk1dIEaJkdzJlsgVxZS5zEJYZrrvvLIxdd8tKoc60/a1s5JFN?= =?us-ascii?Q?vkVSBQqGHO9zJl7nbssHeCxwtSdB+84Q/JbazJmF6UoG0XVCkUOzouxjTFqp?= =?us-ascii?Q?kDHnmA=3D=3D?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB4491.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: ad46d35d-5e02-4d3f-eed9-08d99619b6f3 X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Oct 2021 11:39:03.2487 (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: w3RHbotuO6EF2jxBpa6XBIGYq3arf4vSWXAO0tciq6qv+KUkdlRNSAb6Am7akTTD6HR/2fULt7g8WspVEqMgGVxb/sB7OuwQ6iLyc5mlL+0= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB2732 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH 2/2] ethdev: fix the race condition for fp ops reset 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" > 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 > > =3D> 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 =3D 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 structur= e") > > 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 =3D {.data =3D dummy_data, .clbk =3D dummy_data,}, > > }; > > > > - *fpo =3D dummy_ops; > > + rte_memcpy(fpo, &dummy_ops, sizeof(struct rte_eth_fp_ops)); >=20 > That's not trivial. > Please add a comment to briefly explain that memcpy avoids zeroing of a s= imple assignment. >=20 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 en= sure 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 need= s to be fixed. 2) rte_memcpy() provides some sort of atomicity and it is safe to use it on= its own=20 in MT environment. That's totally wrong. In both cases compiler has total freedom to perform copy in any order i= t 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). =20 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.).=20 But as I said above right now ethdev API is not MT-safe, so it is not r= equired. =20 To summarise - there is no point to mae these changes, and patch comment is wrong and misleading. Konstantin