From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id CB055A04BB; Thu, 17 Sep 2020 14:35:55 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A8F041D639; Thu, 17 Sep 2020 14:35:55 +0200 (CEST) Received: from huawei.com (szxga01-in.huawei.com [45.249.212.187]) by dpdk.org (Postfix) with ESMTP id 93DB21D634 for ; Thu, 17 Sep 2020 14:35:50 +0200 (CEST) Received: from DGGEMM406-HUB.china.huawei.com (unknown [172.30.72.53]) by Forcepoint Email with ESMTP id 924D3A12C7DD204D6732; Thu, 17 Sep 2020 20:35:46 +0800 (CST) Received: from DGGEMM533-MBX.china.huawei.com ([169.254.5.42]) by DGGEMM406-HUB.china.huawei.com ([10.3.20.214]) with mapi id 14.03.0487.000; Thu, 17 Sep 2020 20:35:37 +0800 From: wangyunjian To: Thomas Monjalon CC: "dev@dpdk.org" , "ferruh.yigit@intel.com" , "Lilijun (Jerry)" , xudingke , "keith.wiles@intel.com" Thread-Topic: [dpdk-dev] [PATCH] net/tap: release port upon close Thread-Index: AQHWia6e0zpSV35ZLUS+t1WjJlHjaalsyJKA Date: Thu, 17 Sep 2020 12:35:36 +0000 Message-ID: <34EFBCA9F01B0748BEB6B629CE643AE60DA513F0@DGGEMM533-MBX.china.huawei.com> References: <9a94f272bc7c1e5650295ddf3e02fb19b15e7951.1598596042.git.wangyunjian@huawei.com> <2500283.dqYlU2TMh5@thomas> In-Reply-To: <2500283.dqYlU2TMh5@thomas> Accept-Language: en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.174.185.168] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH] net/tap: release port upon close X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Sunday, September 13, 2020 5:17 PM > To: wangyunjian > Cc: dev@dpdk.org; ferruh.yigit@intel.com; Lilijun (Jerry) > ; xudingke ; > keith.wiles@intel.com > Subject: Re: [dpdk-dev] [PATCH] net/tap: release port upon close >=20 > Hi, >=20 > 28/08/2020 14:37, wangyunjian: > > @@ -1078,6 +1085,23 @@ tap_dev_close(struct rte_eth_dev *dev) > > + > > + /* mac_addrs must not be freed alone because part of dev_private */ > > + dev->data->mac_addrs =3D NULL; > > + > > + internals =3D dev->data->dev_private; > > + TAP_LOG(DEBUG, "Closing %s Ethernet device on numa %u", > > + tuntap_types[internals->type], rte_socket_id()); > > + > > + if (internals->ioctl_sock !=3D -1) { > > + close(internals->ioctl_sock); > > + internals->ioctl_sock =3D -1; > > + } > > + rte_free(dev->process_private); > > + dev->process_private =3D NULL; > > + if (tap_devices_count =3D=3D 1) > > + rte_mp_action_unregister(TAP_MP_KEY); > > + tap_devices_count--; > > } > [...] > > @@ -2446,12 +2466,11 @@ static int > > rte_pmd_tap_remove(struct rte_vdev_device *dev) { > > > > struct rte_eth_dev *eth_dev =3D NULL; > > > > - struct pmd_internals *internals; > > > > /* find the ethdev entry */ > > eth_dev =3D rte_eth_dev_allocated(rte_vdev_device_name(dev)); > > if (!eth_dev) > > > > - return -ENODEV; > > + return 0; > > > > /* mac_addrs must not be freed alone because part of dev_privat= e > */ > > eth_dev->data->mac_addrs =3D NULL; >=20 > The line above can be removed because mac_addrs is not freed in secondary > process, and it is redundant with tap_dev_close(). Thanks for the suggestion. I have fixed it in v2. https://patchwork.dpdk.org/patch/78048/ >=20 > > > > if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > > return rte_eth_dev_release_port(eth_dev); >=20 > There is an inconsistency in secondary process if tap_dev_close() is not = called > from .remove (unplug) but can be called from .dev_close (rte_eth_dev_clos= e). >=20 > I think the process type check must be done inside tap_dev_close(), so th= e > process private resources can be freed. >=20 > > > > tap_dev_close(eth_dev); > > >=20 > This blank line can be removed in my opinion. OK >=20 > > - internals =3D eth_dev->data->dev_private; > > - TAP_LOG(DEBUG, "Closing %s Ethernet device on numa %u", > > - tuntap_types[internals->type], rte_socket_id()); > > - > > - close(internals->ioctl_sock); > > - rte_free(eth_dev->process_private); > > - if (tap_devices_count =3D=3D 1) > > - rte_mp_action_unregister(TAP_MP_KEY); > > - tap_devices_count--; > > rte_eth_dev_release_port(eth_dev); > > > > return 0; >=20 >=20