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 75C0F42D2F; Fri, 23 Jun 2023 15:13:13 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0097C40ED7; Fri, 23 Jun 2023 15:13:13 +0200 (CEST) Received: from NAM04-BN8-obe.outbound.protection.outlook.com (mail-bn8nam04on2076.outbound.protection.outlook.com [40.107.100.76]) by mails.dpdk.org (Postfix) with ESMTP id 243BC406B8; Fri, 23 Jun 2023 15:13:11 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ujg9mVo7TvzFirndLa+P4NVwBqL5SU3ZIfRjDv7KWNnk0UJHHyuFPwIMLNjmD2E6oUzsEDr0tD8YXe9D41+3779iRWNrYbuKT5g2ogOEX+LKh1GnX0/Qyb2oHUkbmi3f/I/zPjMJreCvLnZVZLPNM6g1p90x2PffTYEApixyeYmpSAJ2p1HYgrKNJpqVJ9gWzqOgcgwdZNT8oFq7qVIW6fnKo3J2mbQ0B+t6eUmJkMzpUlBcNNHml7gCDhSo7l5JhWIDNQ9ccqQhv34GuiEJ01YaJGD/Fa0bFZnqe1O1zpQU9JxiQ4GfkDlD+8kt8eT21tm/t1KzVg6gIXpmG6swVA== 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=diY2Z2ny8wPm7qxGolhZuwdpd0vyg/HtYrrfLIXO10Q=; b=f+y3ItgGTbhuKyFYqXe/Nbqg1/+vgfuulLooI/NPpDj+nP0B7xpie7dTLGUcTWZD/d2bMZ7A11VnCmoSOP95UqNyK2QY4Rn98WKlrDadU8D5HcA59utG5t2R3A3MLfnUsvh997Q75ko2OEcpZpsOt9t0I/xVsmvGM7Z2S4uYynZNHwJdyo30U1FlxXk80TreTS1+dfJiNRMFiDsYdl1jmX+oH/lflm5UjSkVFPeFIM+1ldHCNxhYif6Odg+vEVYDeulazU2en9AXr3gKhS/Qc5FFpQq04wgRSQo/1Po7VUbP4wAtfsCQqwszCheCFZhIhHRswXBY//t2jKnMhA1vLg== 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=diY2Z2ny8wPm7qxGolhZuwdpd0vyg/HtYrrfLIXO10Q=; b=W3QTwx1u93L69i5mqH+B5YOxsi3e71CGZrwcytdLmfdNtfy3Z2/SADrpQ8B8tnBT467HUbjLt0SIbdpo5Xg9ykRAxBitA9SVpzqEVJTW9gY00NnfQIWGxOfMKfjr4ni5F3+B8RyEghh6x2R+wFRUYOym8BWRy/eZq/7sJILW+gQ= 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 SN7PR12MB7249.namprd12.prod.outlook.com (2603:10b6:806:2a9::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6521.23; Fri, 23 Jun 2023 13:13:08 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::16e3:326c:5c2a:be42]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::16e3:326c:5c2a:be42%3]) with mapi id 15.20.6521.024; Fri, 23 Jun 2023 13:13:08 +0000 Message-ID: <11bacd8b-f4eb-4af6-3770-05f69c43dfcb@amd.com> Date: Fri, 23 Jun 2023 14:13:01 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Content-Language: en-US To: "lihuisong (C)" , Stephen Hemminger , Kaiwen Deng , Qi Z Zhang Cc: dev@dpdk.org, stable@dpdk.org, qiming.yang@intel.com, yidingx.zhou@intel.com, Chas Williams , "Min Hu (Connor)" , Declan Doherty , Daniel Mrzyglod References: <20230608072636.426803-1-kaiwenx.deng@intel.com> <20230608084113.06e6594f@hermes.local> <595cbf81-d78a-a7f5-3c1b-7c1c70a05fd8@huawei.com> From: Ferruh Yigit Subject: Re: [PATCH] net/bonding: fix iavf bond device query stats In-Reply-To: <595cbf81-d78a-a7f5-3c1b-7c1c70a05fd8@huawei.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO4P265CA0124.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:2c6::14) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|SN7PR12MB7249:EE_ X-MS-Office365-Filtering-Correlation-Id: d3e10b8b-ca88-424a-99dc-08db73eb964e X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: GARKsSnZ3q1xj1BcYRZgz51dJxAUfYoVBasp9fe+lxCsC2qJcljYQ+4eAYhes33hw+achHslIYiNz1khAEckY9mhDFnunz/2FOg/gIpLlyHUGHu05/UrdtvWAizExaoj71czE4eu7EpJIM4gmU7Ob+/odSRTroCGm9BTvo8iR6OM1VVpB6UGKfKNn/72RlfSzatW2S3tDSSGEmfo1tJU07I3IMfnuE0Jutwho6e8bYYeKcAnZFpKXIHRZC+fFS3hd4I/K9p1oYwhpXJE6rqO9af7buS2h7srjWM4kkwliS0InVHqlueqdW8u0/CKgaT7d/IBT9CnX/pG02sPq9YEkruNq6BOAM1uIzPMDxhOTDc8cOwtNkHB7o79gjVrnQfmU7xxDmw+m6Q0AAX7H7KtniOEMy4JYyhNZ1g4toTuAS+E45RLfBLnEcFWqTNGMoPJ+k0vq/9wyjXhi4oOur4jXzZeD8qvFRCyNhdiz0iLrk4OM65Xf6VI8/pmk3libOs+YI09j/fFjKSaBhfSawYL3WJh+hiCXyTc2Ky4BVQEuUqqgj0DfV1yNbzB84jdu5P4cPR17feRd2pHOEb8gGI7991D04ub84SxVaewWWA4wif3iLZHFB4Tq+l+w0t8nKMpy5lvJ5bOTWCuLkflqqoiWQ== 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:(13230028)(4636009)(39860400002)(376002)(396003)(136003)(366004)(346002)(451199021)(5660300002)(31686004)(66946007)(4326008)(66476007)(66556008)(316002)(36756003)(44832011)(7416002)(8936002)(8676002)(41300700001)(2906002)(6486002)(110136005)(54906003)(86362001)(31696002)(478600001)(38100700002)(53546011)(6512007)(26005)(186003)(6506007)(6666004)(83380400001)(2616005)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Z3czVjZFOFRzbk1MNVZqdjF3MEgyMFQ3dDVqUytkOWx2NnhHQ2hXYzFxdjBI?= =?utf-8?B?K0VhelZXbXlQaWMrM0hKK2dTKzA5OFRuWWN4M2tGUjNFMzBrUUVCRW1Ha21I?= =?utf-8?B?MkNsNm8wYXhJeHg5VU4xWS9veW94RmY1QWIwbEFoU055WlZ5SWtGOUtRL1Ev?= =?utf-8?B?WmVMTUNjdGtIV2VsaytjblMrVllFbUs3UHdqc2Y0eVNWZEdkeXUvVGRnTGIv?= =?utf-8?B?U0RmT201YmhDc3NQbmQvTmVBQnZuZnJLU2wxZE8zZEFKcHBDZVFSUko1TkRQ?= =?utf-8?B?RDJwSm9za1dtNkttQ0p0TEtSZmF3QVdBYzJFUFZGeEdwakxLcTM2NFFhbnY2?= =?utf-8?B?NlBTaDJvVmxkODUzcHlzamh6SHdBSjdrTGlMTmoxNjdsNkQrMWttWHpyRk1z?= =?utf-8?B?aE52dmxVSzI0ckVEM0tKWjZ0K2U4Sk05V0k1THhXajNkZThxMzh4bVlxMVdL?= =?utf-8?B?clNkdDY1YjZyTUVIc1l6d2Vxam9zMWRxUlE5SU85aHA1cEh6bWxRTngzU2tq?= =?utf-8?B?QUhvRnlqYVd3bnRvNTVQZjd5ckdRRDhwYm9CeXJhUHRUb3lNckRQdVZOMEhQ?= =?utf-8?B?TWZTVEhWaDgxUkV4aGYveVBnL1ByYzhxbnhxM3k4UloxNDBWNmE2QS80SGcw?= =?utf-8?B?RWR2VmkrcW5RSGJOMytkSFZyZjMzQWZ1eXRjV2xGWkQrYWlPSitNWGJPZ2lG?= =?utf-8?B?ZTBuREwxMXdYYUdDNXFnREVKM3ZPZmlDSXpSWThnbzlPclBIVlUrTVhCSVkx?= =?utf-8?B?VmZod1RubVk5ckd6THVXNFRKL01NMEJ2aGEwa2E3eFdUVUU3c05SYm9UUEg1?= =?utf-8?B?V2RmNmZUQkFMU1VuNjFLSXpDUW12RzRHYkVsMmNUaWgzazErN0p1N0tCRXJK?= =?utf-8?B?bXRiSzhaZkkzbXRhY3ZsMXZNenFGcjZpRlJTcUxNRjFSMS9rTjZLaGl5ZDdm?= =?utf-8?B?MkR0STJIVWVieXA1VmdXZVhPRjlJbUpQSy9Oc1Q5SVhzR3NVNVBtUWhtNmZ5?= =?utf-8?B?M0w4TFJub3VaT2s2dSs2SkoydldWQVZpSWh5QlMzTlhiTWtXckd1SjVmWnVt?= =?utf-8?B?MHlQeC9UUFpDeVlVeHJZcWxUVXdZNURGclFHdkxhejQ2Yk01RmJGdHFlSG1J?= =?utf-8?B?OWdXQnZBeHZmR2pxbStuV1A1WFRDZm9nQjE0UTM5R2dJTnFodVNGbDZjYUVi?= =?utf-8?B?ZVJ3eS9CeGRwbE0wQmJFUE9udnEyV01TT1k4MSsraGZ2VlN0VVBoaENBclYz?= =?utf-8?B?THFyVU5nT0ZLM3BYOFJHcExYOGk2cTRuVEttUWNLMk45VDhMbHpyTGc5bm00?= =?utf-8?B?Um92YjJTNlJQL0loY1ZIY3EwRVZZN1ZDSTVialJKSjkyTFpPcENTLzc0NFEx?= =?utf-8?B?RXlKY0VLV3VmYzNmaGo5LzFSOFl0MTBSY2N6TWRGZXgwTHVJMFcxcitGMlJX?= =?utf-8?B?MkdPNXR4S3hKNEp2NGNJVURCQndkZWJFd2xPclFBVW1HN3d4dVJ2OE5GU0l0?= =?utf-8?B?REVaMUdaQkdPRkFjcUgrYWdVcVBUOEMrMlhvWjEyMU5OaFc3VmxMOXhYMHFQ?= =?utf-8?B?ZnJsSW5NcXBrdWdRbUh2Q3g5VTV3TXRPSlh5ekVIcmdFaU9TeGVOTTdwRHpm?= =?utf-8?B?aXRyTWhyaUlFcmJSbitucU9OQXZ0S0gyZDJUdTh5UGpXV1JuU2lreWNPakUw?= =?utf-8?B?VHlJeGxhZW82ajVTM0xabExQWSsybEh2VjVSRU94L3BHWEVXbzZqZUNwd0Z4?= =?utf-8?B?SlNJbCtkdTRaQ1RCNmg0WjRKUi8rOEhoOTE4cTZzNFhKcGR5SWR4T2VZR0hM?= =?utf-8?B?QVF5cXRSeVhHTEVEU28rVDhUMGQ5TVNucko2Nk5OeG5QM0dsQWZ1L1YxNkNt?= =?utf-8?B?cVRJRUt5YVNXM2kzbzR1Tkh2dndneVJLZFJpRkNaMHhVME15S3JBYmpKeEhM?= =?utf-8?B?SHdPL3pTOW11VHRxbHFrcjd3WnpWSU5ZY0FkbmR0cUZpbzRDYkxEVXFnQTdO?= =?utf-8?B?N2hPZW1ZRER4Z29GSjkybGk3clpJZXY4bDBMaUQ4eVdwM3dlRWdWd0kwU2tm?= =?utf-8?B?R1E0ZWlBbEQ4ZHFZZytobXQxYlVISjZ1WkVtT1RGU3luUXI0N0FPa04xV0Iy?= =?utf-8?Q?2uyRGfca/UR8qhhOJbRwyd2DM?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: d3e10b8b-ca88-424a-99dc-08db73eb964e X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jun 2023 13:13:08.0983 (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: cJlsUid+03IG6BjUYC8yqmBtKT7Tzq+to/shuavvzxZ/Y8vso3jUYLQ52D+0dBco X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB7249 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 6/9/2023 2:38 AM, lihuisong (C) wrote: > > 在 2023/6/8 23:41, Stephen Hemminger 写道: >> On Thu,  8 Jun 2023 15:26:36 +0800 >> Kaiwen Deng wrote: >> >>> If the rte_eth_stats_get function does not work properly, >>> the update function of the slave device does not work >>> properly When device is bonded as BONDING_MODE_TLB mode. >>> >>> This commit adds handling for functions that do not get >>> stats properly. >>> >>> Fixes: 7c76a747e68c ("bond: add mode 5") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Kaiwen Deng >>> --- >>>   drivers/net/bonding/rte_eth_bond_pmd.c | 7 ++++++- >>>   1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c >>> b/drivers/net/bonding/rte_eth_bond_pmd.c >>> index f0c4f7d26b..edce621496 100644 >>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c >>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c >>> @@ -894,6 +894,7 @@ bond_ethdev_update_tlb_slave_cb(void *arg) >>>       uint8_t update_stats = 0; >>>       uint16_t slave_id; >>>       uint16_t i; >>> +    int ret; >>>         internals->slave_update_idx++; >>>   @@ -903,7 +904,10 @@ bond_ethdev_update_tlb_slave_cb(void *arg) >>>         for (i = 0; i < internals->active_slave_count; i++) { >>>           slave_id = internals->active_slaves[i]; >>> -        rte_eth_stats_get(slave_id, &slave_stats); >>> +        ret = rte_eth_stats_get(slave_id, &slave_stats); >>> +        if (ret) >>> +            goto OUT; >>> + >>>           tx_bytes = slave_stats.obytes - tlb_last_obytets[slave_id]; >>>           bandwidth_left(slave_id, tx_bytes, >>>                   internals->slave_update_idx, &bwg_array[i]); >>> @@ -922,6 +926,7 @@ bond_ethdev_update_tlb_slave_cb(void *arg) >>>       for (i = 0; i < slave_count; i++) >>>           internals->tlb_slaves_order[i] = bwg_array[i].slave; >>>   +OUT: >>>       rte_eal_alarm_set(REORDER_PERIOD_MS * 1000, >>> bond_ethdev_update_tlb_slave_cb, >>>               (struct bond_dev_private *)internals); >>>   } >> Why is stats get failing on a device, looks like the real bug is there? >> Better to fix the buggy driver. Other usages might already be affected. > I think this is the case if the driver happens to have an abnormal event > and do reset. > So here need to handle this fairlure. > Agree with Huisong, this is a DPDK API that can return error, so checking return value is reasonable. @Kaiwen, back to Stephen's point, the patch title mentions from iavf, is there anything special with iavf driver that fails to get stats? Main concern is, if this patch to hide a defect in iavf, or is driver only mentioned because problem observed with iavf driver? If this is not iavf specific issue, we can continue with patch, in that case can you please make a new version with following changes: 1- Add error check for both usage of 'rte_eth_stats_get()' in bonding 2- Make label lowercase 3- Print a log message on failure Thanks, ferruh > Additionally, suggest that this API in bond_ethdev_stats_get should do > the same things. >> Silently ignoring the error without logging is also not good. >> Lastly, DPDK coding style is to use lower case for goto labels. > +1 >> .