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 EFBEFA0C54; Tue, 10 Aug 2021 11:12:47 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A3FCD40686; Tue, 10 Aug 2021 11:12:47 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id B97404014F for ; Tue, 10 Aug 2021 11:12:45 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10070"; a="275899730" X-IronPort-AV: E=Sophos;i="5.84,310,1620716400"; d="scan'208";a="275899730" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Aug 2021 02:12:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.84,310,1620716400"; d="scan'208";a="439235104" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by orsmga002.jf.intel.com with ESMTP; 10 Aug 2021 02:12:44 -0700 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) 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.10; Tue, 10 Aug 2021 02:12:44 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10 via Frontend Transport; Tue, 10 Aug 2021 02:12:44 -0700 Received: from NAM04-MW2-obe.outbound.protection.outlook.com (104.47.73.169) 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.10; Tue, 10 Aug 2021 02:12:43 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CD02b/cqaL4say3WfNipgGjB8JQr0eLBSoyF1vUxXSB0+/4DKeW/O0otizzaVRO/PIHViefhfbWkFnDzeJEn1b9G5ubN1+60w9PNpTGIWN9RG1CZF7rYi/oVrX9SS673KmeF+luf5Xa59XQWkNge0qM3bn3LHqg3mnOtIAXJfF4WpdcKxQwVnwOkn9eKNksUWB+SHg6xyEqIOO8W1vioSPxldA1S43QSshSuMfHb2LS1YyASMZtCo7ql8V8uyo8/D1MGrvnyKkOot6CZwX/RM5ZFk8HDU8E+TCVuvio0zth2CUI85xNxk3SrPt1dCQph3Mer+hU+Sfx+M0u6ZIEASw== 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=j/uxqlLtvwCnkcC6qgU+h9+QW836El9F1ovTjUjtMuk=; b=WLm8CsLUE0xBMXydEqR4+D66dIOoKERwYPmAYzY/FtNLbXF4p+L3obpQkRxIoDYiyxuKipp9xe8VQDktVZsfAem4FFN8BNLlhOCS3j+T7cQISg3bWQFJDu5PRTpqm/mgU340PU9xJXLH2lu1KS8y8WAeeEisJR/9tnjxjSSdeOHbTnaE4CFNW7AYe+KttJAP3cPdvMxRwbGvTl9iQ7LQvPTchUyV8t7B5EgZ7oUsfi4obyNePKdM2YoQ+ntYRefAFsGTy3AG6I2q2aw1+Hoi2fe1RWkvNlESdv0eC7cUAzO0sYkz/fDT1xpIQq4Bcy5QVaSb9rG8tEjsL/szju92Qw== 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=j/uxqlLtvwCnkcC6qgU+h9+QW836El9F1ovTjUjtMuk=; b=wgBqDRXAZ9MW0Qnb3g0VnOahz3xrgcqqzftn3c8SiCJD0pvUxfBZJJVIm6ew4R2XN2fAiRMzTNZlojtp7QOGgRC94KKVPaDLUoir5mILQgXcMW6I7CoBDmgcZQYPN0TxjKYblB2yrqPfi2Tz3lmWsIxSaZKSZTzGryRBs540/Dw= Authentication-Results: tilera.com; dkim=none (message not signed) header.d=none;tilera.com; dmarc=none action=none header.from=intel.com; Received: from PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) by PH0PR11MB5029.namprd11.prod.outlook.com (2603:10b6:510:30::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4394.17; Tue, 10 Aug 2021 09:12:43 +0000 Received: from PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::2979:70ca:38a:dbaf]) by PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::2979:70ca:38a:dbaf%6]) with mapi id 15.20.4394.023; Tue, 10 Aug 2021 09:12:43 +0000 To: =?UTF-8?B?546L5b+X5a6P?= CC: , , "Singh, Aman Deep" , Igor Russkikh , "Cyril Chemparathy" References: <20210809062548.30187-1-wangzhihong.wzh@bytedance.com> <20210809065200.31134-1-wangzhihong.wzh@bytedance.com> From: Ferruh Yigit X-User: ferruhy Message-ID: <09afb669-2152-6770-5f89-bebc36bad6e9@intel.com> Date: Tue, 10 Aug 2021 10:12:37 +0100 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-ClientProxiedBy: PR3P191CA0017.EURP191.PROD.OUTLOOK.COM (2603:10a6:102:54::22) To PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [192.168.0.206] (37.228.236.146) by PR3P191CA0017.EURP191.PROD.OUTLOOK.COM (2603:10a6:102:54::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4394.15 via Frontend Transport; Tue, 10 Aug 2021 09:12:41 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: bfcaf619-8236-4013-2753-08d95bdf0296 X-MS-TrafficTypeDiagnostic: PH0PR11MB5029: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: OOPr5XKnngHT8jGic51l7ZqpVIea14y9p8tfskBLmu58dGEZeJw5d5CavZAWn8ylommy2LMplCk+uUFsxMj+ZcIPJ9KhSt8NrVHVf0krorbWJ5UcLOgW1pZxLzc6aG/z7SQlKjcHFWD8mO/nOL/0HCRuIYjBeSc9wSyKuSp0Y8OzA7Rlzxiedok+3GDwRDf4guNymoPyh072TSPYMJuysprDVzFMbMvYSQItRsqYGSSuAetjOsWrxxymT5JO5J77Ifp8ja/9RnNrnE+v7/LsbzumMWiIucCpL+SNTmUpxTdM/7UTa/O0ngXcUF8H+A2f+cpTNAqXfhNmRhpw+xrUS1LwoVrdWckv5U33UirgHCJu/KS0YByeMd8WNDoAOLFv3kQb+qRI+Umq2RZa6CGq1PK6sTmPLq1UKwskhZ1Mx4hRG9gut/ldhQU2sBbB4iYwdQNKwbxKFr9FcyGGZmL9TBbvLIIIc7VGhXI7jsOvrCHfIUyxA5XBh/A6w0hbZfjfljHCb6AepjEepm52ziq7IZxO2pMFw2GgkQjLq/UUzwhhhjHj+/KIYR8OqbijhLOdArSZmRuZ/zJB7BAmZgVbdaP7MRM89xvLFXYwMbD9gfOQ/jabsGGfmgjoZvemTtatD6fivWis3+vVG6c/Qy5XiLBfMOeqZPmF2LaLbpLmHCQVShdwG+FCHorQTuOiNC1Rx1BME7bjumyZ//KOCOUs9w== 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:(4636009)(366004)(26005)(44832011)(66556008)(5660300002)(2616005)(956004)(83380400001)(186003)(53546011)(2906002)(66476007)(8936002)(66946007)(31696002)(508600001)(8676002)(316002)(54906003)(16576012)(6916009)(4326008)(6486002)(36756003)(86362001)(38100700002)(31686004)(6666004)(45980500001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a3NEcW9oQnlZb0lGUmdkM2dqaGpYVFBJL2JJZWgwRHdYSXg0eG85WjRlMUFU?= =?utf-8?B?cVlrUUdvTjJvSkhIUHh0RTFtTzcrdnk1Smljcmg1a3dsMFNpZjRuVXIySjhY?= =?utf-8?B?em02bkVQVnZsSmdtaDN3RU5jaStRcWN2S1lIekhFUE1pSWo4QzlYVUlaNU02?= =?utf-8?B?bDJsL0p3alR3bW50QTJXVkU2bVlSSWludVNxM3B6azVjcldXekt3aVZ1djVV?= =?utf-8?B?SloxdkFCZEtpcVRRMmtGUWl2ZnI5R0JQT0dXTUQ2aTErRWNQNHdteE9DaUtN?= =?utf-8?B?bEFMaDVaeG9KQjZkOWxwc3p3SjB1Vi9UaUZNcHBaY0JVTW55dXE2T3hySGRX?= =?utf-8?B?MU5pWk9BQlFNVmRGSlhVeTlaYTRmdEZsSFpYVmdCOHIzNFpkYzZsOHZ5NDlJ?= =?utf-8?B?R01OM0lSTlBJa3ppWXl1N0dwSmJCbGVDTFp5SThrbGNHalYzcG1DQTBZa3dj?= =?utf-8?B?cXZ0Wk04N2JObC85NzFFQ1JuMTdKdzhMWU41SVNEVk16VEVoRE9EQk4zYXY3?= =?utf-8?B?MHd0dkpwMkhKZ2Rsb0lpZHQ4cm5BVENrMzZHckJ5NENoV0hsL0tLcUNLc1Rw?= =?utf-8?B?WVd5QjlGN1RxR3FaS3p1ZlJSVEtydk5VU01uZTh2YWtVTHJsN09aWVM4cHB3?= =?utf-8?B?MjhLUEtTSFprUWJlQThmcHlJS2JOSGp1Q2lnTnhpN0xQVE5KVWNjVlhhbGJX?= =?utf-8?B?bjBYVHFzM0dwenZXTkl4ODB4T3NtUkFQdENEdUplbVI4RmRNLzJhdHptWUh2?= =?utf-8?B?NW9INFJpSDB2OVlMNXhFaWUyMUIvdkZSUEduTE9PUzlOZDRzVE1henM0ajVo?= =?utf-8?B?WVN5UVh0SWoyMmp4d1JrT24zcXVUNHdDaUNOeCtjbzBacTFlOXh4T25QNy9H?= =?utf-8?B?Z1JzSTk1bm56TW11LzgweXBHRWpGaGlZQVo5b0dFUjdQd2Z5NlM3UStwR01o?= =?utf-8?B?amJwRXBZMVNreHROS1dXdm9ldUpsL2hQWU93SlJpUHZiYjFNYTBVV0RHOEhr?= =?utf-8?B?NnM1TVN0ZGpWSmtkVkxEdDhYY2JQdWFac3B1WWpnK0w1eXZSYW1QMmpOL3dr?= =?utf-8?B?NXJvcVN3Qy8zdnZMSzBOOTJBeFZoNUJtdkdISDVjcnJ1ejZHWnppanoxbzc4?= =?utf-8?B?MndIaFVaV0gySEZKeFVwYTZHaThyTWxiM1VCejhwTDJsUGR0K05nRCtnbENW?= =?utf-8?B?by9jMjFpTWdwSllNbi9GTVpSbE5qTGppYVNvQlEwTFhqOFZvUjVLVXBxdTd4?= =?utf-8?B?elFSQ1lRdFpvQ2xTUG9icUZpTzU5TVhPb1ZrbFZYWVdqZlRkUHVSN3pHQUNM?= =?utf-8?B?QXJqSGh4NnAyNDdjU241dWd5STdUOGErUjAwcmE5M1l5Rnh2b0VCSFRMbkk3?= =?utf-8?B?MEFzVTh4NjlLVlZvbmZyN0hHbTQvZktla2liK2dnUnk4aHRCbFBSVDEyYUZO?= =?utf-8?B?L2hzNzJ2eEE4ZXRJWEJNY3VCWjdKeGcwYVRwNHZncG9SbkFZNHJmVTBCeGYv?= =?utf-8?B?SUwxMGR1MlBxR0F3d3lxRGJ5cXBlM0ZMRmxvdzZKTkRrTDhEQXRkNkU2dTJQ?= =?utf-8?B?amx4Sm9PclhqaTliaVNKaWluM2JIalJvazY5ajFhVWtQNS81MVZ1ayt4dkh5?= =?utf-8?B?QXpCQnJpRU5lNDlmNWdJTGQzVWd5MHk1RVJ4cWxiSzVrbGVaMituOC9Ucmli?= =?utf-8?B?TWZjWEM5UGVMRDVudXNtVnRsaGNHNG1TeEVmSjRpbTBqKytKUmlUbERreWYy?= =?utf-8?Q?l00zZkAGDf4eM/RSo1nSspkMR81fwqQABKxF4xS?= X-MS-Exchange-CrossTenant-Network-Message-Id: bfcaf619-8236-4013-2753-08d95bdf0296 X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5000.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Aug 2021 09:12:43.0269 (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: 3YRS7I2iP+XwIWIBh9ndWcdGudACNGGESTn7xK+vmsUiSIZcA16qJQ25Db/BBnJKQovLTBSC49L9U4MAXqSCkQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5029 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [External] Re: [PATCH v2] app/testpmd: flowgen support ip and udp fields 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 8/10/2021 8:57 AM, 王志宏 wrote: > Thanks for the review Ferruh :) > > On Mon, Aug 9, 2021 at 11:18 PM Ferruh Yigit wrote: >> >> On 8/9/2021 7:52 AM, Zhihong Wang wrote: >>> This patch aims to: >>> 1. Add flexibility by supporting IP & UDP src/dst fields >> >> What is the reason/"use case" of this flexibility? > > The purpose is to emulate pkt generator behaviors. > 'flowgen' forwarding is already to emulate pkt generator, but it was only changing destination IP. What additional benefit does changing udp ports of the packets brings? What is your usecase for this change? >> >>> 2. Improve multi-core performance by using per-core vars> >> >> On multi core this also has syncronization problem, OK to make it per-core. Do >> you have any observed performance difference, if so how much is it? > > Huge difference, one example: 8 core flowgen -> rxonly results: 43 > Mpps (per-core) vs. 9.3 Mpps (shared), of course the numbers "varies > depending on system configuration". > Thanks for clarification. >> >> And can you please separate this to its own patch? This can be before ip/udp update. > > Will do. > >> >>> v2: fix assigning ip header cksum >>> >> >> +1 to update, can you please make it as seperate patch? > > Sure. > >> >> So overall this can be a patchset with 4 patches: >> 1- Fix retry logic (nb_rx -> nb_pkt) >> 2- Use 'rte_ipv4_cksum()' API (instead of static 'ip_sum()') >> 3- User per-core varible (for 'next_flow') >> 4- Support ip/udp src/dst variaty of packets >> > > Great summary. Thanks a lot. > >>> Signed-off-by: Zhihong Wang >>> --- >>> app/test-pmd/flowgen.c | 137 +++++++++++++++++++++++++++++++------------------ >>> 1 file changed, 86 insertions(+), 51 deletions(-) >>> >> >> <...> >> >>> @@ -185,30 +193,57 @@ pkt_burst_flow_gen(struct fwd_stream *fs) >>> } >>> pkts_burst[nb_pkt] = pkt; >>> >>> - next_flow = (next_flow + 1) % cfg_n_flows; >>> + if (++next_udp_dst < cfg_n_udp_dst) >>> + continue; >>> + next_udp_dst = 0; >>> + if (++next_udp_src < cfg_n_udp_src) >>> + continue; >>> + next_udp_src = 0; >>> + if (++next_ip_dst < cfg_n_ip_dst) >>> + continue; >>> + next_ip_dst = 0; >>> + if (++next_ip_src < cfg_n_ip_src) >>> + continue; >>> + next_ip_src = 0; >> >> What is the logic here, can you please clarifiy the packet generation logic both >> in a comment here and in the commit log? > > It's round-robin field by field. Will add the comments. > Thanks. If the receiving end is doing RSS based on IP address, dst address will change in every 100 packets and src will change in every 10000 packets. This is a slight behavior change. When it was only dst ip, it was simple to just increment it, not sure about it in this case. I wonder if we should set all randomly for each packet. I don't know what is the better logic here, we can discuss it more in the next version. >> >>> } >>> >>> nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_pkt); >>> /* >>> * Retry if necessary >>> */ >>> - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) { >>> + if (unlikely(nb_tx < nb_pkt) && fs->retry_enabled) { >>> retry = 0; >>> - while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) { >>> + while (nb_tx < nb_pkt && retry++ < burst_tx_retry_num) { >>> rte_delay_us(burst_tx_delay_time); >>> nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue, >>> - &pkts_burst[nb_tx], nb_rx - nb_tx); >>> + &pkts_burst[nb_tx], nb_pkt - nb_tx); >>> } >> >> +1 to this fix, thanks for it. But can you please make a seperate patch for >> this, with proper 'Fixes:' tag etc.. > > Ok. > >> >>> } >>> - fs->tx_packets += nb_tx; >>> >>> inc_tx_burst_stats(fs, nb_tx); >>> - if (unlikely(nb_tx < nb_pkt)) { >>> - /* Back out the flow counter. */ >>> - next_flow -= (nb_pkt - nb_tx); >>> - while (next_flow < 0) >>> - next_flow += cfg_n_flows; >>> + fs->tx_packets += nb_tx; >>> + /* Catch up flow idx by actual sent. */ >>> + for (i = 0; i < nb_tx; ++i) { >>> + RTE_PER_LCORE(_next_udp_dst) = RTE_PER_LCORE(_next_udp_dst) + 1; >>> + if (RTE_PER_LCORE(_next_udp_dst) < cfg_n_udp_dst) >>> + continue; >>> + RTE_PER_LCORE(_next_udp_dst) = 0; >>> + RTE_PER_LCORE(_next_udp_src) = RTE_PER_LCORE(_next_udp_src) + 1; >>> + if (RTE_PER_LCORE(_next_udp_src) < cfg_n_udp_src) >>> + continue; >>> + RTE_PER_LCORE(_next_udp_src) = 0; >>> + RTE_PER_LCORE(_next_ip_dst) = RTE_PER_LCORE(_next_ip_dst) + 1; >>> + if (RTE_PER_LCORE(_next_ip_dst) < cfg_n_ip_dst) >>> + continue; >>> + RTE_PER_LCORE(_next_ip_dst) = 0; >>> + RTE_PER_LCORE(_next_ip_src) = RTE_PER_LCORE(_next_ip_src) + 1; >>> + if (RTE_PER_LCORE(_next_ip_src) < cfg_n_ip_src) >>> + continue; >>> + RTE_PER_LCORE(_next_ip_src) = 0; >>> + } >> >> Why per-core variables are not used in forward function, but local variables >> (like 'next_ip_src' etc..) used? Is it for the performance, if so what is the >> impact? >> >> And why not directly assign from local variables to per-core variables, but have >> above catch up loop? >> >> > > Local vars are for generating pkts, global ones catch up finally when > nb_tx is clear. Why you are not using global ones to generate packets? This removes the need for catch up? > So flow indexes only increase by actual sent pkt number. > It serves the same purpose of the original "/* backout the flow counter */". > My math isn't good enough to make it look more intelligent though. > Maybe I am missing something, for this case why not just assign back from locals to globals?