From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5E540A052B; Wed, 29 Jul 2020 11:37:53 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A56714C8E; Wed, 29 Jul 2020 11:37:51 +0200 (CEST) Received: from EUR01-HE1-obe.outbound.protection.outlook.com (mail-eopbgr130080.outbound.protection.outlook.com [40.107.13.80]) by dpdk.org (Postfix) with ESMTP id A87DE10A3 for ; Wed, 29 Jul 2020 11:37:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=soqvt+Ncp2qu3p5q9+R5J1W08JPQGYKBW3pz/yWv7DE=; b=uIorX4gcoCmEB9ih9mjyhyqzDU/P9uwhBI8k4kaDwb4h79AsUxQFRfpMelPDvurwOs+SWM5rR6HuFGB+I5aFkKhpxr5MfU11SG8ooXsAoaKQUyAEkA8Mv46wNACqKIyGaZlThU3vMlSuS1cEYW15rR9ryekdWLHQX1ES6C0wDqw= Received: from AM6P194CA0023.EURP194.PROD.OUTLOOK.COM (2603:10a6:209:90::36) by HE1PR08MB2764.eurprd08.prod.outlook.com (2603:10a6:7:2b::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3216.30; Wed, 29 Jul 2020 09:37:48 +0000 Received: from VE1EUR03FT004.eop-EUR03.prod.protection.outlook.com (2603:10a6:209:90:cafe::b5) by AM6P194CA0023.outlook.office365.com (2603:10a6:209:90::36) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3216.23 via Frontend Transport; Wed, 29 Jul 2020 09:37:48 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dpdk.org; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dpdk.org; dmarc=bestguesspass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by VE1EUR03FT004.mail.protection.outlook.com (10.152.18.106) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3216.10 via Frontend Transport; Wed, 29 Jul 2020 09:37:48 +0000 Received: ("Tessian outbound c83312565ef4:v62"); Wed, 29 Jul 2020 09:37:48 +0000 X-CR-MTA-TID: 64aa7808 Received: from cc2dcde636fc.2 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 30F5681C-481B-4E58-BB00-F1FF80758DA3.1; Wed, 29 Jul 2020 09:37:43 +0000 Received: from EUR05-DB8-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id cc2dcde636fc.2 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Wed, 29 Jul 2020 09:37:43 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hwjJ+ClmIy+47N4m0msxrs+LNlc0S/yQLtUM5WRoesudGCarxd7BjCG5qGfXnYta9ac82uR3ZryDEv77jgKcNs3bXX/iv2YoKr+fbsjC3TItvWwhMp0Xi3EXLYXX1zf2nO1qq2mgqI/XD1XmLBUzWXZKGGy9DU2XpDZbVNLKzNQHzgZ3UhOwP1hXJ3dXnoaCtMYmTBKVcELuMsP9GQRwE6ErAeuYRPC7axQNIHY6/fyED5YRDpKs4l89JPqq89O/q3UZT0KyoQ4WSBeqSSZvTMOOfNob3/jjrxKsZKKD4T1u0R/rIqtD6DRB+TXT4yI/8JOnUU1v2al3gYN+3S1ZAQ== 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-SenderADCheck; bh=soqvt+Ncp2qu3p5q9+R5J1W08JPQGYKBW3pz/yWv7DE=; b=cqFtOp/szayjGECUL9uKYtuiGQrxu3+Bn39ChkyfJmr+vOPdUC3gB9AnosCLX0IvPIr9qFYX5yB99NZ5bcIHO2qIE3mztAQ3+8GqUMLsL5WUSx4P4AcjRX9aD3Mk5j5nAo0dDQ5BGWBAjKnJvXFUXuOoWoH0W58Z/4nrfs+iuuBMVzfx1rWQUz0nvhIaPFyjX/7YhnAJLqigPER3ACWbuextkUTWnQTtOfei4WD0MoQxlg9phP3KwFvC+gdKAEnFQWHpY4Pf0uu77I072r/cErR1Q2oWjQJ+P5jxcT1geju/kPUGNYzw4znR9LgEc2Q9mhB+KfvofKvRa+Fdz2YBEg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=soqvt+Ncp2qu3p5q9+R5J1W08JPQGYKBW3pz/yWv7DE=; b=uIorX4gcoCmEB9ih9mjyhyqzDU/P9uwhBI8k4kaDwb4h79AsUxQFRfpMelPDvurwOs+SWM5rR6HuFGB+I5aFkKhpxr5MfU11SG8ooXsAoaKQUyAEkA8Mv46wNACqKIyGaZlThU3vMlSuS1cEYW15rR9ryekdWLHQX1ES6C0wDqw= Received: from VE1PR08MB4640.eurprd08.prod.outlook.com (2603:10a6:802:b2::11) by VI1PR08MB2782.eurprd08.prod.outlook.com (2603:10a6:802:1c::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3216.33; Wed, 29 Jul 2020 09:37:41 +0000 Received: from VE1PR08MB4640.eurprd08.prod.outlook.com ([fe80::28a3:3a4e:65ca:5707]) by VE1PR08MB4640.eurprd08.prod.outlook.com ([fe80::28a3:3a4e:65ca:5707%3]) with mapi id 15.20.3216.033; Wed, 29 Jul 2020 09:37:41 +0000 From: Phil Yang To: Slava Ovsiienko CC: Matan Azrad , Raslan Darawsheh , "thomas@monjalon.net" , "ferruh.yigit@intel.com" , Honnappa Nagarahalli , nd , "dev@dpdk.org" , nd , nd Thread-Topic: [dpdk-dev] [PATCH] app/testpmd: fix txonly mode timestamp intitialization Thread-Index: AQHWZCpe/3OmdULKVEGKjH29L88WFakczJ6QgAFp3QCAAA1c8A== Date: Wed, 29 Jul 2020 09:37:41 +0000 Message-ID: References: <1595863595-11344-1-git-send-email-viacheslavo@mellanox.com> In-Reply-To: Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ts-tracking-id: 0add16d2-4bde-4b37-a2e8-23c5a4b16041.0 x-checkrecipientchecked: true Authentication-Results-Original: mellanox.com; dkim=none (message not signed) header.d=none; mellanox.com; dmarc=none action=none header.from=arm.com; x-originating-ip: [203.126.0.111] x-ms-publictraffictype: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: b33fd47c-9cb6-418e-6558-08d833a30e48 x-ms-traffictypediagnostic: VI1PR08MB2782:|HE1PR08MB2764: x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr x-ms-exchange-transport-forked: True X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true nodisclaimer: true x-ms-oob-tlc-oobclassifiers: OLM:10000;OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: MG3gW3RFzj+LFJuq7G+SSatXoNAgY2LKNyAvHZNpOEzfOZVDL7WHsgwQB4D5ojyq1bIO2JT1R+i9yQFpiVoml0+RylbdjJIrwQ1yM0FEjm6LKdr6uQG9Ng1s4nyy0gL6Bb60VK+dwstNuRy3ldIdP/6kOWjVKDxtUepxrbXY+eGCOClYQARu6ZgwX4ZxTFdIx+NpwgFlT+YPi84z0WCQwWnUyf/ZiZj81mTphwzZU1/FPlx46HDZ8YG5qF1Lxe2bw31rjJw731oHRwQhhwTZUptTUXBEJrkDqpZnAK92mP0fa3M7qovHKxzLts+u0y1IqqeqiaV1nile5vQZxg+zfw== X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VE1PR08MB4640.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(396003)(376002)(136003)(346002)(39860400002)(366004)(4326008)(52536014)(6506007)(26005)(53546011)(8936002)(7696005)(5660300002)(186003)(54906003)(316002)(33656002)(2906002)(66556008)(9686003)(478600001)(76116006)(66946007)(71200400001)(66476007)(64756008)(66446008)(83380400001)(6916009)(55016002)(86362001); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: wl+/xdTp9K2plNZ22i+Aa3+/M7t4xUjvGNYjMy8EdPtGa37WxjgfLARQdaEX53M0Ru2v3PXqt47rDPNc4+yA6d244Miozwy/PMOf+TBNC62/haV1RwjF95pTu3BL5BiMKi3A06eT3u7ZSfqsTsl2/dadHeiCnRQ/xdDx9BLhL+aD5nwhsZ4l5aSXtgRfYoC+xuGtTlu7Rh1koDh3BsY4XzaDVD5ainYAXTPe0rpmy+NGJ17Fn1mww7CVegIkv6yhZFLBnO+yA5K+zWtt6ZYYwaSAOFsww+frI5/UyD/Gcq6hTiC6/amzCieW/AT5V9QgrS/j28tmStkkYkD+I+FdALzjnN6tfHcK4fPptHnV8OCq+5MMAH25C9ij3XgzMTE56KP1/INB1Y5B9O/dP4gZLs9Ygubpsot5upgymb50uvE/6rpEtiY9PEhOU5M31hWBVdEP8tcmhGkZmkTISzr4lv188L3L87bmpS3BRqgjTjk= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB2782 Original-Authentication-Results: mellanox.com; dkim=none (message not signed) header.d=none; mellanox.com; dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR03FT004.eop-EUR03.prod.protection.outlook.com X-MS-Office365-Filtering-Correlation-Id-Prvs: c32edc40-f6c6-42c9-4a97-08d833a30a37 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: MtCyusvd2eOBfa4xYuIclav/+PMQgN9U+AkJQBY5/ViFMN2SrVmsOfLbldJAybEQdUto7kOUZHVmstKd0/sQtdHU9FvLHh7xFlkcWoT7dtsPSmGcn/Fw1cITen+wGRtZBXDAv0KV+3oWjt+pNrFFBF/askDHejOnCkdbjTTUYgUpUIjqgV+MBZLJuxD7pdwQPMBDqbAbQNxsBtFgoMmaMBnjoMGAEFR+bMfftEXwiPTRQ0poMuyTsBVOEDYSQB4ROTXgLrQsY+3QM1Wr1q7jqzwRskXOVz1L3uYY8+e1tIicBKj1SjnonZW2lX1f5kPFgIHPHtNfBPJG/NCUIoCAEly0XzS/zJHnISxJ0WoxVMAqa6M0+AMhNnjd4Iqvx2fkruaoKNjYsf/b8s0BlMxobw== X-Forefront-Antispam-Report: CIP:63.35.35.123; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:64aa7808-outbound-1.mta.getcheckrecipient.com; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; CAT:NONE; SFTY:; SFS:(4636009)(376002)(346002)(136003)(39860400002)(396003)(46966005)(70206006)(36906005)(82310400002)(33656002)(478600001)(26005)(81166007)(70586007)(86362001)(83380400001)(82740400003)(356005)(2906002)(47076004)(336012)(8936002)(55016002)(9686003)(53546011)(186003)(316002)(6862004)(6506007)(4326008)(7696005)(52536014)(54906003)(5660300002); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jul 2020 09:37:48.4097 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: b33fd47c-9cb6-418e-6558-08d833a30e48 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: VE1EUR03FT004.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR08MB2764 Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix txonly mode timestamp intitialization X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Hi Slava, Thanks for your analysis. Some comments inline. > -----Original Message----- > From: Slava Ovsiienko > Sent: Wednesday, July 29, 2020 4:08 PM > To: Phil Yang > Cc: Matan Azrad ; Raslan Darawsheh > ; thomas@monjalon.net; ferruh.yigit@intel.com; > Honnappa Nagarahalli ; nd > ; dev@dpdk.org; nd > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix txonly mode timestamp > intitialization >=20 > Hi, Phil >=20 > Very nice comment, I found it is useful, I'm on moving mlx5 PMD to use C1= 1 > atomic primitives, thank you. > But, don't you think it is an overkill for testpmd case and would make co= de > wordy and less readable ? > How does testpmd start the forwarding? Let's have a glance at > launch_packet_forwarding(): >=20 > - initialize forwarding with port_fwd_begin() - It is tx_only_begin for t= xonly > mode, master core context > - launch the forwarding cores rte_eal_remote_launch(), the forwarding wil= l > happen in forward core context >=20 > Let's reconsider: >=20 > - tx_only_begin() is called once in master core context, forwarding is n= ot > launched yet, there is NO any > concurrent access to variables, we can set ones in any way, NO atomic, > volatile, etc. is needed at all. > We should do setup and commit all our memory writes - rte_wmb() seems I thought this barrier in this patch was only for synchronizing the 'timest= amp_init_req' between master core and forwarding cores. But in fact, it is more than that. I am clear now. I think it might be helpful to add some comments to this barrier.=20 > to be the very native way > to do it once for all preceding writes. Performance can be neglected here= - it > is not a datapath. Agreed. It is not in the datapath and it won't hurt performance. >=20 > - pkt_burst_prepare() is being executed in the forwarding core context, a= nd > perform only READ access > to the global variables, those are stable while forwarding lasts, and ca= n be > considered as constants. > No atomic, barriers, etc. are needed either. I agree - the volatile is > redundant ("over-reassurance"), let's > remove it. But atomic access - not needed, would make code complicated. If we remove the volatile qualifier, then these atomic builtins becomes unn= ecessary. Because all these 32-bits READs are atomic. Thanks, Phil >=20 > What do you think? Do you insist on atomic-barriered access implementing? >=20 > With best regards, > Slava >=20 > > -----Original Message----- > > From: Phil Yang > > Sent: Tuesday, July 28, 2020 19:24 > > To: Slava Ovsiienko > > Cc: Matan Azrad ; Raslan Darawsheh > > ; Thomas Monjalon ; > > ferruh.yigit@intel.com; Honnappa Nagarahalli > > ; nd ; dev@dpdk.org; > nd > > > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix txonly mode timestamp > > intitialization > > > > > -----Original Message----- > > > From: dev On Behalf Of Viacheslav Ovsiienko > > > Sent: Monday, July 27, 2020 11:27 PM > > > To: dev@dpdk.org > > > Cc: matan@mellanox.com; rasland@mellanox.com; > > thomas@monjalon.net; > > > ferruh.yigit@intel.com > > > Subject: [dpdk-dev] [PATCH] app/testpmd: fix txonly mode timestamp > > > intitialization > > > > > > The testpmd application forwards data in multiple threads. > > > In the txonly mode the Tx timestamps must be initialized on per threa= d > > > basis to provide phase shift for the packet burst being sent. This pe= r > > > thread initialization was performed on zero value of the variable in > > > thread local storage and happened only once after testpmd forwarding > > > start. Executing "start" and "stop" commands did not cause thread > > > local variables zeroing and wrong timestamp values were used. > > > > I think it is too heavy to use rte_wmb() to guarantee the visibility of > > 'timestamp_init_req' updating for subsequent read operations. > > We can use C11 atomics with explicit memory ordering instead of > rte_wmb() > > to achieve the same goal. > > > > > > > > Fixes: 4940344dab1d ("app/testpmd: add Tx scheduling command") > > > > > > Signed-off-by: Viacheslav Ovsiienko > > > --- > > > app/test-pmd/txonly.c | 11 ++++++++++- > > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > > > diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c index > > > 97f4a45..415431d 100644 > > > --- a/app/test-pmd/txonly.c > > > +++ b/app/test-pmd/txonly.c > > > @@ -55,9 +55,13 @@ > > > static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx packets= . > > > */ RTE_DEFINE_PER_LCORE(uint64_t, timestamp_qskew); > > > /**< Timestamp offset per queue */ > > > +RTE_DEFINE_PER_LCORE(uint32_t, timestamp_idone); /**< Timestamp > > init > > > done. */ > > > + > > > static uint64_t timestamp_mask; /**< Timestamp dynamic flag mask */ > > > static int32_t timestamp_off; /**< Timestamp dynamic field offset */ > > > static bool timestamp_enable; /**< Timestamp enable */ > > > +static volatile uint32_t timestamp_init_req; > > > > If we use C11 atomic builtins for 'timestamp_init_req' accessing, the v= olatile > > key word becomes unnecessary. > > Because they will generate same instructions. > > > > > + /**< Timestamp initialization request. */ > > > static uint64_t timestamp_initial[RTE_MAX_ETHPORTS]; > > > > > > static void > > > @@ -229,7 +233,8 @@ > > > rte_be64_t ts; > > > } timestamp_mark; > > > > > > - if (unlikely(!skew)) { > > > + if (unlikely(timestamp_init_req !=3D > > > > if (unlikely(__atomic_load_n(×tamp_init_req, > __ATOMIC_RELAXED) !=3D > > > > > + RTE_PER_LCORE(timestamp_idone))) { > > > struct rte_eth_dev *dev =3D &rte_eth_devices[fs- > > > >tx_port]; > > > unsigned int txqs_n =3D dev->data->nb_tx_queues; > > > uint64_t phase =3D tx_pkt_times_inter * fs->tx_queue > / > > @@ -241,6 > > > +246,7 @@ > > > skew =3D timestamp_initial[fs->tx_port] + > > > tx_pkt_times_inter + phase; > > > RTE_PER_LCORE(timestamp_qskew) =3D skew; > > > + RTE_PER_LCORE(timestamp_idone) =3D > > > timestamp_init_req; > > > > > > RTE_PER_LCORE(timestamp_idone) =3D > > __atomic_load_n(×tamp_init_req, __ATOMIC_RELAXED); > > > > > > > } > > > timestamp_mark.pkt_idx =3D rte_cpu_to_be_16(idx); > > > timestamp_mark.queue_idx =3D rte_cpu_to_be_16(fs- > > > >tx_queue); > > > @@ -426,6 +432,9 @@ > > > timestamp_mask && > > > timestamp_off >=3D 0 && > > > !rte_eth_read_clock(pi, ×tamp_initial[pi]); > > > + if (timestamp_enable) > > > + timestamp_init_req++; > > > > > > __atomic_add_fetch(×tamp_init_req, 1, __ATOMIC_ACQ_REL); > > > > > > > + rte_wmb(); > > > > We can remove it now. > > > > > > Thanks, > > Phil