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 B59EFA04FF for ; Fri, 29 Apr 2022 15:32:03 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A90F842824; Fri, 29 Apr 2022 15:32:02 +0200 (CEST) Received: from NAM11-CO1-obe.outbound.protection.outlook.com (mail-co1nam11on2046.outbound.protection.outlook.com [40.107.220.46]) by mails.dpdk.org (Postfix) with ESMTP id 36525410E3; Fri, 29 Apr 2022 15:32:00 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=aiS0nB3ko6299a79S1vQAXUgUqSqGUMnY5RiUF/GtLHIVo/jA+6893legYb706AjtWcj1a8bEPuyYJ3tjr6Fk5m5hKBm8Tu62g89xHPifOvOU/qMdv9hWGeXkfkDGpAnLN/0jnFZIjvIGu+Mba5hVD4GHe4RBrK7D50SxhU0jjiIGiafbzB/+vyfsO1NlqVZzaQfY1kuh1V9Pgh9n8VjfMaYpM7K8PRS6rSrg7ihjWriwDMPiLGRzjBViPfCreGV6QBfJdKt3EWVhhtaWUPaWJSqlm71U7QkcGnZNk09m542DCa79CBOUwxrCmAiNJJBJ6yPz6p11JdC+o4+b38C5Q== 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=NI54RuyXg/9ZCAj2zeJ/Km7vNQojSirHJNEA5Rakmtk=; b=A33i93I4YP9XYgKGS6UR4c61yMhYWhyWHD92b/QwO9E9BsT1rFlQ24ydij9aUX2V2PZgTwIAMdnys+zDAVoM+CtX07LHClCD10znx3Ft9MytJIoxGWYpFsldm6OYJKbp0BM3t3Q/b0tvFgM4w6SfBisX10KI8mo6lvskqeEicdQ5tq3ONJRjMAJ1sJO2N0FzQRcWE2u7gaPlSGi1l3nARhdFJtq1myxv/tOU8IFPD380rqCJnCHUnV1rAOtGrh8nThdvtvElQ3N6azEt2/0ZK2VY5MlZ5XP8iKoFBTnZlDeBuJMrUuPPxTLUdgXYB/JfX32Jaf2ZOwBLAsh6mwUbFA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 149.199.80.198) smtp.rcpttodomain=huawei.com smtp.mailfrom=xilinx.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=xilinx.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xilinx.onmicrosoft.com; s=selector2-xilinx-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=NI54RuyXg/9ZCAj2zeJ/Km7vNQojSirHJNEA5Rakmtk=; b=jy6EAa508PSBUZ9rieCcyrcNhjg8nfmCA6uRYUJjkaBI9rteNmSUq+Iw6MqEcRI2VYZT+mCuf8VUp4miDsZINQ2emyLq6b12fDHomAJDUlwc+4H6aiMjoiy+LrhZoa2Le+Nd12lIAeSRpkYdE75KC9BSmHn5BkJtkzQcUVrjhEE= Received: from SN7PR04CA0232.namprd04.prod.outlook.com (2603:10b6:806:127::27) by CO1PR02MB8555.namprd02.prod.outlook.com (2603:10b6:303:15a::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5206.13; Fri, 29 Apr 2022 13:31:58 +0000 Received: from SN1NAM02FT0012.eop-nam02.prod.protection.outlook.com (2603:10b6:806:127:cafe::4) by SN7PR04CA0232.outlook.office365.com (2603:10b6:806:127::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5186.15 via Frontend Transport; Fri, 29 Apr 2022 13:31:57 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 149.199.80.198) smtp.mailfrom=xilinx.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=xilinx.com; Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates 149.199.80.198 as permitted sender) receiver=protection.outlook.com; client-ip=149.199.80.198; helo=xir-pvapexch02.xlnx.xilinx.com; Received: from xir-pvapexch02.xlnx.xilinx.com (149.199.80.198) by SN1NAM02FT0012.mail.protection.outlook.com (10.97.4.93) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.5206.12 via Frontend Transport; Fri, 29 Apr 2022 13:31:57 +0000 Received: from xir-pvapexch01.xlnx.xilinx.com (172.21.17.15) by xir-pvapexch02.xlnx.xilinx.com (172.21.17.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.14; Fri, 29 Apr 2022 14:31:55 +0100 Received: from smtp.xilinx.com (172.21.105.197) by xir-pvapexch01.xlnx.xilinx.com (172.21.17.15) with Microsoft SMTP Server id 15.1.2176.14 via Frontend Transport; Fri, 29 Apr 2022 14:31:55 +0100 Envelope-to: humin29@huawei.com, dev@dpdk.org, lihuisong@huawei.com, stable@dpdk.org, chas3@att.com, radu.nicolau@intel.com Received: from [172.21.34.28] (port=4065) by smtp.xilinx.com with esmtp (Exim 4.90) (envelope-from ) id 1nkQix-0002kB-BR; Fri, 29 Apr 2022 14:31:55 +0100 Message-ID: Date: Fri, 29 Apr 2022 14:31:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Subject: Re: [PATCH V2 1/4] net/bonding: fix non-active slaves aren't stopped Content-Language: en-US To: "Min Hu (Connor)" , CC: Huisong Li , , Chas Williams , Radu Nicolau References: <20211025063922.34066-1-humin29@huawei.com> <20220324030036.4761-1-humin29@huawei.com> <20220324030036.4761-2-humin29@huawei.com> <212e85ef-228a-d31e-e5a1-8b2331c7eef9@xilinx.com> From: Ferruh Yigit In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: d06f8e34-c7cd-4196-7a41-08da29e4a200 X-MS-TrafficTypeDiagnostic: CO1PR02MB8555:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: VbAHpv9rs4eoASBoFHS4raLH48nxtrDM2YI7tqFNNgQx6Mc/Oi9aCCjpfCIi78W+x4G3eA/kmNZ8URKD8XcARJbmToWnsYYH02u0EbJl77qDahG0O8dzgsx4w1dV62EMdOI9c8txYTjwdmbr5r/lus51iQuGgMhG0vCk3277hMAZLHmtgkL6zLhIw+1SaFp0LR2XaQrlVkkw1yswnVmAkdN6wAJjCGE4ZcjwPXkcdr448tZyFAJPw5UqvAWhGYd4wHn0jaqLVTOMq+e51a4rii9qkOo5RVjIAQVwM0ahvHGjyOSEVnH5xrWV2LnxZMTWCoF/UC1UK9De6Albbm2/59d0U58Kvv6pKRKvESeO2PNIDMoxMwDg2jG28stfdCq0KZCWscXS1nSJYV17TxmXW9t0FUtpNe9erAG4HX4kCsi/CIc9RWKFPEK2VThsn+yoHhxUbHe64dcxXf4/4D1hGOMMQcYkuhj5WTieulUXfkl2gBlaO7L9f6Z/ZonFZqRTJiQJ1FVDingJ2j2sYe2l6wJnMg23ILTYOtlUDvtuLypo/XK7CBf3w5pNMWGfwovl4rdWKamdIneoczlMuFChaNlo2Cp2k2tfdRbnyoAqLGbU6a1hTvB1iL0uUptklPlc0an2iH485qA9Xq8RalwA1hjAB6dviaCPiFgANLjvzF/ZHDCUrjNvd3CJ3+8XAuCRF8n4/aJisrHqEhYyT6+nlS0lD4hyUmCNHZQ0k0pEmow= X-Forefront-Antispam-Report: CIP:149.199.80.198; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:xir-pvapexch02.xlnx.xilinx.com; PTR:unknown-80-198.xilinx.com; CAT:NONE; SFS:(13230001)(4636009)(40470700004)(36840700001)(46966006)(2616005)(82310400005)(7636003)(83380400001)(356005)(40460700003)(2906002)(26005)(70206006)(70586007)(8676002)(53546011)(4326008)(5660300002)(44832011)(47076005)(186003)(31696002)(316002)(9786002)(426003)(36756003)(8936002)(336012)(54906003)(110136005)(31686004)(508600001)(36860700001)(50156003)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Apr 2022 13:31:57.0912 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: d06f8e34-c7cd-4196-7a41-08da29e4a200 X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c; Ip=[149.199.80.198]; Helo=[xir-pvapexch02.xlnx.xilinx.com] X-MS-Exchange-CrossTenant-AuthSource: SN1NAM02FT0012.eop-nam02.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR02MB8555 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 4/29/2022 7:45 AM, Min Hu (Connor) wrote: > Hi, Ferruh, > > 在 2022/4/27 2:19, Ferruh Yigit 写道: >> On 3/24/2022 3:00 AM, Min Hu (Connor) wrote: >>> From: Huisong Li >>> >>> When stopping a bonded port, all slaves should be deactivated. But only >> >> s/deactivated/stopped/ ? > not agreed. deactivated and stopped are different state for slave. > Just to clarify the sentences, otherwise I see the 'stopped' and 'deactivated' states are different. Next sentences complains that not all ports are stopped: "But only active slaves are stopped.", so I thought intention in this sentences to claim that all slaves should be stopped (but it mentions all slaves should be 'deactivated'). As long as you address the disconnection between two sentences, I don't mind the wording. >> >>> active slaves are stopped. So fix it and do "deactivae_slave()" for >>> active >> >> s/deactivae_slave()/deactivate_slave()/ >> > agreed. > >>> slaves. >> >> Hi Connor, >> >> When a bonding port is closed, is it clear if all slave ports or >> active slave ports should be stopped? > Yes, I think all the slave ports should be stopped(or try to be stopped). >> >>> >>> Fixes: 0911d4ec0183 ("net/bonding: fix crash when stopping mode 4 port") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Huisong Li >>> Signed-off-by: Min Hu (Connor) >>> --- >>>   drivers/net/bonding/rte_eth_bond_pmd.c | 20 +++++++++++--------- >>>   1 file changed, 11 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c >>> b/drivers/net/bonding/rte_eth_bond_pmd.c >>> index b305b6a35b..469dc71170 100644 >>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c >>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c >>> @@ -2118,18 +2118,20 @@ bond_ethdev_stop(struct rte_eth_dev *eth_dev) >>>       internals->link_status_polling_enabled = 0; >>>       for (i = 0; i < internals->slave_count; i++) { >>>           uint16_t slave_id = internals->slaves[i].port_id; >>> + >>> +        internals->slaves[i].last_link_status = 0; >>> +        ret = rte_eth_dev_stop(slave_id); >>> +        if (ret != 0) { >>> +            RTE_BOND_LOG(ERR, "Failed to stop device on port %u", >>> +                     slave_id); >>> +            return ret; >> >> Should it return here or try to stop all ports? >> What about to record the return status, but keep continue to stop all >> ports. And return error if any of the stop failed? > I think no need to do this. APP only see the bonded device. If bonded > device stop failed, it means it works failed. And the number of > "stopped" successfully slave does not make any sense. > OK if trying to stop as much as possible 'slave' devices doesn't make sense, we can keep as it is. Btw, when functions fails at this point, bonding device itself already marked as stopped, right? And some of the slave devices may be stopped already before failure. I don't know how confusing this is for the user, that stop() function is failed but bonding device state is 'stopped'. I don't know if function should recover at least bonding device status (back to started) on failure, what do you think? >> >>> +        } >>> + >>> +        /* active slaves need to deactivate. */ >> >> " active slaves need to be deactivated. " ? > agreed. >> >>>           if (find_slave_by_id(internals->active_slaves, >>>                   internals->active_slave_count, slave_id) != >>> -                        internals->active_slave_count) { >>> -            internals->slaves[i].last_link_status = 0; >>> -            ret = rte_eth_dev_stop(slave_id); >>> -            if (ret != 0) { >>> -                RTE_BOND_LOG(ERR, "Failed to stop device on port %u", >>> -                         slave_id); >>> -                return ret; >>> -            } >>> +                internals->active_slave_count) >> >> I think original indentation for this line is better. >> > agreed. >>>               deactivate_slave(eth_dev, slave_id); >>> -        } >>>       } >>>       return 0; >> >> .