From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id AD848DE3; Wed, 23 May 2018 07:22:44 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 May 2018 22:22:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,431,1520924400"; d="scan'208";a="49220308" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga002.fm.intel.com with ESMTP; 22 May 2018 22:22:43 -0700 Received: from fmsmsx154.amr.corp.intel.com (10.18.116.70) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 22 May 2018 22:22:43 -0700 Received: from BGSMSX108.gar.corp.intel.com (10.223.4.192) by FMSMSX154.amr.corp.intel.com (10.18.116.70) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 22 May 2018 22:22:43 -0700 Received: from bgsmsx101.gar.corp.intel.com ([169.254.1.244]) by BGSMSX108.gar.corp.intel.com ([169.254.8.91]) with mapi id 14.03.0319.002; Wed, 23 May 2018 10:52:39 +0530 From: "Varghese, Vipin" To: "Wiles, Keith" CC: Ophir Munk , "dev@dpdk.org" , "Pascal Mazon" , Thomas Monjalon , "Olga Shern" , Shahaf Shuler , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v4] net/tap: fix device removal when no queues exist Thread-Index: AQHT8NkQT73g8Huwb0qepDqV/O4X5KQ8wQbw//+lI4CAAGQfcA== Date: Wed, 23 May 2018 05:22:38 +0000 Message-ID: <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D1F88AA@BGSMSX101.gar.corp.intel.com> References: <1526632068-7467-1-git-send-email-ophirmu@mellanox.com> <1526889273-14691-1-git-send-email-ophirmu@mellanox.com> <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D1F8841@BGSMSX101.gar.corp.intel.com> <4ABF777E-475A-4BCA-9537-72F0EE72BA69@intel.com> In-Reply-To: <4ABF777E-475A-4BCA-9537-72F0EE72BA69@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZjM1NWJlNGEtOWFmNi00ZjA2LWEyYjktODUwZDhlOGJjOWFkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiVnJkZnhTYzJcL0E1akFBRzJCenZsRVpwRWZRS0pOaWxCek1RVDVEU28rbmxoa01NMWl1NGIwT1AzREdVeU92VE4ifQ== dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [10.223.10.10] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4] net/tap: fix device removal when no queues exist 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: , X-List-Received-Date: Wed, 23 May 2018 05:22:45 -0000 Sure, shared a suggestion. If not required can drop the same. > -----Original Message----- > From: Wiles, Keith > Sent: Wednesday, May 23, 2018 10:24 AM > To: Varghese, Vipin > Cc: Ophir Munk ; dev@dpdk.org; Pascal Mazon > ; Thomas Monjalon ; > Olga Shern ; Shahaf Shuler ; > stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v4] net/tap: fix device removal when no qu= eues > exist >=20 >=20 >=20 > > On May 22, 2018, at 11:50 PM, Varghese, Vipin > wrote: > > > > Hi Ophir, > > > > One suggestion shared inline to email > > > > > > > >> > >> diff --git a/drivers/net/tap/rte_eth_tap.c > >> b/drivers/net/tap/rte_eth_tap.c index > >> c006d07..52ef799 100644 > >> --- a/drivers/net/tap/rte_eth_tap.c > >> +++ b/drivers/net/tap/rte_eth_tap.c > >> @@ -929,6 +929,15 @@ tap_dev_close(struct rte_eth_dev *dev) > >> ioctl(internals->ioctl_sock, SIOCSIFFLAGS, > >> &internals->remote_initial_flags); > >> } > >> + > >> + if (internals->ka_fd !=3D -1) { > >> + close(internals->ka_fd); > > > > Do we need to notify the user which fd is been closed via LOG DEBUG? >=20 > Why would we want to have a LOG DEBUG here, it would make the debug > output a bit chatty IMO. I mean you could have one, but it seems ok as it= is to > me. >=20 > > > >> + internals->ka_fd =3D -1; > >> + } > >> + /* > >> + * Since TUN device has no more opened file descriptors > >> + * it will be removed from kernel > >> + */ > >> } > >> > >> static void > >> @@ -1549,6 +1558,7 @@ eth_dev_tap_create(struct rte_vdev_device > >> *vdev, char *tap_name, > >> dev->intr_handle =3D &pmd->intr_handle; > >> > >> /* Presetup the fds to -1 as being not valid */ > >> + pmd->ka_fd =3D -1; > >> for (i =3D 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) { > >> pmd->rxq[i].fd =3D -1; > >> pmd->txq[i].fd =3D -1; > >> @@ -1561,13 +1571,17 @@ eth_dev_tap_create(struct rte_vdev_device > >> *vdev, char *tap_name, > >> rte_memcpy(&pmd->eth_addr, mac_addr, > sizeof(*mac_addr)); > >> } > >> > >> - /* Immediately create the netdevice (this will create the 1st queue)= . */ > >> - /* rx queue */ > >> - if (tap_setup_queue(dev, pmd, 0, 1) =3D=3D -1) > >> - goto error_exit; > >> - /* tx queue */ > >> - if (tap_setup_queue(dev, pmd, 0, 0) =3D=3D -1) > >> + /* > >> + * Allocate a TUN device keep-alive file descriptor that will only b= e > >> + * closed when the TUN device itself is closed or removed. > >> + * This keep-alive file descriptor will guarantee that the TUN devic= e > >> + * exists even when all of its queues are closed > >> + */ > >> + pmd->ka_fd =3D tun_alloc(pmd); > >> + if (pmd->ka_fd =3D=3D -1) { > >> + TAP_LOG(ERR, "Unable to create %s interface", tuntap_name); > >> goto error_exit; > >> + } > >> > >> ifr.ifr_mtu =3D dev->data->mtu; > >> if (tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1, LOCAL_AND_REMOTE) < 0) @@ > >> -1961,9 +1975,12 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev) > >> > >> close(internals->ioctl_sock); > >> rte_free(eth_dev->data->dev_private); > >> - > >> rte_eth_dev_release_port(eth_dev); > >> > >> + if (internals->ka_fd !=3D -1) { > >> + close(internals->ka_fd); > >> + internals->ka_fd =3D -1; > >> + } > >> return 0; > >> } > >> > >> diff --git a/drivers/net/tap/rte_eth_tap.h > >> b/drivers/net/tap/rte_eth_tap.h index > >> babe42d..575dce4 100644 > >> --- a/drivers/net/tap/rte_eth_tap.h > >> +++ b/drivers/net/tap/rte_eth_tap.h > >> @@ -81,6 +81,7 @@ struct pmd_internals { > >> struct rx_queue rxq[RTE_PMD_TAP_MAX_QUEUES]; /* List of RX > queues */ > >> struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES]; /* List of TX queues > */ > >> struct rte_intr_handle intr_handle; /* LSC interrupt handle.= */ > >> + int ka_fd; /* keep-alive file descriptor */ > >> }; > >> > >> /* tap_intr.c */ > >> -- > >> 2.7.4 > > >=20 > Regards, > Keith