From patchwork Fri Jan 27 22:45:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ferruh Yigit X-Patchwork-Id: 122607 X-Patchwork-Delegate: ferruh.yigit@amd.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1C275424A4; Fri, 27 Jan 2023 23:45:30 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CD89F40150; Fri, 27 Jan 2023 23:45:29 +0100 (CET) Received: from NAM11-CO1-obe.outbound.protection.outlook.com (mail-co1nam11hn2201.outbound.protection.outlook.com [52.100.173.201]) by mails.dpdk.org (Postfix) with ESMTP id CF50640146; Fri, 27 Jan 2023 23:45:27 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ALzGmp+iaMgR6o8YajAr9C6fWK08eG0ovgnzA9J+5mj2R31Ex7f+nJle1WEINiU/cg7JVaj6d2FzFcjtPfhnN2gY+cwkOf3jkAOjhF/zfi+GIOi/5ioWZcQXhrcf6lRK10NOKyi3/m4wyhYBBsB/QkFeBGEUyv91+hPnY5w00bwOww0WirBMFKzHg/XCVClLPfJFFha7tb0OgvvIgY5z5bObEnhr948bJHsANJ9lFoatOCoyyQeIzADCN6Ka1gC4kuKv0wFVSvMtLjTxMqmHvpuuGs0GEPBPzRZigM3nL9Wm0BBPpxP4NGKeDMvItIRCYTrQnUZ7cn5fqkGtON+iAg== 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=m4GmuW7mHs+pg6rbL4rzbvjiop5HtL3pFiPp1Pl+FzQ=; b=SWrJqhjcgKaQZ/8FtED4W6KaOj2UmgZBF1hPT1DU1ZpLkrnA8riCqD/F0/RoczqJhhFfPL1w0iz+BfNWOr1nUNiVnenfJQfD6TA+RpUQsjdCCvpHszF8w0wOxkog1i3jSTke6eAOsR+QP3GIW6lyfedSZbX+O7r3PNHxH1rvmnLNnyoACwTrj2MbkSbFEdAxY2KCtkzw2G8IBRavXFMPyAv73KqdhAeZ0qdNXbGtrDhqpntbJQsgHA1TlH64v8jButO28qPOVfT+edL/XPYH80D0VCOPZU3U2WtjtvcJU1ysdMuzNwdnt85mltF4m/H53gtD7hLN6gncnIrZ7KPz2A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=intel.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); 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=m4GmuW7mHs+pg6rbL4rzbvjiop5HtL3pFiPp1Pl+FzQ=; b=YsHPEHHM/zqfJpMtTJ98TeUp+Mr9nanrcVRdHzZuk1824Rz2VMnhEGWj6aurHPoSFeVtXyNOTk8iahWPLIJMkvuSIsou8yLV7lqTGHpl9UtSIH/FjKaUJkJUgmXh4ameAQ7+DT8GwWtMTQkshS9tCs4CidcNSb5tBNGnDFNsqo8= Received: from MW4PR03CA0129.namprd03.prod.outlook.com (2603:10b6:303:8c::14) by BL3PR12MB6449.namprd12.prod.outlook.com (2603:10b6:208:3b8::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6043.25; Fri, 27 Jan 2023 22:45:25 +0000 Received: from CO1NAM11FT103.eop-nam11.prod.protection.outlook.com (2603:10b6:303:8c:cafe::18) by MW4PR03CA0129.outlook.office365.com (2603:10b6:303:8c::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6043.25 via Frontend Transport; Fri, 27 Jan 2023 22:45:24 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by CO1NAM11FT103.mail.protection.outlook.com (10.13.174.252) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.6043.22 via Frontend Transport; Fri, 27 Jan 2023 22:45:24 +0000 Received: from telcodpdk.amd.com (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.34; Fri, 27 Jan 2023 16:45:23 -0600 From: Ferruh Yigit To: Aman Singh , Yuying Zhang , Michael Qiu , Pablo de Lara CC: , , Subject: [PATCH] app/testpmd: fix link check condition on port start Date: Fri, 27 Jan 2023 22:45:13 +0000 Message-ID: <20230127224514.2650827-1-ferruh.yigit@amd.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-Originating-IP: [10.180.168.240] X-ClientProxiedBy: SATLEXMB04.amd.com (10.181.40.145) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CO1NAM11FT103:EE_|BL3PR12MB6449:EE_ X-MS-Office365-Filtering-Correlation-Id: 789a7fac-1b64-4e10-b7b2-08db00b82dfd X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 0OX2RQmZojK0sLcK3BmtlMABYVGNY/ZBQkls7a4qoT0zwsdbIBVSs8LJBacQHeIyBnRemofM4y5+99yy7WHvQIou5bM2y12KP+bWdcATvMxWnjYUhpU8Yc5Rr2pLhGoijAn3kISmyNMrdJjYohBlGLcy+yyxtgh3ff7CckwZPvlcqJfVyNQXAQnvb+InE7M0qKkKdXY2e1GPSRABK86jQuRW8E1dW5+pvc7WtmmuOgYxXptD/cRX3o8W6Rk5dr98Z3W7mb9MJoA7gsdNeFF0jUvhLR44dAv4JSdlgu1JL1NPwBuh+KyR+WexsbUcMh7cvTJLEhU3lKMwEMLQuMfOjKAwqpqHKJYZeq6G1JFeDEDHZwriQD0TkNpy3bO1tos2K3DBN41fLpzN/xFgJURC+ohbbAmrURX+utt2GrDBTNvE03xK+xiBkcui+ugmt555RocLhbCneKWcthjbq5VRyaoCkmgktAQ32bdxJ9wniPYehpZVVBFZlzCYJfkdgJ0wdOaCCFtIZFiR6WU1OcK3nNz2sPBfV3+mPJO1sm6uaLcLcGGJh2Llps/p5U0yqGhqEwEQP0Bkhi7EXMzQBFDNkNw6SuL5uT44dPJ/eXbrwh04D/rdsBzaqpWhuYQajpF13aFLe8zlfBqWeOGVnqya6mw/DSFDFpbxp3yOzzPmPwDuMXAAjfC8Nsvh1hNPys/d70i5XQeLLeFu5s63UjA8BAFIjUq8kBdLAT5RyXYtmzA= X-Forefront-Antispam-Report: CIP:165.204.84.17; CTRY:US; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:SATLEXMB04.amd.com; PTR:InfoDomainNonexistent; CAT:NONE; SFS:(13230025)(4636009)(396003)(34036004)(376002)(136003)(39860400002)(346002)(451199018)(40470700004)(46966006)(36840700001)(26005)(2616005)(41320700001)(4326008)(8676002)(70206006)(336012)(70586007)(7696005)(426003)(16526019)(47076005)(186003)(82310400005)(36756003)(2906002)(86362001)(83380400001)(8936002)(36860700001)(508600001)(810100004)(40480700001)(316002)(54906003)(110136005)(40460700003)(6666004)(1076003)(81166007)(41300700001)(5660300002)(44832011)(356005)(36900700001); DIR:OUT; SFP:1501; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jan 2023 22:45:24.7079 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 789a7fac-1b64-4e10-b7b2-08db00b82dfd X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d; Ip=[165.204.84.17]; Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: CO1NAM11FT103.eop-nam11.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL3PR12MB6449 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 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 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 --- Cc: michael.qiu@intel.com --- app/test-pmd/testpmd.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 134d79a55547..2a1125c9fe81 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -2880,7 +2880,7 @@ update_bonding_port_dev_conf(portid_t bond_pid) int start_port(portid_t pid) { - int diag, need_check_link_status = -1; + int diag; portid_t pi; portid_t p_pi = RTE_MAX_ETHPORTS; portid_t pl[RTE_MAX_ETHPORTS]; @@ -2891,6 +2891,9 @@ start_port(portid_t pid) queueid_t qi; struct rte_port *port; struct rte_eth_hairpin_cap cap; + bool at_least_one_port_exist = false; + bool all_ports_already_started = true; + bool at_least_one_port_successfully_started = false; if (port_id_is_invalid(pid, ENABLED_WARN)) return 0; @@ -2906,11 +2909,13 @@ start_port(portid_t pid) continue; } - need_check_link_status = 0; + at_least_one_port_exist = true; + port = &ports[pi]; - if (port->port_status == RTE_PORT_STOPPED) + if (port->port_status == RTE_PORT_STOPPED) { port->port_status = RTE_PORT_HANDLING; - else { + all_ports_already_started = false; + } else { fprintf(stderr, "Port %d is now not stopped\n", pi); continue; } @@ -3130,15 +3135,14 @@ start_port(portid_t pid) printf("Port %d: " RTE_ETHER_ADDR_PRT_FMT "\n", pi, RTE_ETHER_ADDR_BYTES(&port->eth_addr)); - /* at least one port started, need checking link status */ - need_check_link_status = 1; + at_least_one_port_successfully_started = true; pl[cfg_pi++] = pi; } - if (need_check_link_status == 1 && !no_link_check) + if (at_least_one_port_successfully_started && !no_link_check) check_all_ports_link_status(RTE_PORT_ALL); - else if (need_check_link_status == 0) + else if (at_least_one_port_exist & all_ports_already_started) fprintf(stderr, "Please stop the ports first\n"); if (hairpin_mode & 0xf) {