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 44568A0C4D; Tue, 24 Aug 2021 16:42:18 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B70D8406A3; Tue, 24 Aug 2021 16:42:17 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 0B1FD40687 for ; Tue, 24 Aug 2021 16:42:15 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10086"; a="278334397" X-IronPort-AV: E=Sophos;i="5.84,347,1620716400"; d="scan'208";a="278334397" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Aug 2021 07:42:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.84,347,1620716400"; d="scan'208";a="455512072" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmsmga007.fm.intel.com with ESMTP; 24 Aug 2021 07:42:13 -0700 Received: from orsmsx604.amr.corp.intel.com (10.22.229.17) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10; Tue, 24 Aug 2021 07:42:13 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx604.amr.corp.intel.com (10.22.229.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10 via Frontend Transport; Tue, 24 Aug 2021 07:42:13 -0700 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.46) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.10; Tue, 24 Aug 2021 07:42:12 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Dbb1jgdRcy4qjC/VhvHfMvP2qprHv/FkrQ2JPAFm7phWKT15/hF+bc7t9s/xFa3Ok8620kS4I9f95aPUlEOdlqRBfpbXIoN0UzjvhlLQKwy4cn2wbd9Q8t5pYsGwD8xC4DA+dS8JakS40azu9S1W5fGn8aZwe5RHwhy6xxQ7HastmZP0x3WbAPcCFj063vv9MbbMT22nOLXU543b+V610DqKtMabEQP8uirul9EFliN7BRJYe/CgV970L2j+XjNKgHw9bN61WjxsvNBld/ErnWD5lWguDUVx7T4ajGHrCUUO9IZKc7PqmAOcwu9gzUL1h2rtZjfNzi1i/Wq9zKt3yw== 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-SenderADCheck; bh=qaCJ0yrRQe4PSBE8K/NhDgr18Qg3oUt1motjMDS2c/Q=; b=Sayk1QwNU9QySGD+1gqROXEVxBE1Jkc2ivC5Esb3rlSHXKI5zuI1WnRTJ+cNqI++UG2Pm/Q8hzvjM4z9RMXDmD8oVzILX9WHpZW+EXyES/Owd/kn5mH3SAsPeWknu89pJtXwbPkNToojoLnGu/osQoqFoL3PWcgwMN3wL9AFcOtLJMMcOl8ssJ4AY/vbMoxG5TwDyLSAYFoVtQVFnKE9Lh39AOWMp1XeKGXuz8KMhyjKVIlfkFeRFkG0vazoOWjjdVQm4KN8HB8RMNsfJDLSsFTQu6xD2LBIXc8ZS+SLaioiaVFi2JxG9CAy5NZTBkp59Clyp34GSRrp23ks833CVQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=qaCJ0yrRQe4PSBE8K/NhDgr18Qg3oUt1motjMDS2c/Q=; b=D859FbLn8rJO1KwM4+2Js0juRXAeVx1STLlo2dnyAjR7i/littprf74LualzbomFKMsi+3RJbfSWd9Wyz3bv/poU4WUCpdcbbH8WIU67xav5WhPQOWwNxZaujKQTEnCOw0KWXKeRCHy2YdL/4MgGGGOL9AXTYitgznHblddJwTY= Authentication-Results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=intel.com; Received: from PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) by PH0PR11MB4774.namprd11.prod.outlook.com (2603:10b6:510:40::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4436.19; Tue, 24 Aug 2021 14:42:12 +0000 Received: from PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::2979:70ca:38a:dbaf]) by PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::2979:70ca:38a:dbaf%6]) with mapi id 15.20.4436.024; Tue, 24 Aug 2021 14:42:12 +0000 To: Huisong Li , Thomas Monjalon CC: Andrew Rybchenko , "dev@dpdk.org" , Anatoly Burakov , David Marchand References: <1627908397-51565-1-git-send-email-lihuisong@huawei.com> <1627957839-38279-1-git-send-email-lihuisong@huawei.com> <9670389d-ebbc-9d9c-0cac-c7e8826ecb6f@huawei.com> <21383486.34YfpWhNxb@thomas> From: Ferruh Yigit X-User: ferruhy Message-ID: Date: Tue, 24 Aug 2021 15:42:06 +0100 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-ClientProxiedBy: DB8PR09CA0030.eurprd09.prod.outlook.com (2603:10a6:10:a0::43) To PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [192.168.0.206] (37.228.236.146) by DB8PR09CA0030.eurprd09.prod.outlook.com (2603:10a6:10:a0::43) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4436.19 via Frontend Transport; Tue, 24 Aug 2021 14:42:10 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 32f4abff-d145-40dd-a914-08d9670d5bc3 X-MS-TrafficTypeDiagnostic: PH0PR11MB4774: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Vbr1+0q3tWRncUA2yFcampdqM3TvefVLV44uq4h4+T5xiRnkW2OnXFIK5UDl6+f7oT1ig6jfZioTHOQ59F425NhyvMDBt/D1CZ2/HigozL5PptLt1X5q6Cea8y5dGzOOXYgkix5WesXP9yGsTkUcwIDiCRBvAIxaJ9lJsYFg20gBsDXQHOa9WCOFKgJKrDaJeUGxCRjxhOP7x9Uj460RHt49HczJ3eLZ6Xmp2rB1fHo4S1TxY1nbueKoX7hDrwqYuuGbEZ8hJsN+IEhDxpFaoPJMC9QSN4ilrFb8OlvHar/iqlROFogZ6Cfsp1e3FTuGK1qJisgxpBlvr/tiWL/bJnAsQt8xXc3U8dFPYK3GYahgaAQWyMB6E4CmcH/ZdIHgPvOsofOdnYwF7NgZ2tYjgilL1fDm8koz1mlgJihiiQfStTj/+wKfeNK4gqex4G6h1A1T1xPhx3Om8uVH+OxUJpZ9HrpiugX9svYIROAgtIYxpGtRPGVYmFCsby8/tuA6tRkNodEUxBbkg91cWIasVXfNUkAB/6IC9zXIggVm1hgxiulj9sgkoP9IkG24Pl/4rTAsHJQR34jY0WD/gbQqjCDpq0XVqtdFn9p8DS9H4sm46KDrFVgokJpWWnmIOYeDR+TlZQxy8PqAccZTj13kKhOHDBvcnfGmBKrk3uQuf7IFXPcVNmHZ2ZAKo4mWqMUAOvIat5+B5Zo0FVV0KIBWrFG7WBgi3h8uxIWbxuo2moA= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PH0PR11MB5000.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(376002)(136003)(346002)(366004)(396003)(39860400002)(316002)(38100700002)(31696002)(16576012)(2616005)(86362001)(6666004)(8936002)(6486002)(956004)(54906003)(186003)(4326008)(8676002)(5660300002)(83380400001)(36756003)(44832011)(66556008)(66476007)(2906002)(66946007)(53546011)(478600001)(26005)(31686004)(110136005)(21314003)(45980500001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RGpHcmlQUlp4UXFTWEMzQXFKTGxJQ2labWFHQ0tzbjhlQW51Q2lCaG9qSGdm?= =?utf-8?B?Yk4vaEowZEgrQ2pzZFJPVGcrTkVWd1VjMkUzb1dyaXRldTl4dXFpd2JKdVph?= =?utf-8?B?c3k4OW85Q1hsdkV4VHFQU0ZXb2R5SjVFczd0elV4VjVNdUNIdDBwc0syTGlu?= =?utf-8?B?WWVCMXJmaFFJYStKRHpFQVlURGlIRVVxVnNML0l5eUlXcFNldUxPREJUR0dy?= =?utf-8?B?dE12N1NlSWVtY2Q4TEdsTGE2eTZjS3B2alJRek8xQ2wxTkNBUEFBWjNKZnZ6?= =?utf-8?B?SGh4RXdBWjYvSm9keG00SWo0UHZWUjUrTWRKZS96S1p1N2M4aTk5MDdTaUMz?= =?utf-8?B?bXduYXZHTVoxNzAwbFFnNmNLTHcwUDJrbVdoOWZmeDU5eWEwa0U2Q3NOWndL?= =?utf-8?B?b3BtVGRWelJXMWVWYkxBY2haQWtyTmQ3QjFUS2FFUWt2c05mR1VjZ2ZIM3dY?= =?utf-8?B?allJME1KQUhUR0lGclg2L2ErNEp5R1dHZTZCdG4yUUJSVmpmSUY1T0RybUtB?= =?utf-8?B?OGd3c3JtTW80MWJRZENBM00xNlQ0aDY1NjBYLzIrYkk0VGZZS3hIcjRVQyta?= =?utf-8?B?ZFg5S2xxTy9ub3pvcWRUSXEvM01kNXN4ak56YlZCY1VpaWxKUjlsUDhwRmI3?= =?utf-8?B?S2ROaDQyQXEwOTkvNkJFejhDRnBoOHlXai9mZG1jRTJHdVNCK2xlUURNM1BU?= =?utf-8?B?bDJwTkpJZVE0c24yOUplR05tK0ZRbzlCcGlyM1JDR21na0FUWFJNN1JHdjZh?= =?utf-8?B?aW8rUHdqcWtsUWFZSFMrTWEvK2lQWHF1clBRWG1WNkR4N1lxQk5TWlJkeTJz?= =?utf-8?B?bGwrUEk3VjFtOXRJWmp2aDNSRFVpTWtOQkxiRVMvL0NFQ0UxcEdSUXVpV1pN?= =?utf-8?B?Z2dnZE9ZNUp4V1VqUUJtV0xWYVRSdDZYUS9ra052YTFhc2pMd0MzRFMvcnlP?= =?utf-8?B?emdnTEdPRWZNNEU1VXlCbTVDeHJXZUw0Q0c3TlJoTDV1RzRFeXhvZzQwQnBo?= =?utf-8?B?dy9oZy95cnFZZVNqNFc5dk5Lb0hNVHRhZm9ObU0zMlk0ZThyT3ZTdTIwbHRH?= =?utf-8?B?ZVRKTU9qOGxjVTBab01JWlZZVTZIU1pVRGpMMzNYclN4ZHBUeHlrbzN3NFE4?= =?utf-8?B?Mkg4QUo0elBoa2tSaU56cVZsUzZqRisyZEg5em9UMUthcWk1VktXZ3Z2dHFu?= =?utf-8?B?RHJpc2U3SlhkZTJVYVRHWDBLUkpuT0o3NHhNUnMrczhEM2NPZGVBbWVYM0RW?= =?utf-8?B?WnpnSXV2WE9GVWkrTWx5U3ZUczcyUTBkWjhtQ3A3NktZTWlGRVhiTTdNL241?= =?utf-8?B?L3VCM1VtcW94SjY5M2c2ZHRzb1QrTlR1ZnhZdGQyOUVWTi9LRGQ2WEJXRTVs?= =?utf-8?B?ME0ya3RBbGcwMUVIN283ZGNmcGE1b2hCTmQ5ek5YYzFiWGlKTnI3WkVBQVpS?= =?utf-8?B?RXlGVkd2K25UdUFZODJNcnBFckdFaXczdENoMDlET1E0Zm5YNGEwckQwc013?= =?utf-8?B?QkJFZ0tYR3drVUZOVFFOYTl6T1hJTEc3YUpqN1p0KzNtNGRkbHBjUkV1eW1K?= =?utf-8?B?NVRWUWp5OWtEQTNGcG40bTR1SlY3a3JpYWhOZEtnUWlpdjBKVXdEMGxWemJx?= =?utf-8?B?YnV1dzFzOTZkYktFOXdsUHJET29WckhMSSs3S1JBNVZ1aGZGOGtUUUJvQVBQ?= =?utf-8?B?eUNVcGFjanYxcGt0Vm02ejVvODJ5eWFBT2s0VWZ5eXRRU1NpeVUzaDZad3dQ?= =?utf-8?Q?IFUbMP0Okkj1VpDUJlqBsDuPi0Xh+34ZWZON1wZ?= X-MS-Exchange-CrossTenant-Network-Message-Id: 32f4abff-d145-40dd-a914-08d9670d5bc3 X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5000.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Aug 2021 14:42:12.3193 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: hkYGuDEGr7oUvPb3Nfr8r9LtYecnwNS/iw+7SQAFIapUZ+/YcD0CLX31tPP+aSa9j8/fnVd9JSDEfSvWsbySfg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4774 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [RFC V2] ethdev: fix issue that dev close in PMD calls twice 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 Sender: "dev" On 8/19/2021 4:45 AM, Huisong Li wrote: > 在 2021/8/18 19:24, Ferruh Yigit 写道: >> On 8/13/2021 9:16 AM, Huisong Li wrote: >>> 在 2021/8/13 14:12, Thomas Monjalon 写道: >>>> 13/08/2021 04:11, Huisong Li: >>>>> Hi, all >>>>> >>>>> This patch can enhance the security of device uninstallation to >>>>> eliminate dependency on user usage methods. >>>>> >>>>> Can you check this patch? >>>>> >>>>> >>>>> 在 2021/8/3 10:30, Huisong Li 写道: >>>>>> Ethernet devices in DPDK can be released by rte_eth_dev_close() and >>>>>> rte_dev_remove(). These APIs both call xxx_dev_close() in PMD layer >>>>>> to uninstall hardware. However, the two APIs do not have explicit >>>>>> invocation restrictions. In other words, at the ethdev layer, it is >>>>>> possible to call rte_eth_dev_close() before calling rte_dev_remove() >>>>>> or rte_eal_hotplug_remove(). In such a bad scenario, >>>> It is not a bad scenario. >>>> If there is no more port for the device after calling close, >>>> the device should be removed automatically. >>>> Keep in mind "close" is for one port, "remove" is for the entire device >>>> which can have more than one port. >>> I know. >>> >>> dev_close() is for removing an eth device. And rte_dev_remove() can be used >>> >>> for removing the rte device and all its eth devices belonging to the rte device. >>> >>> In rte_dev_remove(), "remove" is executed in primary or one of secondary, >>> >>> all eth devices having same pci address will be closed and removed. >>> >>>>>> the primary >>>>>> process may be fine, but it may cause that xxx_dev_close() in the PMD >>>>>> layer will be called twice in the secondary process. So this patch >>>>>> fixes it. >>>> If a port is closed in primary, it should be the same in secondary. >>>> >>>> >>>>>> +    /* >>>>>> +     * The eth_dev->data->name doesn't be cleared by the secondary process, >>>>>> +     * so above "eth_dev" isn't NULL after rte_eth_dev_close() called. >>>> This assumption is not clear. All should be closed together. >>> However, dev_close() does not have the feature similar to rte_dev_remove(). >>> >>> Namely, it is not guaranteed that all eth devices are closed together in ethdev >>> layer. It depends on app or user. >>> >>> If the app does not close together, the operation of repeatedly uninstalling an >>> eth device in the secondary process >>> >>> will be triggered when dev_close() is first called by one secondary process, and >>> then rte_dev_remove() is called. >>> >>> So I think it should be avoided. >> First of all, I am not sure about calling 'rte_eth_dev_close()' or >> 'rte_dev_remove()' from the secondary process. >> There are explicit checks in various locations to prevent clearing resources >> completely from secondary process. > > There's no denying that. > > Generally, hardware resources of eth device and shared data of the primary and > secondary process > > are cleared by primary, which are controled by ethdev layer or PMD layer. > > But there may be some private data or resources of each process (primary or > secondary ), such as mp action > > registered by rte_mp_action_register() or others.  For these resources, the > secondary process still needs to clear. > > Namely, both primary and secondary processes need to prevent repeated offloading > of resources. > >> >> Calling 'rte_eth_dev_close()' or 'rte_dev_remove()' by secondary is technically >> can be done but application needs to be extra cautious and should take extra >> measures and synchronization to make it work. >> Regular use-case is secondary processes do the packet processing and all control >> commands run by primary. > > You are right. We have a consensus that 'rte_eth_dev_close()' or 'rte_dev_remove()' > > can be called by primary and secondary processes. > > But DPDK framework cannot assume user behavior.😁 > > We need to make it more secure and reliable for both primary and secondary > processes. > >> >> In primary, if you call 'rte_eth_dev_close()' it will clear all ethdev resources >> and further 'rte_dev_remove()' call will detect missing ethdev resources and >> won't try to clear them again. >> >> In secondary, if you call 'rte_eth_dev_close()', it WON'T clear all resources >> and further 'rte_dev_remove()' call (either from primary or secondary) will try >> to clean ethdev resources again. You are trying to prevent this retry in remove >> happening for secondary process. > > Right. However, if secondary process in PMD layer has its own private resources > to be > > cleared, it still need to do it by calling 'rte_eth_dev_close()' or > 'rte_dev_remove()'. > >> In secondary it won't free ethdev resources anyway if you let it continue, but I >> guess here you are trying to prevent the PMD dev_close() called again. Why? Is >> it just for optimization or does it cause unexpected behavior in the PMD? >> >> >> Overall, to free resources you need to do the 'rte_eth_dev_close()' or >> 'rte_dev_remove()' in the primary anyway. So instead of this workaround, I would >> suggest making PMD dev_close() safe to be called multiple times (if this is the >> problem.) > > In conclusion,  primary and secondary processes in PMD layer may have their own > > private data and resources, which need to be processed and released. > > Currently,  these for PMD are either handled and cleaned up in dev_close() or > remove(). > > However, code streams in rte_dev_remove() cannot ensure that the uninstallation > > from secondary process will not be repeated if rte_eth_dev_close() is first > called by > > secondary(primary is ok, plese review this patch). > > I think, this is the same for each PMD and is better suited to doing it in > ethdev layer. > This patch prevents to call dev_close() twice in the secondary process, is this fixing a theoretical problem or an actual problem? If it is an actual problem can you please provide details, callstack of the problematic case? >> >> And again, please re-consider calling 'rte_eth_dev_close()' or >> 'rte_dev_remove()' from the secondary process. >> >>>>>> +     * Namely, whether "eth_dev" is NULL cannot be used to determine whether >>>>>> +     * an ethdev port has been released. >>>>>> +     * For both primary process and secondary process, eth_dev->state is >>>>>> +     * RTE_ETH_DEV_UNUSED, which means the ethdev port has been released. >>>>>> +     */ >>>>>> +    if (eth_dev->state == RTE_ETH_DEV_UNUSED) { >>>>>> +        RTE_ETHDEV_LOG(INFO, "The ethdev port has been released."); >>>>>> +        return 0; >>>>>> +    } >>>> >>>> . >> .