From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 777B9235; Wed, 23 May 2018 06:53:49 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 May 2018 21:53:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,431,1520924400"; d="scan'208";a="201679704" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga004.jf.intel.com with ESMTP; 22 May 2018 21:53:47 -0700 Received: from fmsmsx114.amr.corp.intel.com (10.18.116.8) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 22 May 2018 21:53:46 -0700 Received: from fmsmsx117.amr.corp.intel.com ([169.254.3.235]) by FMSMSX114.amr.corp.intel.com ([10.18.116.8]) with mapi id 14.03.0319.002; Tue, 22 May 2018 21:53:45 -0700 From: "Wiles, Keith" To: "Varghese, Vipin" 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: AQHT8lGOq/r229Wi2ES9BlPubkEjRaQ9NMAA Date: Wed, 23 May 2018 04:53:45 +0000 Message-ID: <4ABF777E-475A-4BCA-9537-72F0EE72BA69@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> In-Reply-To: <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D1F8841@BGSMSX101.gar.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.53.246] Content-Type: text/plain; charset="us-ascii" Content-ID: <02321858A114904F9246A6C7CBF6336B@intel.com> 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 04:53:50 -0000 > On May 22, 2018, at 11:50 PM, Varghese, Vipin = wrote: >=20 > Hi Ophir, >=20 > One suggestion shared inline to email >=20 > >=20 >>=20 >> 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); >=20 > Do we need to notify the user which fd is been closed via LOG DEBUG? 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 >> + */ >> } >>=20 >> 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; >>=20 >> /* 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)); >> } >>=20 >> - /* 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 be >> + * closed when the TUN device itself is closed or removed. >> + * This keep-alive file descriptor will guarantee that the TUN device >> + * 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; >> + } >>=20 >> 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) >>=20 >> close(internals->ioctl_sock); >> rte_free(eth_dev->data->dev_private); >> - >> rte_eth_dev_release_port(eth_dev); >>=20 >> + if (internals->ka_fd !=3D -1) { >> + close(internals->ka_fd); >> + internals->ka_fd =3D -1; >> + } >> return 0; >> } >>=20 >> 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 */ >> }; >>=20 >> /* tap_intr.c */ >> -- >> 2.7.4 >=20 Regards, Keith