From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id C698A5F36; Thu, 17 May 2018 14:59:58 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 May 2018 05:59:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,410,1520924400"; d="scan'208";a="40683247" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga008.fm.intel.com with ESMTP; 17 May 2018 05:59:57 -0700 Received: from fmsmsx124.amr.corp.intel.com (10.18.125.39) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 17 May 2018 05:59:56 -0700 Received: from fmsmsx117.amr.corp.intel.com ([169.254.3.235]) by fmsmsx124.amr.corp.intel.com ([169.254.8.29]) with mapi id 14.03.0319.002; Thu, 17 May 2018 05:59:56 -0700 From: "Wiles, Keith" To: Ophir Munk CC: "dev@dpdk.org" , Pascal Mazon , Thomas Monjalon , Olga Shern , Shahaf Shuler , "stable@dpdk.org" Thread-Topic: [PATCH v2] net/tap: fix device removal when no queues exist Thread-Index: AQHT7cQqvsWSa/E9okKhAfzZEOQVdqQ0V7eA Date: Thu, 17 May 2018 12:59:56 +0000 Message-ID: <1E0EA271-1891-4336-BCB8-A1A6A6F0BDF3@intel.com> References: <1526487019-3737-1-git-send-email-ophirmu@mellanox.com> <1526550455-14072-1-git-send-email-ophirmu@mellanox.com> In-Reply-To: <1526550455-14072-1-git-send-email-ophirmu@mellanox.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.255.79.233] Content-Type: text/plain; charset="us-ascii" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2] 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: Thu, 17 May 2018 12:59:59 -0000 > On May 17, 2018, at 2:47 AM, Ophir Munk wrote: >=20 > TAP device is created following its first queue creation. Multiple > queues can be added or removed over time. In Linux terminology those > are file descriptors which are opened or closed over time. As long as > the number of opened file descriptors is positive - TAP device will > appear as a Linux device. In case all queues are released (the > equivalent of all file descriptors being closed) the TAP device will > be removed. This can lead to abnormalities in different scenarios > where the TAP device should exist even if all its queues are released. > In order to make TAP existence independent of its number of queues - > an extra file descriptor is opened on TAP creation and is closed on > TAP closure. It's only purpose is to serve as a keep-alive mechanism > for the TAP device. >=20 > Fixes: bf7b7f437b49 ("net/tap: create netdevice during probing") > Cc: stable@dpdk.org >=20 > Signed-off-by: Ophir Munk > --- > v1: > Initial release > v2: > Reword commit message (a fixing patch) >=20 > drivers/net/tap/rte_eth_tap.c | 31 ++++++++++++++++++++++++------- > drivers/net/tap/rte_eth_tap.h | 1 + > 2 files changed, 25 insertions(+), 7 deletions(-) I did not see where ka_fd is set to -1 at startup, just in case we fail bef= ore the first open attempt and possible hit the close code in the remove ro= utine. I did not look at the complete driver, but I think it maybe reasonab= le to add that initial variable setup. >=20 > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.= c > index c006d07..6901edc 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); > + internals->ka_fd =3D -1; > + } > + /* > + * Since TUN device has no more opened file descriptors > + * it will be removed from kernel > + */ > } >=20 > static void > @@ -1561,13 +1570,18 @@ 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 < 0) { > + pmd->ka_fd =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 */ > --=20 > 2.7.4 >=20 Regards, Keith