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 747E742828; Fri, 24 Mar 2023 02:27:47 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B5C124114A; Fri, 24 Mar 2023 02:27:46 +0100 (CET) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mails.dpdk.org (Postfix) with ESMTP id 282624068E; Fri, 24 Mar 2023 02:27:43 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1679621264; x=1711157264; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=zkSrpNRUIjrsisFtnhBHAoIi3/milllSjP9Sg4q7THk=; b=RGuq0ZY6gjwxVKcusbboa+7WL+wjOI5RQNoDMNMEhdU+OaiiRszg+mNr rdBudEy4vsmNEfVQwAcK+lKnFuZ8cxww3AkSqipAYcUG/gVwXpmIEkK3l eXM5HvW8gwra83sWPGIU/3oK/3AYk3OBZnMwx6oCM7YE39qFHRleCVWbR mdYfz9vHLGtuoOlrUxkJnjbmGl9ssSJfu82wB5r8Tq0HgOreuWnHWvpzc shbtzIh/biR+i0YgIv8pXLp1Tg7rjcGLYt7RX51rhu26eNbMvwS4uxKQT +i4IO8SudqdWIxkYN4uWQ3CLM3rMm5fAl5tH/M5Qc8oTwV1D/KvUYnB+V A==; X-IronPort-AV: E=McAfee;i="6600,9927,10658"; a="367410555" X-IronPort-AV: E=Sophos;i="5.98,286,1673942400"; d="scan'208";a="367410555" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Mar 2023 18:27:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10658"; a="684968457" X-IronPort-AV: E=Sophos;i="5.98,286,1673942400"; d="scan'208";a="684968457" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by fmsmga007.fm.intel.com with ESMTP; 23 Mar 2023 18:27:34 -0700 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) by ORSMSX603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21; Thu, 23 Mar 2023 18:27:33 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) 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.2507.21 via Frontend Transport; Thu, 23 Mar 2023 18:27:33 -0700 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (104.47.57.177) 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.2507.21; Thu, 23 Mar 2023 18:27:33 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YB76dTBA1bPeaN4eZUAwohuMYmMhCmlnYsgWR6GcH/sfzHrkUs+ZU4QFg5O2DS3GJBEOZkQXflPok28e6LHa6irWtRlbv+jGk2NDccjIeJMBH2GGLzE3uz4s4whq078MkD8jLbPF0SQbvvlvEahaHAW/yQ5AU6F6Fk/H7A+U84EqDgI/ggW8kFOpziGSJK7grg4jJDQI4CrCrcLgj76w4lyqHo1QQrowp+9jtsuMTTGB+opwxvvvT2BQSAE1DtilM3xxxR7/tVuUfnA4Wdnf1cwfxrJ2s65w3CqHIbFjp8ANLKNXJZxg1ezK3kZtDXYpKE7R0548yzBXuyrT3E2+cQ== 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=VYJMRZCSW5kJUtfCMmox1pPwydc9pvjIyT3BWqAD4Vo=; b=IVybGZyjT+YV/R8mGtPbLbhlAnfp/feqaZ4Nz1LGG4PHmzd9JKEu9gttnJZB7s1MzF0TzSYA2wcCjOyNEZ5cReeHHkmaeBx95hqPQXoxqDAtPIxglupldupABaJRKi8EtoUJV5SbaM6neGW8YlvbwOm9ZNxvqBP+e37Y6bkaOcEZRPEl9WVb62p35h8wPdxQu7Nu5DdW1kshdKemE87HCRt76T2XODUDx8x4G2HLIyf2VqPisx+A2qVTp3usUnH5i8SELT5nVtbRP5TmLG0ELLz3SkhzTDnuvkS1hhoKqT5W4K9i5mxDiJIFvjjLzk76Z6tPV8oFAeif+xvlEO7xZg== 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 Received: from DM6PR11MB3530.namprd11.prod.outlook.com (2603:10b6:5:72::19) by SJ0PR11MB6768.namprd11.prod.outlook.com (2603:10b6:a03:47f::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6178.38; Fri, 24 Mar 2023 01:27:31 +0000 Received: from DM6PR11MB3530.namprd11.prod.outlook.com ([fe80::2a1b:9bf9:445f:567f]) by DM6PR11MB3530.namprd11.prod.outlook.com ([fe80::2a1b:9bf9:445f:567f%3]) with mapi id 15.20.6178.037; Fri, 24 Mar 2023 01:27:31 +0000 From: "Huang, Wei" To: Thomas Monjalon CC: "dev@dpdk.org" , "david.marchand@redhat.com" , "stable@dpdk.org" , "Xu, Rosen" , "Zhang, Tianfei" , "Zhang, Qi Z" Subject: RE: [PATCH v1] raw/ifpga: remove virtual device unplug operation Thread-Topic: [PATCH v1] raw/ifpga: remove virtual device unplug operation Thread-Index: AQHZWHHvun1srISqFUW9KK/CSBP1OK8D2uEAgACG2SCAAIedgIAAAFNwgAAltoCAAPfrAIAAsegAgADg8PCAAH53gIABEc8w Date: Fri, 24 Mar 2023 01:27:31 +0000 Message-ID: References: <20230316204445.360330-1-wei.huang@intel.com> <5367342.29KlJPOoH8@thomas> <1923267.fIoEIV5pvu@thomas> In-Reply-To: <1923267.fIoEIV5pvu@thomas> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: DM6PR11MB3530:EE_|SJ0PR11MB6768:EE_ x-ms-office365-filtering-correlation-id: f4c6faae-8d38-4056-6262-08db2c06f01a x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 8nIys675QpjmPV+hNrHX7Ym3kyUGYiG4n5Y1zDm6arhOfZdlOjRH9DoXVIB4AqNQAIzSZ+k2pzyhcPVNIjO3hAH4uI3Z+fsuIhdF4ecGXgoilCtvZBF81apJv3CZ54MHVgto8Fym+e3Rw1E1CQS3OvGt9Kq/uNrTIMVqenRytMXA6XCEA+5XaDU4fOGTSVsCmRcgPRLt6iyWBH4Mnpvw27qG4rNKJEpd1vbGkFs4Blj34J8vqfOnuRGjwziDSeR4y3KXqWBA8H6Lhq02RNvCsLgGEo4d0hSxWsJWhoQbv2Y4DugiiVcyyYI9bhHJMO0ubDcXDphdjkJze3QMKjBFsDttmLZfjrG3UkGSQxoqA8LEvxM75e4bF7wv0kA6IEVKQYkT6x9to3/e1EohjG5DfhW4aE2m3ony7DSEuN2uxw74pd/fTVpOUN/vNgX9LGUR+5pgkfhJI/+uJlevFdV+cdVJPAwDOCHwwe3gIB0ytM+gpn1KHET+gezRiBIbrOZLhnyuw8k4Yudl9BFljraoaV52ziDnHsvAAvHG0FS+aw6zwwUWZ6CnqGbAptm7wWUp1UGKG2RfjrsMf+Zq0CM5LIJrVTue2+LOppqtlERSLrY2iWAJzyNek8/PhbeBhEggA/t68I2/Ll+bvD2UUNt7mCHg1ojiQdIvUyyM9TjFLnu/E1W7sdtDgFLUucI7soKhhn1PlPog0rqfysAF1CSvtA== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR11MB3530.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(376002)(346002)(39860400002)(396003)(366004)(136003)(451199018)(7696005)(54906003)(316002)(478600001)(55016003)(71200400001)(83380400001)(41300700001)(9686003)(38100700002)(53546011)(26005)(186003)(6506007)(86362001)(38070700005)(66946007)(4326008)(6916009)(64756008)(66556008)(8676002)(66476007)(66446008)(76116006)(82960400001)(107886003)(33656002)(122000001)(8936002)(2906002)(52536014)(5660300002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?s+OPtiKCR1lXKk0z9fHhESSIjSqp18CvtJmfoRA3q+/40l/9aCYAd3P74amR?= =?us-ascii?Q?+0wksg8WptWIEUWp9i/fr1opf4HuioXBa2WKp7blsgCLJ+8k3LAtrLQlwlsy?= =?us-ascii?Q?wZIwXjSq6VjQcKWwPhu2iCJSD+ns8dlkSbIV6rHx42caWq1YUxqdOVfyKaAI?= =?us-ascii?Q?BzPSRNm+3PcaitA3RSDW4cS48bVEGCViYfwGFUKkeSQPIKIHtBvhmlZg8HkY?= =?us-ascii?Q?gNaZiCOL6hKr94IQG2xGm+OVYttDl71fMT0zMI9mUOc2VmjTE9je3pUL1HUW?= =?us-ascii?Q?cTBzajKepT77WGtvosWA5NH8pjvSnUsKZYreW8zNln3UEtP8toQrJTpLSO8j?= =?us-ascii?Q?kgh8S/yAeizvOa26SMZMK6iWJkNjdFQ6a1uugWXb20VxhT//GvALyHAwvGzb?= =?us-ascii?Q?WXFIYBbuxvCLIBg3areGQz5WXuF7UiPVTLBWekhEGdpmUL1QXdlmX2RdqNac?= =?us-ascii?Q?rvto0un1yLDkSALj8bx4FBJXZGtpNSMyxTK064MAL0udnSZaCDhCeEJmS93l?= =?us-ascii?Q?/vtoBVVgam+NJr1lRujvF3vkWp9YFvgtPfLmPJSaArgJYFbAaLxSrgTWZMuv?= =?us-ascii?Q?D74dOLotmrNvQrfG4xojU/+gJRmDFey1rbEDD6RwXedoQESCp/8myMbRQYa2?= =?us-ascii?Q?K8QTyf9w4XFOK0OOPJtdku0mDHn2W79owozMksN+6sgAMpVpmTHH/cPV+U9K?= =?us-ascii?Q?WP9dZXueF4lbVxVjtApTIXq1tHAfQn0Nj2X+Kebeoz4hks+Fe3Nzj6cxlH/u?= =?us-ascii?Q?LvlRRLm5UKKyGR56AeTTkIfVh236s/N2nrVPk6SsNcDz0g2B7wUs08d4jCrp?= =?us-ascii?Q?fFnKRmAH62ALjWaAyDUFjJUY7KSEUxY4JZsyf3IVIvjQ4MCGyXLQB7gHrONy?= =?us-ascii?Q?rK20hj7ks7X/Yr9JUyO6wqlWKUdjtdZLK4jFah4+jBqfZzBYs2zWOgxOFP2Q?= =?us-ascii?Q?zBQw58B/34yaEg4xneuMTEBW0p0WPGb8SqugHn59EPWoSnmqy1h/Ox0cwNab?= =?us-ascii?Q?IMOBfV1r6QFqQUQMjxPvF4L8uinTGIaoPp7Ci+oouaxzBoiOlfG28MpAQtM9?= =?us-ascii?Q?JF9KABoTsWBSzxo3JHvPIlJ64ZGYHfVM4ISd9jj3GQBqbLUTuuBl6NEY7Mp/?= =?us-ascii?Q?HO77wwHPCjGFrdbgJrz3qraL5MPFmgeMjMVYT4ypw8YrNeXw/PS+u31YX9i6?= =?us-ascii?Q?6XYsNIxWuC/TXuks6Eb8/wfY6lHRC1t2csk232WvfuJ4ImLzsazNfa2id3JL?= =?us-ascii?Q?jegbsziQN3ofnhQosCts1PmGzdpY3HN41KG9tPySHeJNT9G3yv/euOvUcUSV?= =?us-ascii?Q?C8yh0nfCCAdVWIG9waM+X29l2513ZT2TVukU5m+d66uysBR4PIo+gmMC5OBm?= =?us-ascii?Q?VhEz/+E3XjZPui23au3g2xDz+BLEZ9WJY+2h1YurhH+FIszgtVm9yVOV9Pql?= =?us-ascii?Q?yoUoA7G+puQCGUJYUWRK6rDAU2okwOesL71dRGEAqRACJb0JrRHY6nbmaJt2?= =?us-ascii?Q?lFVhrBgKYyVnbEzM+qQiLREwSh8rNKhTsLbzggemgubuEP1u+rK+/tOTvzbj?= =?us-ascii?Q?pUTUyvWApjA2FkVokZ1yXYZW6A6CLGNT3tSs0pyK?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3530.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: f4c6faae-8d38-4056-6262-08db2c06f01a X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Mar 2023 01:27:31.2431 (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: IXTfJRMIeEIvGYI/OXfXc+toNP2/mCHqhKCFyIgn7hPeWWhKs/HpK3WzaoGcSG+HDxzVE1p5JUHdd2l72eVBXg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB6768 X-OriginatorOrg: intel.com 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 > -----Original Message----- > From: Thomas Monjalon > Sent: Thursday, March 23, 2023 16:52 > To: Huang, Wei > Cc: dev@dpdk.org; david.marchand@redhat.com; stable@dpdk.org; Xu, > Rosen ; Zhang, Tianfei ; > Zhang, Qi Z > Subject: Re: [PATCH v1] raw/ifpga: remove virtual device unplug operation >=20 > 23/03/2023 04:26, Huang, Wei: > > From: Thomas Monjalon > > > 22/03/2023 02:26, Huang, Wei: > > > > From: Thomas Monjalon > > > > > 21/03/2023 09:41, Huang, Wei: > > > > > > From: Thomas Monjalon > > > > > > > 21/03/2023 01:11, Huang, Wei: > > > > > > > > From: Thomas Monjalon > > > > > > > > > 16/03/2023 21:44, Wei Huang: > > > > > > > > > > VDEV bus has implemented cleanup() function to perform > > > > > > > > > > cleanup for devices on the bus during eal_cleanup(), > > > > > > > > > > so there is no need for ifpga driver to record virtual > > > > > > > > > > devices and > > > unplug them. > > > > > > > > > > > > > > > > > > Why no need? > > > > > > > > > If the application wants to explicitly remove a device, > > > > > > > > > what > > > happens? > > > > > > > > > > > > > > > > > > > > > > > > > > EAL will output an error information "Cannot find plugged > > > > > > > > device > > > (%s)". > > > > > > > > > > > > > > It does not look what we expect. > > > > > > > > > > > > > Let me clear it. > > > > > > With this patch, no error information will be outputted. > > > > > > Without this patch, error information will be outputted. > > > > > > Because bus cleanup action will unplug virtual device, then > > > > > > ifpga PMD unplug the virtual device which is already be > > > > > > cleanup, > > > > > > > > > > Why ipfga unplug the device after the bus cleanup? > > > > > I'm not following. > > > > > > > > > The virtual device is created upon ifpga, if VDEV bus doesn't > > > > perform cleanup, ifpga has the responsibility to unplug these virtu= al > devices. > > > > > > Really I don't understand the flow. > > > Are you talking about EAL cleanup case? > > Yes, it's about EAL cleanup. > > > What happens first? Do you need ifpga to be called first? > > The cleanup flow is rte_eal_cleanup() -> eal_bus_cleanup() > > eal_bus_cleanup() will call each bus's cleanup method to complete clean= up > work. > > There are three types of device related to ifpga PMD: PCI device, VDEV > device and AFU device. > > VDEV device is created on PCI device, it's a mediate device which purpo= se > is to plug a AFU device on IFPGA bus. > > Before eal_bus_cleanup() is implemented, application will call > rte_pmd_ifpga_cleanup() to remove PCI device, VDEV device will be > removed when PCI device is removed, AFU device will be removed when > VDEV device is removed, it works fine. > > Now eal_bus_cleanup() takes the job, application has no need to call > rte_pmd_ifpga_cleanup(), ifpga PCI device has no need to remove ifpga > VDEV device and ifpga VDEV device has no need to remove ifpga AFU device. >=20 > That's the problem in your approach. > You need to keep the code removing children devices. > And if all children are removed, then the parent should remove itself. > If you implement these 2 conditions, the cleanup can happen in any order. >=20 OK, this patch should be rejected, I will commit another patch according to= your comments. > > > I think you need the correct checks to allow any order of cleanup. > > When this patch is committed, no order dependent on cleanup. > > > > > > > > bus->find_device() returns NULL, > > > > > > EAL output "Cannot find plugged device (%s)\n" at line 302 in > > > > > > eal_common_dev.c > > > > > > > > > > Anyway, the good answer is not to completely remove the "remove" > > > > > operation. > > > > > > > > > If not to completely remove the "remove", the same virtual device > > > > will be > > > unplug twice, is it reasonable? > > > > > > You need to add a check to not unplug something already unplugged. > > > But you must allow the user calling "remove" directly. > > > > > rte_pmd_ifpga_cleanup() is the only one interface for user to calling > > "remove" directly, >=20 > No, there are functions in EAL to "remove" devices, like rte_dev_remove()= , > and we must make sure it works effectively. >=20 > > when this patch is committed, VDEV and AFU device will not be unplugged > twice. > > For PCI device, the implementation of rte_pmd_ifpga_cleanup() is like > below > > for (i =3D 0; i < IFPGA_RAWDEV_NUM; i++) { > > dev =3D &ifpga_rawdevices[i]; > > if (dev->rawdev) { > > rte_rawdev_pmd_release(dev->rawdev); > > dev->rawdev =3D NULL; > > } > > } > > If ifpga PCI device is already removed, dev->rawdev is NULL, it will no= t be > unplugged again. >=20 >=20 >=20