From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id A414F2BE5 for ; Fri, 3 Feb 2017 21:32:35 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP; 03 Feb 2017 12:32:34 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,330,1477983600"; d="scan'208";a="1102765107" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga001.fm.intel.com with ESMTP; 03 Feb 2017 12:32:34 -0800 Received: from fmsmsx113.amr.corp.intel.com ([169.254.13.230]) by FMSMSX108.amr.corp.intel.com ([169.254.9.130]) with mapi id 14.03.0248.002; Fri, 3 Feb 2017 12:32:34 -0800 From: "Wiles, Keith" To: Pascal Mazon CC: "dev@dpdk.org" Thread-Topic: [PATCH 2/5] net/tap: fix multi-queue support Thread-Index: AQHSfgFE2y4tPZ1hjUyWgAwpaRlF9qFYQ2QA Date: Fri, 3 Feb 2017 20:32:33 +0000 Message-ID: <51B6DC50-F585-4979-8F68-BC4B160A9D60@intel.com> References: <20170202223330.39240-1-keith.wiles@intel.com> <20170202223330.39240-2-keith.wiles@intel.com> <20170203103740.60e76f0e@paques.dev.6wind.com> In-Reply-To: <20170203103740.60e76f0e@paques.dev.6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.252.141.150] Content-Type: text/plain; charset="us-ascii" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 2/5] net/tap: fix multi-queue support 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: Fri, 03 Feb 2017 20:32:36 -0000 > On Feb 3, 2017, at 3:37 AM, Pascal Mazon wrote: >=20 > On Thu, 2 Feb 2017 16:33:27 -0600 > Keith Wiles wrote: >=20 >> Signed-off-by: Keith Wiles >> --- >> drivers/net/tap/rte_eth_tap.c | 93 >> ++++++++++++++++++++++--------------------- 1 file changed, 48 insertion= s(+), >> 45 deletions(-) >>=20 >> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap= .c >> index 3f179c3..9ed7a87 100644 >> --- a/drivers/net/tap/rte_eth_tap.c >> +++ b/drivers/net/tap/rte_eth_tap.c >> @@ -115,10 +115,9 @@ struct pmd_internals { >> * supplied name. >> */ >> static int >> -tun_alloc(char *name) >> +tun_alloc(char *name, uint16_t qid) >> { >> struct ifreq ifr; >> - unsigned int features; >> int fd; >>=20 >> memset(&ifr, 0, sizeof(struct ifreq)); >> @@ -133,55 +132,57 @@ tun_alloc(char *name) >> goto error; >> } >>=20 >> - /* Grab the TUN features to verify we can work */ >> - if (ioctl(fd, TUNGETFEATURES, &features) < 0) { >> - RTE_LOG(ERR, PMD, "Unable to get TUN/TAP features\n"); >> - goto error; >> - } >> - RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features); >> + /* This can only be done once per interface */ >> + if (qid =3D=3D 0) { >> + unsigned int features; >> + >> + /* Grab the TUN features to verify we can work */ >> + if (ioctl(fd, TUNGETFEATURES, &features) < 0) { >> + RTE_LOG(ERR, PMD, "Unable to get TUN/TAP >> features\n"); >> + goto error; >> + } >> + RTE_LOG(DEBUG, PMD, "TUN/TAP Features %08x\n", features); >>=20 >> #ifdef IFF_MULTI_QUEUE >> - if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES > 1)) { >> - RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n"); >> - goto error; >> - } else if ((features & IFF_ONE_QUEUE) && >> - (RTE_PMD_TAP_MAX_QUEUES =3D=3D 1)) { >> - ifr.ifr_flags |=3D IFF_ONE_QUEUE; >> - RTE_LOG(DEBUG, PMD, "Single queue only support\n"); >> - } else { >> - ifr.ifr_flags |=3D IFF_MULTI_QUEUE; >> - RTE_LOG(DEBUG, PMD, "Multi-queue support for %d queues\n", >> - RTE_PMD_TAP_MAX_QUEUES); >> - } >> + if (!(features & IFF_MULTI_QUEUE) && (RTE_PMD_TAP_MAX_QUEUES >>> 1)) { >> + RTE_LOG(DEBUG, PMD, "TUN/TAP device only one >> queue\n"); >> + goto error; >> + } else if ((features & IFF_ONE_QUEUE) && >> + (RTE_PMD_TAP_MAX_QUEUES =3D=3D 1)) { >> + ifr.ifr_flags |=3D IFF_ONE_QUEUE; >> + RTE_LOG(DEBUG, PMD, "Single queue only support\n"); >> + } else { >> + ifr.ifr_flags |=3D IFF_MULTI_QUEUE; >> + RTE_LOG(DEBUG, PMD, "Multi-queue support for %d >> queues\n", >> + RTE_PMD_TAP_MAX_QUEUES); >> + } >> #else >> - if (RTE_PMD_TAP_MAX_QUEUES > 1) { >> - RTE_LOG(DEBUG, PMD, "TUN/TAP device only one queue\n"); >> - goto error; >> - } else { >> - ifr.ifr_flags |=3D IFF_ONE_QUEUE; >> - RTE_LOG(DEBUG, PMD, "Single queue only support\n"); >> - } >> + if (RTE_PMD_TAP_MAX_QUEUES > 1) { >> + RTE_LOG(DEBUG, PMD, "TUN/TAP device only one >> queue\n"); >> + goto error; >> + } else { >> + ifr.ifr_flags |=3D IFF_ONE_QUEUE; >> + RTE_LOG(DEBUG, PMD, "Single queue only support\n"); >> + } >> #endif >>=20 >> - /* Set the TUN/TAP configuration and get the name if needed */ >> - if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) { >> - RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n", >> - ifr.ifr_name); >> - perror("TUNSETIFF"); >> - goto error; >> - } >> + /* Set the TUN/TAP configuration and get the name if needed >> */ >> + if (ioctl(fd, TUNSETIFF, (void *)&ifr) < 0) { >> + RTE_LOG(ERR, PMD, "Unable to set TUNSETIFF for %s\n", >> + ifr.ifr_name); >> + perror("TUNSETIFF"); >> + goto error; >> + } >>=20 >> - /* Always set the file descriptor to non-blocking */ >> - if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) { >> - RTE_LOG(ERR, PMD, "Unable to set to nonblocking\n"); >> - perror("F_SETFL, NONBLOCK"); >> - goto error; >> + /* Always set the file descriptor to non-blocking */ >> + if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) { >> + RTE_LOG(WARNING, PMD, "Unable to set %s to >> nonblocking\n", >> + ifr.ifr_name); >> + perror("F_SETFL, NONBLOCK"); >> + goto error; >> + } >> } >>=20 >> - /* If the name is different that new name as default */ >> - if (name && strcmp(name, ifr.ifr_name)) >> - snprintf(name, RTE_ETH_NAME_MAX_LEN - 1, "%s", ifr.ifr_name); >> - >> return fd; >>=20 >> error: >> @@ -512,7 +513,7 @@ tap_setup_queue(struct rte_eth_dev *dev, >> if (fd < 0) { >> RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid >> %d\n", pmd->name, qid); >> - fd =3D tun_alloc(pmd->name); >> + fd =3D tun_alloc(pmd->name, qid); >> if (fd < 0) { >> RTE_LOG(ERR, PMD, "tun_alloc(%s) failed\n", >> pmd->name); return -1; >> @@ -711,7 +712,7 @@ eth_dev_tap_create(const char *name, char *tap_name) >> snprintf(dev->data->name, sizeof(dev->data->name), "%s", name); >>=20 >> /* Create the first Tap device */ >> - fd =3D tun_alloc(tap_name); >> + fd =3D tun_alloc(tap_name, 0); >> if (fd < 0) { >> RTE_LOG(ERR, PMD, "tun_alloc() failed\n"); >> goto error_exit; >> @@ -739,6 +740,8 @@ eth_dev_tap_create(const char *name, char *tap_name) >> error_exit: >> RTE_PMD_DEBUG_TRACE("Unable to initialize %s\n", name); >>=20 >> + if (fd > 0) >> + close(fd); >> rte_free(data); >> rte_free(pmd); >>=20 >=20 > The patch looks good to me. >=20 > Can you maybe detail in the commit log that now queue 0 is created while > probing, while other queues are handled through tap_setup_queue() only? In my changes before your patch set I had removed creating queue 0 at probe= and only created the interface/queue at queue setup time. It would have re= quired a more changes compared to your patches and the original code. :-( >=20 > Pascal Regards, Keith