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 15DE0A0C46; Wed, 18 Aug 2021 13:25:01 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9DEFE4069E; Wed, 18 Aug 2021 13:25:00 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id 27DA040151 for ; Wed, 18 Aug 2021 13:24:57 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10079"; a="203452888" X-IronPort-AV: E=Sophos;i="5.84,330,1620716400"; d="scan'208";a="203452888" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Aug 2021 04:24:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.84,330,1620716400"; d="scan'208";a="511007994" Received: from orsmsx604.amr.corp.intel.com ([10.22.229.17]) by fmsmga004.fm.intel.com with ESMTP; 18 Aug 2021 04:24:56 -0700 Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) 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; Wed, 18 Aug 2021 04:24:56 -0700 Received: from orsmsx607.amr.corp.intel.com (10.22.229.20) by ORSMSX611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10; Wed, 18 Aug 2021 04:24:55 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx607.amr.corp.intel.com (10.22.229.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10 via Frontend Transport; Wed, 18 Aug 2021 04:24:55 -0700 Received: from NAM02-BN1-obe.outbound.protection.outlook.com (104.47.51.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; Wed, 18 Aug 2021 04:24:55 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UdbHp/FokjLLd3XWnADjPIR1HMEOxET1dCaA9/lFLkCg+onszA4Dnr2jNcGzGSIgxARwmv2cjLIJ/5NTb/hj2DhQ/NqD/0bRhqXbHj7ye+KWnjcWinZh4UZuKzWdIt1r3pRunWp53r+s2F7PJD6xStloY0RkowPwHW2QZq3p8Mvy5655Mre8ssJf4/SjxhxKMmcqcVnTTuiddbha+FiZUOP7jo1zEFbQrIuTEXapYJ6iUpGAY6/qZe+Ev2RphMPgxRbGQN0ORzeW1wGvo25uKBF98anEFZIy7nlaOKpVOZaN6sJ9uOrbNj7tmKiheCb16WRbad/IiozKQ044i80sOw== 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=ff0wHmew8x1G/58QA3KC5ctk0OG8LF3O3bo1bjfUVjU=; b=T0gxJJFGPFEdHHRTGxx3KiC1fVX6J7sygEywHkx6l2tkPdqN5yv+jaca05yCPx/t8qdle5nHWlOD+uSVH4p5yVh8LIcbOjBNEzzCm06ohUn3EzEpTSB3T7tE0OZ3blQR15RnScMUl5rqbN3HxULKXKpb50HQeZTg6jK7FFZ/9BcwgbBMfnEeMWAON4ljlWK/mKCJ2yF45Bf384yi9HSh8BuJqTnXFsS82JLq6KhcHaIJ7Ii8X2HdqK/fDTGV3ER7ZndQ40h2XEmlu9Sljy0e3bttvw/P5Jl9sYA8SfGG6c9waycPh/p01BCClX2eUP/+NUeOzg+cxP7tOUFnC0UiFQ== 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=ff0wHmew8x1G/58QA3KC5ctk0OG8LF3O3bo1bjfUVjU=; b=RDg1UJaOsuBuATaqYjIsll/SODq1R28YjOMmcPl/axx4PGGTF7k5t0F2oPnr3dfOQ9TfL+JVgbWA9FE9MkrEni7za2D2a5fKDqvDJkDT/mo84DMc9tByfOg1/Cc1glx6v6tVdWMW+VasClPn+XBuUkDZjD/ByBX20/0dh7+4xYQ= 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 PH0PR11MB5109.namprd11.prod.outlook.com (2603:10b6:510:3e::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4436.19; Wed, 18 Aug 2021 11:24:54 +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.019; Wed, 18 Aug 2021 11:24:54 +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: Wed, 18 Aug 2021 12:24:47 +0100 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-ClientProxiedBy: ZR0P278CA0055.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:21::6) 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 ZR0P278CA0055.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:21::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4436.19 via Frontend Transport; Wed, 18 Aug 2021 11:24:52 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 4d9ee11f-0453-4b53-064f-08d9623acd37 X-MS-TrafficTypeDiagnostic: PH0PR11MB5109: 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: ROoTSYeVj+VUleBbcKqhU5HMF6zL/tHjRLejwyZhVbw9ArfIVkT10zcaHTvxHdZW2S+lJTvkgblHVDJIDcGsM1MwXzO3hApySvI8dhZaFLPPug9pSz1VRmLmZGJvAkQXBRrhzcQygyhhg2mGgjnHAQPMmmoMjCcQUJ3CA+6ruIl/jKqNz3srE+pZzCW6lypl0lt0WkpM6lVpDO4P6YU3kNj/JWS3ZxAqxdzxmGwWwhb+Op9CX23lfw0gsoGPP93s7bxA5ESLPsa0PmHUYfplHPhaLnwEEzySVXYE4oHAvNj0o7Inky0DUErw+MqVjBEqBfDY5rf5hK72WJZWTCBgzdSnSVmuRh7cnrwhrXBubdRaaggCABX7ZyCfVKdEvYi/J7XIKIeTr51s9It5DdUUv+S/ofrho/p8o5TODkSj/lnR//HQlIlKJ+BYhGf6G0xeEqjDMw9gct4tZdt/h9p4dWsJe2HKMnEw3xvk2FXCAizHtk8XdbmfF552u9wEJHJL0LSbRtHxHcnehQl6C33Px4Z8Q52U32NGoLiaxzuVAuFt8/TrR4KEYK7ZODgK7g2q21qmffH8D7VviWivNYmkTUNprAbQg5cDPGqJxq7X/2Tr4NTQe3imyAdZgTtGoRrQfCqDpN0bXx73MDYxVBdQa9I25RCpRjzkE6xUOjrR4WlKBl0LuL+NAuQ5Pj+rNen+13ABOFAervPc+qmfHkPTNyV6VnGedSBtjFic45lvtnA= 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)(39860400002)(376002)(136003)(396003)(366004)(346002)(8936002)(8676002)(2906002)(83380400001)(86362001)(31696002)(2616005)(956004)(38100700002)(44832011)(54906003)(53546011)(5660300002)(31686004)(4326008)(110136005)(478600001)(66946007)(6486002)(6666004)(36756003)(66556008)(66476007)(186003)(26005)(316002)(16576012)(21314003)(45980500001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZmtZQ3RwMkNIQ2YwNmRIUmNNRm1wa1RXZlVUT01WRWwweld5MVY2dzZkZ3ZF?= =?utf-8?B?SHBhR0NrV25rc0NFQmg0cDI1b2xMNGliMlNMazV1cGdoNUxab29LVWM2bGh2?= =?utf-8?B?OW4rU28xNVJWSTZZMStSeGFQWEZWb0ZoTjV5WUhudWlPb0FBR3Bxak9ZemZq?= =?utf-8?B?ZEtjcEVyNmhxVTlMMFJ6Y3JzMkN0aE1NWDcrM05wMVVzZFFFZ2VTaDZSSk92?= =?utf-8?B?R1dLVk1jTWhlN2RmYndBZ3FzRlhkNWdJNVQ0RGkyV2hzZUNKZG55VUlMcnRO?= =?utf-8?B?dWJCM1JyZlJpK3hvandXbVhldEJNclZTWlo5alVEU0p1cTVvMzNmMFlSMVow?= =?utf-8?B?RTA2d1FPS05SWTlJVmhMeGJSa1NXR0ZVZ0hkMUsxVWY4RHRzamFyZEdTcFBh?= =?utf-8?B?Q0VLdTQ1eU44d1pJSnNuUDFxR09ta2trRnpMdm03VmxlSUdVdDBxeS9YRHlF?= =?utf-8?B?Wk9uaWZNajhlakUxRGhkUjJUWGFQeTJJNGJxeWdEVjlyR0JTMGRJWHoxaWJ4?= =?utf-8?B?TU5iMkI3b2wwMXc4Q3VaRjZRY1JYUXhZUXVJYmR2aERURVpjaGdORjJENEQ3?= =?utf-8?B?RmljMW9WL2dHdUhCeWtLUStCanhMTHJFbE5vMGFiOFBXMU9aUzkzbkdWUnF6?= =?utf-8?B?a1BjL254c2dBZmE5dklVcXpTc2x2c3NxYkEyckhON1gwYjhOa3lMTTRsNWtx?= =?utf-8?B?T0p3YlZaY0MxQmY5RUxhUHMyN2VCTHUrb0IwM1FXV3h2ZUo3NjB2Qmt3M3ZC?= =?utf-8?B?N24zT3ZzYUdvVkZ5aU9hQXdkb2JVT2hXNzlRaC9SQ0ZkNGljVHRsanJvQmxC?= =?utf-8?B?OW9udFJKRVhRbkVLZXhhRVBRNEd4YTB0YlJYR053VitmU21mS0FXV0ZIRkdE?= =?utf-8?B?YnhXTUtxOTE5disxUkx5cURtRVd0UTlOL3I4ZC9icGliY3ZYMWl3NURWakk2?= =?utf-8?B?bDFHYXhOcFA0SklMaEJzTGZBaEJxaS9JWDlWdThwc01BTXhPcytHNndCWE1Y?= =?utf-8?B?dXZlc3pIY1hhaVNSMUhiR1Fkd0dSbEV6RzgwNTlWcVlTUDFWdlljUWttZ2Qz?= =?utf-8?B?VHhEMi9Ydmhya0p6QjZPS09DSUpNUTgyaG5EcUtMRTFIZ2s3NjEvNGxxODk1?= =?utf-8?B?ZXU2SElYeTJlN21ZOGIvenpQQjByZ3ZqdUdnVm82REJwQS9WaGhTRHdVcDE2?= =?utf-8?B?bW1NbWhvK3lIUEY2QUdabnZoT29XZ2JIVnFvYlRsZ2tRdldZQXVmcUE4WnA2?= =?utf-8?B?ejF5TUNlQmlkMXZWMlNKYzRTTDhZY2llWUQ0NzFmMlc5ZHE0cnkxZ0Vqck9S?= =?utf-8?B?UkhnU2hGYkR2ZmtVcCt2SEh2WjhGbnZ6ckV6S2N4MzI4OFptWEIvRS9ndVBx?= =?utf-8?B?V0tiR0ErajltckhlU3JXdVIxQXdXeE5iV09TZ0ZVSGtrUDN6c0RxdXIzWGRs?= =?utf-8?B?bE12SGNjZFlIamRkT1hFZHVCb2FiVWtmVG9hMnhUbDFkdWRnaVNVRHdqWERV?= =?utf-8?B?b0hPbWVZTkg5TXFpWTdJQXJRQk5HRVZXM1cxdmxNZkxCZHNiZ3ZORTRnQllu?= =?utf-8?B?RGQ3c0hoSjRqTTA4Y0svTUZDNFhlMWRwUUxkYy9SQVBWVEliREVld0trcm01?= =?utf-8?B?b3lhSjN6ZEU2bmFjc1JLWUlwbTcrZDBiWU5xdXhRSDhpZEViV1oxK0o4dlli?= =?utf-8?B?K3NBMG9kbHZ1bnpFakQ1aGNBaFRBK3g2QXNrd20yTmorZzEwdERFYmYrc3Zh?= =?utf-8?Q?VXIMgfAYaTp1LIJd3UZU5KN4X1GHZ1aTOYI76aT?= X-MS-Exchange-CrossTenant-Network-Message-Id: 4d9ee11f-0453-4b53-064f-08d9623acd37 X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5000.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Aug 2021 11:24:54.1500 (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: bjWUhZMeUMCRiSfxtF1EOpg+ota95SE+r9QUW3lWz9N/jYbawKFVURRvMrh+W6CMRl29wEdwuQEClI0o8pGYng== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5109 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/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. 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. 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. 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.) 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; >>>> +    } >> >> >> .