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 B222141E05; Tue, 7 Mar 2023 12:43:09 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 52F6B40E03; Tue, 7 Mar 2023 12:43:09 +0100 (CET) Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11on2041.outbound.protection.outlook.com [40.107.236.41]) by mails.dpdk.org (Postfix) with ESMTP id 612774067E; Tue, 7 Mar 2023 12:43:07 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=S/H0qFk1XEhjb8n6bbA0WRcALw0PhCNq8K9hdTAaIzPya/I2yawniPfNiWdjJOYsMyL/sCgn8Sru9/CdMpThFBRD91ZsdZmDufNlQa87DZx4v6dBh1yTVTM/CRn0LNFPWIAXnyMlRxRwfc1rNt9tulnqcyfAb8MuJMgOeOW4k9bJD0AFq5RTXJB1Z9v56K4jjgvUnBxngDGKiUx/1E97zMeAFDmteg5xI00zuDXp7l8XHtXLtqEY3yOW3QHTZ8c3mOg+3ynBFEkl2b1pSObimU7E6vjdquPu+/96IhT+bycCN+SYAtlm7YWTplVWgOILYoisNEXJ1wDbBMtyy/fzPQ== 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=NIVABibi/T8qreQ59/kfhjmXLAxdFlNXVRuDnlnRWpc=; b=oTE0rgPVgJW1JTP+yX+OuHW7S0IhusA0pkunLPDA92TiOyNJLOec2dzEJIuLnJEWPXK59tRXzr4SJvNa3et3/fKXn32WinrkShn/9gkl7a2R6q4gRpu4a/sxnSRQY29+Z7pyMLFfqTDMDnuun6IEhr3k4QjrYFix08kW5u4DdDhjOxY3B620fbwRX+YCtsV38+0e+F94zIjZa3pwAXQuEQSWegRhNviU/bY8kZB83mZl210tlB4nXGhKHWdO8OdZrZTzfzkvYIDXN0Qcbw0Bk65rrX4S6AuQgVJ3I6z2bsWjOmScVJIG0v3j4OeX3Xy/OsiPwyys8MRtpexIK3POeA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=NIVABibi/T8qreQ59/kfhjmXLAxdFlNXVRuDnlnRWpc=; b=phwsiamZguaMPukCBxyZwp9oxpUOWXnbVgyOUFGLJze7oPOncXFMsouPjSLFy1MzLkwjksWXSM+3J2byqCDF68IBwH1vbixflRe636TUpHZNGaOOOq24r7SX0C0gIsv4sf8i+6ifY7WAaDNfWDwimHk6E/SX3CwCo8Acg1I46LY= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by CY8PR12MB7754.namprd12.prod.outlook.com (2603:10b6:930:86::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6156.28; Tue, 7 Mar 2023 11:43:05 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::dd5a:8a5c:f493:9640]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::dd5a:8a5c:f493:9640%4]) with mapi id 15.20.6156.029; Tue, 7 Mar 2023 11:43:04 +0000 Message-ID: <34b0ccca-f846-2ecd-61e3-0531037545de@amd.com> Date: Tue, 7 Mar 2023 11:41:16 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Content-Language: en-US To: "He, ShiyangX" , "dev@dpdk.org" Cc: "Zhou, YidingX" , "stable@dpdk.org" , "Zhang, Yuying" , "Singh, Aman Deep" , "Burakov, Anatoly" , Matan Azrad , Dmitry Kozlyuk References: <20221230075554.25244-1-shiyangx.he@intel.com> <20230223144106.707999-1-shiyangx.he@intel.com> <6b982bad-f9e6-62be-7a0d-30c7431889ad@amd.com> From: Ferruh Yigit Subject: Re: [PATCH v3] app/testpmd: fix secondary process not forwarding In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P265CA0034.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:2ae::8) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|CY8PR12MB7754:EE_ X-MS-Office365-Filtering-Correlation-Id: b1720251-d7b4-40cf-cc2c-08db1f011cd1 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Eg6DqvndtJRoCZO/hl0ql74c9dtNku7U4pTy5YhoUSoWwfnRVfZ/uY1bOIL1bWWJXHp5eyKBoK0gmwpkzhQ5/ZYHExSuXxP5LStyZyMTrezQBbVBvD7nujpkyWHymqljA+m1STSlJwyDrctqARbKJslLX5NWsRWYvvyFWT1ZzfRSU4C5CoKbWSKhBC4yhZ1vIA36zqtY4wWLE2U8sPe2c1NfKVDP1SHIWrkxkaV4JLk586aZfBPnW/3FsVioMAKwbX0uveJRmuWWlafa0acvTORuYkm0cA0aeoXdCduypD/F+gLKCtY82JP3lSMxeY9QG9Q6LslZXaNtTSZxAQb+5Cdd8cmyb6a2YKPfRNiemU7CMzb+/KuEMe+HSNp/hqnMgnipbA8/9R8UrUYkCpKJuzIQXT+Air8EA+w8J8R9jh7/LkNuXwtlCN/dLQjxA6kS8FeMPiBDdrQ9T/oSemGO+45n/uNtfI8Kb1Hg5f4Ql6dbOLqmtTDnLqdiSw7zmMPfIWhdHn42B9WTC33rHnD1XeXBeyzCDe1rSYVeTNuze5RFt4EYJhChPVQcqq3/dexkgXHEFaAfh2pyKRiqJ23ka0W6oBPf01rO1AhgDdLXDaLS4s/tFdNyuGVIJvO0rVH+4W/ooNDgsrLpNCBOjhE+Hpk9EgKfDzsgdOzcxDrumaaJ/o65aPt0D/ZG+Mctx9PwMVmV1B8bCabASi0o9ro8mFZ0jeBgHRpCySz+zI84Oig= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(346002)(396003)(366004)(39860400002)(136003)(376002)(451199018)(86362001)(38100700002)(66476007)(31696002)(36756003)(44832011)(66946007)(2906002)(5660300002)(41300700001)(8676002)(8936002)(4326008)(66556008)(2616005)(186003)(53546011)(26005)(83380400001)(6512007)(6506007)(110136005)(54906003)(478600001)(6666004)(316002)(6486002)(31686004)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MSt1TDJtbndpWEhqQkQ4Z08wZVdybXg4Qy95YTJrUDN4YUp5L1lmdlFLSHR1?= =?utf-8?B?VHcxV0tPVE8wK1RsV25BSFpSR1NKeGc5VUhzQ3R3MWtIaFpkNU04SUpOVnFa?= =?utf-8?B?VGllbFJSbGdkTWM2V204SWJhSFpjUGZoL1UvWmF4QkhKcTFFREp4M3FsRGR4?= =?utf-8?B?YkJPK09KQi9wMXgyTXM5Tm50U3IybzZ2OW5UVHJ3N2ljcXRnZjQyMVFwemVO?= =?utf-8?B?Nnd2NDcranF4d2RrbUIwQVZ5K2tuTGJhR3lac01rTkVFUVBXOElSZy9ETUpT?= =?utf-8?B?K3Y2WkxrVXVja1JXQnVuTlN2aVJyejJ6R2NvcEpTb016K09UV2NHR3Q1OFBq?= =?utf-8?B?QkljQWNwZU9Id1BmaVRNTVNaWTZaeGlGblNaalRkL1U1NkFhZFZMNk1COHpr?= =?utf-8?B?eFJKaDdUaHNVbEptQVJiMlNyV2orTjI0dlRLb2RJWVBpZjE2NERWdldlWkpL?= =?utf-8?B?SzN4UFFpemkvbUpUZUo2MVM3N0dUdExlVDI3aHBDa3FqdVlPOUY2K0lyVjhI?= =?utf-8?B?V1l0OXdESnpubjFuSzJ3cXBBNFJOYVpBY1VFTi91ZWN2L2FUamE3bkVOZy9a?= =?utf-8?B?b1hOMmFsOHRlWU1nQTQweHd4WUtnNGNuSUpYT0w2Q1pUMmpueTVET0F5TjRG?= =?utf-8?B?ajAzUTdkQ0piQzFiVFdiWk1OVnFWK0ZQTTczUHluV1VTVld5R3ZJOFA5Z0hv?= =?utf-8?B?U0pHU2NRV3FnOU04WTBHTmN2a3Q4aXorYllFTDk0QU9nQnk4SFlXVm9RZHlk?= =?utf-8?B?NDcyYkVsaTBBOERNVTFGN1d5eEhJVTVHeFhWU3pwY0JuU3JwQnVDZDh2WmVu?= =?utf-8?B?NEtCcXpxL2NleUNVYWJWeksxWHZXemRLOUU4QTZvaDV5bTl5bFRYLzl4YUNM?= =?utf-8?B?OFFoQVFqTjgzMWo3aHkxSFJUN1lJTm9yQldGYjFsaXRrN3lLaWJHblVEbFVm?= =?utf-8?B?eUYxc2UyekdNSGE0VWdDWUdNQklVSUZoc2FHTE40Z1lMUERDeFBFaHZTT0dQ?= =?utf-8?B?UTRYY1M3SnoyVmxFWFFWT2hXOGdyQWpQemJqaWI1NXRjdUhBbXplZ3hzZEdl?= =?utf-8?B?a0FQS1ZVTkpVam15RGo3ZG94VUdMZkN0UElxWHQvaHdXcDlrMStpRlp2MWNr?= =?utf-8?B?NkczY3RQZWhWSzJndjM0bUZTcUd5TlpUVVlLODV6QTBTSTdVY2dSREtvQ002?= =?utf-8?B?M0EyclkzNDN2c0EydEFhS0hmY3BCcGNIcmZTT05CdFZvYUFtQU1tRHNkSWM2?= =?utf-8?B?bCtKTVcyMWZPUjdmN01DQUlFZGNzK0lhd0RndXlnZ1gxcS9uTDdVUnNOR0RB?= =?utf-8?B?MFE1RE9hV21Oenc5dUJ1akN4ekZNdFFCbmhVSFk2eEpuMFIvMmFUOUxJeGJz?= =?utf-8?B?VDNYUlp2UjU0NHhIbyt4MUpCVkovRXFoc3JLck03MXppTU5aWnVnZ1pobE93?= =?utf-8?B?QmdndjFpbExKako4eW5FTGh6Z0JNREhrQkdKYVlDV202NDNWcEZSM2ZTVmxX?= =?utf-8?B?M1NNRjAwQzlvZStSVStzYXgvaHBIY3dRc2VkTXVFQ0lyQkRrSUxNUFpNUFBL?= =?utf-8?B?Yjh2VzNaUEx4TFgrUjdDNmMwYWJWVnBreXR5NWNadkp1bDJRV0lIdmNyQUhj?= =?utf-8?B?Z0xJZVNqbExHNk1Cb05oYmpiWHhBcXF0TnlNY0JzNnJwbzI4d3V1cjVwcUN6?= =?utf-8?B?d2tIQXFKUWJkV2VENTNaWmFvT3pKUDBDcE4yL1NPZ1BWRWxXK3BUWUNSaEZm?= =?utf-8?B?VS9RNGJDQkdGM1B5WGhQQU43REFzeWtIaVdPZmNXYUZ3cVJUVzRRT1lMZW1t?= =?utf-8?B?bURFeHE2YjhyaEZtbm5SbDFGYjhhcjVvSDVlb3FiMTF1K0l3WWlzU3QyRDNa?= =?utf-8?B?d3pzT3RhcTdYQ0o5d3VMZjRLU3FBNmZSejg5TUJQbTI0RkpCbFM4NVp4ek53?= =?utf-8?B?WVl4Tmh4L3VOQnJNSmYrT1A1bDRvc01HL1p6WTE5RWtjeVZFWTgwczIxNzRH?= =?utf-8?B?Um02Tk5mUXhhbFo5dFVJa21hdWlqR0pHWkF1TytDb1lIYWJmaE4wV0hrTnpo?= =?utf-8?B?b3FOYnk0Z1R6NlBTVVV1LzlKcUpnYUh4VUN0WDhnVUgrOFIrMXBPeFpMWDI0?= =?utf-8?Q?XHN+sS5bbgNprAMN8hRdIZqU2?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: b1720251-d7b4-40cf-cc2c-08db1f011cd1 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2023 11:43:04.4497 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 4/vbx1HIsqGFvMow/QlAXm7BsnP7ofBLpUq2lEdK78CfWSc+SILjFLOz3gZbvo2/ X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB7754 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 3/7/2023 3:25 AM, He, ShiyangX wrote: > > >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Monday, March 6, 2023 11:06 PM >> To: He, ShiyangX ; dev@dpdk.org >> Cc: Zhou, YidingX ; stable@dpdk.org; Zhang, Yuying >> ; Singh, Aman Deep >> ; Burakov, Anatoly >> ; Matan Azrad ; Dmitry >> Kozlyuk >> Subject: Re: [PATCH v3] app/testpmd: fix secondary process not forwarding >> >> On 2/23/2023 2:41 PM, Shiyang He wrote: >>> Under multi-process scenario, the secondary process gets queue state >>> from the wrong location (the global variable 'ports'). Therefore, the >>> secondary process can not forward since "stream_init" is not called. >>> >>> This commit fixes the issue by calling 'rte_eth_rx/tx_queue_info_get' >>> to get queue state from shared memory. >>> >>> Fixes: 3c4426db54fc ("app/testpmd: do not poll stopped queues") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Shiyang He >>> Acked-by: Yuying Zhang >>> >>> v3: Add return value description >>> --- >>> app/test-pmd/testpmd.c | 45 >>> ++++++++++++++++++++++++++++++++++++++++-- >>> 1 file changed, 43 insertions(+), 2 deletions(-) >>> >>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index >>> 0c14325b8d..a050472aea 100644 >>> --- a/app/test-pmd/testpmd.c >>> +++ b/app/test-pmd/testpmd.c >>> @@ -2418,9 +2418,50 @@ start_packet_forwarding(int with_tx_first) >>> if (!pkt_fwd_shared_rxq_check()) >>> return; >>> >>> - if (stream_init != NULL) >>> - for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++) >>> + if (stream_init != NULL) { >>> + for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++) { >>> + if (rte_eal_process_type() == RTE_PROC_SECONDARY) >> { >>> + struct fwd_stream *fs = fwd_streams[i]; >>> + struct rte_eth_rxq_info rx_qinfo; >>> + struct rte_eth_txq_info tx_qinfo; >>> + int32_t rc; >>> + rc = rte_eth_rx_queue_info_get(fs->rx_port, >>> + fs->rx_queue, &rx_qinfo); >>> + if (rc == 0) { >>> + ports[fs->rx_port].rxq[fs- >>> rx_queue].state = >>> + rx_qinfo.queue_state; >>> + } else if (rc == -ENOTSUP) { >>> + /* Set the rxq state to >> RTE_ETH_QUEUE_STATE_STARTED >>> + * to ensure that the PMDs do not >> implement >>> + * rte_eth_rx_queue_info_get can >> forward. >>> + */ >>> + ports[fs->rx_port].rxq[fs- >>> rx_queue].state = >>> + >> RTE_ETH_QUEUE_STATE_STARTED; >>> + } else { >>> + TESTPMD_LOG(WARNING, >>> + "Failed to get rx queue >> info\n"); >>> + } >>> + >>> + rc = rte_eth_tx_queue_info_get(fs->tx_port, >>> + fs->tx_queue, &tx_qinfo); >>> + if (rc == 0) { >>> + ports[fs->tx_port].txq[fs- >>> tx_queue].state = >>> + tx_qinfo.queue_state; >>> + } else if (rc == -ENOTSUP) { >>> + /* Set the txq state to >> RTE_ETH_QUEUE_STATE_STARTED >>> + * to ensure that the PMDs do not >> implement >>> + * rte_eth_tx_queue_info_get can >> forward. >>> + */ >>> + ports[fs->tx_port].txq[fs- >>> tx_queue].state = >>> + >> RTE_ETH_QUEUE_STATE_STARTED; >>> + } else { >>> + TESTPMD_LOG(WARNING, >>> + "Failed to get tx queue >> info\n"); >>> + } >>> + } >>> stream_init(fwd_streams[i]); >>> + } >>> + } >>> >> >> >> Testpmd duplicates some dpdk/ethdev state/config in application level, and >> this can bite in multiple cases, as it is happening here. >> >> I am not sure if this was a design decision, but I think instead of testpmd >> storing ethdev related state/config in application level, it should store only >> application level state/config, and when ethdev related state/config is >> required app should get it directly from ethdev. >> >> It may be too late already for testpmd, there is a mixed usage, but I am for >> preferring this approach when there is an opportunity. >> >> >> >> For above issue, why queue state needs to be stored in application level 'port' >> variable? >> Where is this queue state used? >> >> Can it work to get queue state directly from ethdev where this state is used, >> instead of storing it in the 'port' variable in advance? >> >> And perhaps testpmd 'port' variable can be updated there, both for primary >> and secondary, for backward compatibility (other existing users of this queue >> state). >> >> What do you think? > > Thanks for your comments! > > It is an effective method to get queue state directly from ethdev where this state is used. > I also don't know the design meaning of the 'ports' variable. If modification is needed, > a higher level of design and more work are required. > > As a bug fix, apart from extracting the code block into a function, is the solution feasible? Hi Shiyang, As a bug fix, this issue (testpmd stored state not being up to date for secondary process) looks like have potential to occur many different flavors, that is why what about having a central update? I think 'start_port()' can be a good place for this kind of update: start_port() { ... if (secondary) update_state() } update_state() { update_queue_state() } update_queue_state() { } Having secondary checks and updates in multiple places can make code harder to understand. What do you think to update as above?