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 077FB41C45 for ; Wed, 8 Feb 2023 23:55:01 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EBFBF410EE; Wed, 8 Feb 2023 23:55:00 +0100 (CET) Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2089.outbound.protection.outlook.com [40.107.243.89]) by mails.dpdk.org (Postfix) with ESMTP id E1A3140141; Wed, 8 Feb 2023 23:54:58 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=i59ccOeU50OaUg0UvLidJF7uUWj9kHfymZyzxNPcZcBZpXfiqI0+g+Ob3+n4RxVhEXzX7km/mHl7m0AkMC06uDwH8zvGsMhBYFloiv+0FQ7y6iMs+hbSxf/mb1tmBlp3hEGB0jhZw03MUI+JBDJXA34N3VsDoAqXkIxdoG3svvIyx+O1jo7tju3/NvR0tKwNhVQs7puJep6UUwHtuCH3MFUOwnaFpuNmCeAD+w9yRuF3JcPCAall1qyQJWp1uhH1bgYDxCx7qenz6XRBPWrNkBmaeHBWbI05yNAcvTkfSuELPT3ZouvtcBBVpJCPZ9ud5vHIxqxD7s9S+h8SWUUtTA== 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=iolQn61ehWYTu6DyD+YmI2qxIQOoAwB8x7DAu25xDys=; b=Mo6wi31blT5fSac8G8LvyQ7Bi3SqC/UUwpV1UEc6WMTV0/H/W5lH1P5ADisErOf22kA70spdla1NKAM4cOUBXC/mp8hx1Jua8ugipbyEVsTFJ9GM27J/Pt3Qfyw6MTvyt2ZUl1TusQ3K3PZbzRBmuXu6WmMi3MhgOF6O9E5HTQsPuAEaof2oWarFYUjpapdSgRhqHkOKZ70M9/qJbHJggHveudFiemosokJatEnPZAI6IxXYaPQUH2AJnZBYSvjXrb+zV8vGYCl9BkWmoO9kF1CAvvIFAQyNmu5BswvDNUjmTcV8uteWp/d0/d8k34Ze9ghtbsh0STvSjiOI8mT7MQ== 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=iolQn61ehWYTu6DyD+YmI2qxIQOoAwB8x7DAu25xDys=; b=ugGATbM1DbBV2blANaz60qNLJTm431zZApvYzAyQdGV15dgOkxfELEB80niPUD+yeUBBbJ4Rr7ULJB84ExbLiO4vU0GVlWALWAkesM1gj+O59Nhwref9k1BryCqEXNWbx/Vnj1q9y/DEz4FvX6SenHznzJuQVXYtj5eIPwuoD8w= 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 PH8PR12MB6963.namprd12.prod.outlook.com (2603:10b6:510:1be::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6086.17; Wed, 8 Feb 2023 22:54:56 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::3614:22ed:ed5:5b48]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::3614:22ed:ed5:5b48%6]) with mapi id 15.20.6086.017; Wed, 8 Feb 2023 22:54:56 +0000 Message-ID: <09aa00fa-d040-a061-46a9-7ad85a49c159@amd.com> Date: Wed, 8 Feb 2023 22:54:49 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH] app/testpmd: fix link check condition on port start Content-Language: en-US To: "Singh, Aman Deep" , Yuying Zhang , Michael Qiu , Pablo de Lara Cc: dev@dpdk.org, stable@dpdk.org, michael.qiu@intel.com References: <20230127224514.2650827-1-ferruh.yigit@amd.com> From: Ferruh Yigit In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P123CA0575.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:276::7) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|PH8PR12MB6963:EE_ X-MS-Office365-Filtering-Correlation-Id: 8ba81c16-2b32-400d-2dd4-08db0a277f34 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 0ZzkNoZHaQMs1IzFYiDGWLF3SIOiXkOfmOt0ELz90LjZy+e7jks00IZsgLiCZxJz89ZePGP32yjlm70KQQ/8c/c0DLKw77vymsYkojL+fjgcx9h69yNbfxj0aviK5o1IRhg4kit1ajWciBe1RXNb7ksyTtqf7pVwG6g4fb2xh8w/T0/IW6LF7iN9bauNxTPZbfqEMzJaUL+6iPDMawYzf4Jo7SHqxv5TiU0hGX2IJX7PMu1my6DdxU9fWNcvCvTt1szDJmnAWP1RoJNaCtDysR+T5jHXnHuMbL6vK4nwoJlAClhxryTtco5NNuJBIfMMuTTanlvxahiKQj2vtkQZj3vSSqcdFYSDAGGiVbLA7qhakWvsmRhXc95eupy1xPQz77FM+4rPv0+5smiPe8a9yH5112xID+1taOJRx/JlFUQvK5gB8iAm4WemH/7a4sibIxxTUwxJNx6tzRQiLLiHJ9N7gOxMy354D1UPfTciSKpwpj7TGD7aUVqoOT3RW8QWF+V9ThvNjdyXXavfq+gbv+3bQmtp7bgN1MgfsbRVkilrmiYdVh9ztDCLHhJnX6/EGT45KdH4wZmyfkvT4u13GoKl/w2iszyZod89NdMiIgkJulHWJxQpXeiEdwzauFgbpj3jAitWn+0fnQukRsK9Y49sXg3O9B5HhqdW3nEaIlYIwPCUK2acwY0SepqRfbF0SyFjOuybqx8d1pYBtD1gg2hYkr1XdVCRnTNQXo5cVVw= 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)(136003)(396003)(39860400002)(346002)(366004)(376002)(451199018)(66476007)(4326008)(66946007)(8676002)(316002)(38100700002)(66556008)(83380400001)(31696002)(110136005)(86362001)(5660300002)(6486002)(31686004)(478600001)(6506007)(53546011)(2906002)(44832011)(6512007)(26005)(186003)(36756003)(6666004)(41300700001)(8936002)(2616005)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?U2szaUsxZ1I4K2lQL3VWcGJMQkhWdklTdzUrUVFsNW9aSmtuQW1qdDRGREpU?= =?utf-8?B?dUgxY0pibmwyVkxYc1VJWXhreW1nb1BRZTlOeDNVT2pBZzhCd0dRTGhBT0tJ?= =?utf-8?B?V0wycFdEdHovUXk4WjliNDlVL2lRa3VMSGQwZFBYY3BkREs5MXVoSHJ4ODla?= =?utf-8?B?V0pkUzZpbnROM2VCMStJcXdRMm0zZzhOSUl1RUIxekpBMXg2QVNBOWFVMDJH?= =?utf-8?B?bnJrcFN2cGhyOWZjK0VpOGw4K3dwM1Z6M1M1VzNaTnlkWmtxTVJNQU95bWsw?= =?utf-8?B?WEMyanlGUkVOQVdmOVcvUzRzbVV3b1UrcW9UNUFqb21qaTN4R29CZ2dDZnZG?= =?utf-8?B?MHp5NEFLaUxSUm9mZ2daaDdRdG4vSGpGdUkxbHBvOVd3Z3BZRmZrRHZveGUz?= =?utf-8?B?SnpGbVVERjZYTVVINWVnbnZFaTJRbVFqVlZFTE8yR0VrczZsU2ZUeDFHbmtY?= =?utf-8?B?Uk1CdGtoUG5lTm9vSmZEYTRINDhQeko2S3lnY1hER3l5T1Qxd0xIWDZRTjdu?= =?utf-8?B?aVlKaC9IOE5UVHdFTEtaUk1LMzBhU2NYanVUd1E5Z2lVQjlZcVRmcElNYzNu?= =?utf-8?B?NzhpcWRLV0FodlRQY3FIaWVJUkJhWWJMTzRLN2YxS25Wd3RNUGc2SStzZlBx?= =?utf-8?B?RENnaGZLcVFENzJib01BWlZuclphWWN3NSt2eXB3VHZ3VFExeGd3ZCtCTURu?= =?utf-8?B?SGpwQkV5Q0pGZ2paUENTT1VWZytaRUtHYnRwSnlPYTN1aWVSclF3bWNsMEJ6?= =?utf-8?B?V3l4dkxadXhjYTZVZ2ljbVlRZ3IxcTBrMG5uc1N0VTJyQ2U3SUdRTjA4WW85?= =?utf-8?B?MmduRDFmTUViTFpGamRpWWNVYTdjVWg0OVdRSXplWEJ0VVB5VDRSR1AySUpZ?= =?utf-8?B?Ky9YcWkzMHVMQkVZZ1ZySFJvOEI5MEZ5ODZvL1IvYVcwMlVIbGZTR0N4RVdh?= =?utf-8?B?QUpnTk5PV2JlYVdNVkMyRnE0KzZKalUwOE5saWFRMVBCU0VUcFphR01yL2dI?= =?utf-8?B?aFI5OWJIL2lZc2QvejJpVmpNNGNSeWpkdHdIS3dySkZGQU15ZkVmZFlwenNB?= =?utf-8?B?WDJYVU51QnR5SmNBbDhPRnBHamovbmNjNitwcHVMRGhNWWJvQWk1cksvcEth?= =?utf-8?B?UTZUNjhhSHo1Z25xZ0N2T3NpZTlCZVhweHVEdmJpaVZpVVc5Z1NUTUJsZFFj?= =?utf-8?B?ZTJnR2ZzUzVJVG01YzZYaHlUYUt6QmZLcTFUVzluVHhCeHNTeFhvNVd1TVp0?= =?utf-8?B?NWUzTkEvTmdZSGc3VUZSMFRicm41ejFzb2loNWxDeXVkUWZXMytaN1R3NmRH?= =?utf-8?B?cTFleDFjVGFwVmdNVWYxNDJMZVE4U3FoMWlvVW9xRWJuYVorYXM1N3ZhcndQ?= =?utf-8?B?bndRdjlvK3R3cjN1ZlpWbEluSDMyQTVkaGk5RWdrZUJPcFFrMHVhYzh0KzN1?= =?utf-8?B?M0UxNjZUYVBGcUJTS1hIOUtIOGloSjdZN3Y0R2tPNityT1dmeVpsSG11NklO?= =?utf-8?B?RFlEcFQ2K3NUV0RQeWR2SWFSV1JrSURXTVdYeHdFc1BhV1BxSThldHp3WVZz?= =?utf-8?B?T1VUKzdic0NXc0o2WnhlK3diSXVPeUdLbmpMYm1yK25CV3pBb2pnYXFuYlZU?= =?utf-8?B?NDl6bXpObEM5SW4zdVFsYng1V3FNb1dERHpuc052U2xFdWxiWThQNlhKSmMw?= =?utf-8?B?YWt5U0tJNmxjekY2SEdlRmhDZVljbTdrSExRcXVNQnV5YXdzV3FGZmwxYUJ3?= =?utf-8?B?TnQ4OEF6UU5YUVAwWkM3eWFzazVqeWpmYnVTeTduRGRiMzNscC9jT1liSXdK?= =?utf-8?B?bzVrbjZNN01NL096MUk2QTFEeDR1dmNEVWZNVi9ZTTh5OSs5aEIydmpIZ20z?= =?utf-8?B?eUd0NmUwWkRQb0dBUUgwblNIZDN4U3kvam9xY1M5YUM2aFVOckFPUmRhRGNi?= =?utf-8?B?Wmp1ZU41RUpLNEM2SlhhSmVVU3JNR210NmdPamF0aVFoN2xPZ0xXbkZicjJ5?= =?utf-8?B?cnFweE8zZGNuOEN4bnpBNjhnMzNwa3pSOWh6OS9YQUx1MkRoLzlQd0pnN3Jq?= =?utf-8?B?SEl3M2s0a1ZCVVh1a2RKRmhBZGJYdG85TkdQUEZ3dHpjL3IxeENDU0hJZUJC?= =?utf-8?Q?kN9qSUo6n9jxqGggoGc/UXSLF?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8ba81c16-2b32-400d-2dd4-08db0a277f34 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Feb 2023 22:54:56.0636 (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: m8jognfgNaMJ7knNPKPZPjNuX9lsZ+QwMyBK8NA243ucMASsVKvESH0+J9OgVdI+ X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR12MB6963 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On 2/3/2023 9:56 AM, Singh, Aman Deep wrote: > > On 1/28/2023 4:15 AM, Ferruh Yigit wrote: >> In testpmd port start function, 'need_check_link_status' variable is >> used to detect if a link check is required after port is started. >> >> Intention is if at least one port is started, link check should be >> called, and initially 'need_check_link_status' used as following: >> ``` >> start_port >> need_check_link_status <- 0 >> for each p in port >> ret <- config & start p >> if ret is failure >> break >> need_check_link_status <- 1 >> if need_check_link_status >> check link >> else >> log failure message >> ``` >> >> Later above logic is modified [1] because when there is no port at all, >> 'need_check_link_status' remains zero and it causes and error message >> although this is a valid use case. >> >> For this code updated as following: >> >> ``` >> start_port >> need_check_link_status <- -1 >> for each p in port >> need_check_link_status <- 0 >> ret <- config & start p >> if ret is failure >> break >> need_check_link_status <- 1 >> if need_check_link_status == 1 >> check link >> else if need_check_link_status == 0 >> log failure message >> ``` >> >> This modification works fine if 'start_port()' called for a single port, >> but function support both single port and all ports with 'RTE_PORT_ALL' >> parameter to the function. >> >> When it is called for all ports, above logic is wrong because >> 'need_check_link_status' value reset on each iteration of the loop. >> >> For multi port case, if last port fails to start, >> 'need_check_link_status' will be zero and no link check will be done and >> it will log a wrong error message. >> >> Overall there are three cases to cover: >> * No port exist at all >> * All ports are already started >> * At least on port started successfully > > Minor typo, on => one > Fixed while merging. >> To cover all three cases, one option is to use 'need_check_link_status' >> have multiple values to reflect above cases. >> But meaning of values are not obvious which can lead more issues in the >> future. >> >> Instead converting 'need_check_link_status' to multiple boolean >> variables whose names are self explanatory. >> >> This fixes issue and link check called if at least one port started >> successfully as intended. >> Also log message only printed when at least one port exists and all >> ports are already in started state. >> >> [1] >> Fixes: 92d2703e2c43 ("app/testpmd: fix log with no bound device") >> Cc: stable@dpdk.org >> >> Signed-off-by: Ferruh Yigit > > Acked-by: Aman Singh > Applied to dpdk-next-net/main, thanks.