From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by dpdk.org (Postfix) with ESMTP id 4DE7E1D8A for ; Wed, 22 Nov 2017 09:23:57 +0100 (CET) Received: by mail-wm0-f66.google.com with SMTP id y80so8316326wmd.0 for ; Wed, 22 Nov 2017 00:23:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=UxR+72gt+PEyr2jINoDBGEZ48JXZp7vfWQAU1xMVaYw=; b=N0k7cwMbtizwCsN8L7gqlyapsTLfOuFWP6WS7bu2lcTEFEufGqckrXNUThcN6qiWLo 9eo75c2Xq96pQ2P2fhd82dlSmlIlmyMsCxC5yC2cZ+WINNzjBgluHwKO2ZY2PcL2810K DTLEW19+ND9p13eFu8tFZHO40bQPIYMcnTOYoDJyzwWX/ap6XkPJVNqptIW8kwh+C+A7 YuSfdQkYq2nve2SM9h82IMK6j23xbEIX7YrKKjSvdKW4qhte8kFzuXWn8E+fmu1aX+f+ NgXI6KZ5As+7s+KyIr9/4M6H08cHZKpxi71Ji7wL2E2q33IGFhAtT733Z46xUVQrbZv2 dKJA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=UxR+72gt+PEyr2jINoDBGEZ48JXZp7vfWQAU1xMVaYw=; b=QVlHyV26d9RSS02YyqjRe6bSgOcTLwBAzJ4dcqCdcny/2umlVve7BFmNHiglhFQueI fra6z2kWoquWwnI03quDefYHMNyqfADhoYzONXxa9IW1w/5rdJM4Udyx5F90f7Au1Twi NN7T4KkKXG6/e1INuiJ7YbiqtckqLFq5RSYMCrthOth0lXCc72PqMA/RSjSHtiLEv1bH By/gXLzMLLNNRJY7VlyFfchAATLyphSAUEWHDdWG3RRR3PnuS13XYPtvICDClJvIeuJE Nu9J+NBphJooRkgrb2sJ5tC+q7HtT8hlEUWqu4a9ZavLWP2rRn+IJixUBKcfjsYj00jJ QONw== X-Gm-Message-State: AJaThX5Co5YWYewo0hJqfJZFtth+ltTYtxP3ak7u7PlCCkosYBoWZp0k fbyHp0IAVmxlqzKv1FT9SE8ydMaBZF8= X-Google-Smtp-Source: AGs4zMZYL7IF1cpOAPdbh1DtGzQIwBIkldqr2znwvnEoFS1ubsBckF9mmp8i3Cj/2bWrQQfzm0/Egg== X-Received: by 10.80.186.193 with SMTP id x59mr28989930ede.256.1511339036582; Wed, 22 Nov 2017 00:23:56 -0800 (PST) Received: from [192.168.1.58] (82.107.69.91.rev.sfr.net. [91.69.107.82]) by smtp.gmail.com with ESMTPSA id y40sm10564326ede.0.2017.11.22.00.23.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 Nov 2017 00:23:55 -0800 (PST) To: Moti Haimovsky Cc: dev@dpdk.org References: <1511186362-72289-1-git-send-email-motih@mellanox.com> From: Pascal Mazon Message-ID: Date: Wed, 22 Nov 2017 09:23:55 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1511186362-72289-1-git-send-email-motih@mellanox.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH] net/tap: add Rx interrupts 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, 22 Nov 2017 08:23:57 -0000 Hi, The patch looks globally fine to me. I'll inline just one comment. Best regards, Pascal On 20/11/2017 14:59, Moti Haimovsky wrote: > This patch adds support for registering and waiting for Rx interrupts. > This allows applications to wait for Rx events from the PMD using the > DPDK rte_epoll subsystem. > > Signed-off-by: Moti Haimovsky > --- > doc/guides/nics/features/tap.ini | 1 + > drivers/net/tap/Makefile | 1 + > drivers/net/tap/rte_eth_tap.c | 17 ++++- > drivers/net/tap/rte_eth_tap.h | 4 ++ > drivers/net/tap/tap_intr.c | 147 +++++++++++++++++++++++++++++++++++++++ > 5 files changed, 169 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/tap/tap_intr.c > > diff --git a/doc/guides/nics/features/tap.ini b/doc/guides/nics/features/tap.ini > index f0e893d..8bc5187 100644 > --- a/doc/guides/nics/features/tap.ini > +++ b/doc/guides/nics/features/tap.ini > @@ -7,6 +7,7 @@ > Speed capabilities = P > Link status = Y > Link status event = Y > +Rx interrupt = Y > Jumbo frame = Y > Promiscuous mode = Y > Allmulticast mode = Y > diff --git a/drivers/net/tap/Makefile b/drivers/net/tap/Makefile > index 405b49e..6636b77 100644 > --- a/drivers/net/tap/Makefile > +++ b/drivers/net/tap/Makefile > @@ -54,6 +54,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += rte_eth_tap.c > SRCS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += tap_flow.c > SRCS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += tap_netlink.c > SRCS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += tap_tcmsgs.c > +SRCS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += tap_intr.c > > include $(RTE_SDK)/mk/rte.lib.mk > > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c > index 6b27679..7e6de68 100644 > --- a/drivers/net/tap/rte_eth_tap.c > +++ b/drivers/net/tap/rte_eth_tap.c > @@ -1127,7 +1127,7 @@ enum ioctl_mode { > } > > static int > -tap_intr_handle_set(struct rte_eth_dev *dev, int set) > +tap_lsc_intr_handle_set(struct rte_eth_dev *dev, int set) > { > struct pmd_internals *pmd = dev->data->dev_private; > > @@ -1152,6 +1152,20 @@ enum ioctl_mode { > tap_dev_intr_handler, dev); > } > > +static int > +tap_intr_handle_set(struct rte_eth_dev *dev, int set) > +{ > + int err; > + > + err = tap_lsc_intr_handle_set(dev, set); > + if (err) > + return err; > + err = tap_rx_intr_vec_set(dev, set); > + if (err && set) > + tap_lsc_intr_handle_set(dev, 0); > + return err; > +} > + > static const uint32_t* > tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused) > { > @@ -1284,6 +1298,7 @@ enum ioctl_mode { > > pmd->intr_handle.type = RTE_INTR_HANDLE_EXT; > pmd->intr_handle.fd = -1; > + dev->intr_handle = &pmd->intr_handle; > > /* Presetup the fds to -1 as being not valid */ > for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) { > diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h > index 829f32f..fccc4b8 100644 > --- a/drivers/net/tap/rte_eth_tap.h > +++ b/drivers/net/tap/rte_eth_tap.h > @@ -98,4 +98,8 @@ struct pmd_internals { > struct rte_intr_handle intr_handle; /* LSC interrupt handle. */ > }; > > +/* tap_intr.c */ > + > +int tap_rx_intr_vec_set(struct rte_eth_dev *dev, int set); > + > #endif /* _RTE_ETH_TAP_H_ */ > diff --git a/drivers/net/tap/tap_intr.c b/drivers/net/tap/tap_intr.c > new file mode 100644 > index 0000000..ef25ca6 > --- /dev/null > +++ b/drivers/net/tap/tap_intr.c > @@ -0,0 +1,147 @@ > +/*- > + * BSD LICENSE > + * > + * Copyright 2017 Mellanox > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * > + * * Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * * Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in > + * the documentation and/or other materials provided with the > + * distribution. > + * * Neither the name of the copyright holder nor the names of its > + * contributors may be used to endorse or promote products derived > + * from this software without specific prior written permission. > + * > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + */ > + > +/** > + * @file > + * Interrupts handling for tap driver. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > + > +/** > + * Unregister Rx interrupts free the queue interrupt vector. > + * > + * @param dev > + * Pointer to the tap rte_eth_dev structure. > + */ > +static void > +tap_rx_intr_vec_uninstall(struct rte_eth_dev *dev) > +{ > + struct pmd_internals *pmd = dev->data->dev_private; > + struct rte_intr_handle *intr_handle = &pmd->intr_handle; > + > + rte_intr_free_epoll_fd(intr_handle); > + free(intr_handle->intr_vec); > + intr_handle->intr_vec = NULL; > + intr_handle->nb_efd = 0; > +} > + > +/** > + * Allocate Rx queue interrupt vector and register Rx interrupts. > + * > + * @param dev > + * Pointer to the tap rte_eth_dev device structure. > + * > + * @return > + * 0 on success, negative errno value otherwise and rte_errno is set. > + */ > +static int > +tap_rx_intr_vec_install(struct rte_eth_dev *dev) > +{ > + struct pmd_internals *pmd = dev->data->dev_private; > + unsigned int rxqs_n = pmd->dev->data->nb_rx_queues; > + struct rte_intr_handle *intr_handle = &pmd->intr_handle; > + unsigned int n = RTE_MIN(rxqs_n, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID); > + unsigned int i; > + unsigned int count = 0; > + > + if (!dev->data->dev_conf.intr_conf.rxq) > + return 0; > + intr_handle->intr_vec = malloc(sizeof(intr_handle->intr_vec[rxqs_n])); > + if (intr_handle->intr_vec == NULL) { > + rte_errno = ENOMEM; > + RTE_LOG(ERR, PMD, > + "failed to allocate memory for interrupt vector," > + " Rx interrupts will not be supported"); > + return -rte_errno; > + } > + for (i = 0; i < n; i++) { > + struct rx_queue *rxq = pmd->dev->data->rx_queues[i]; > + > + /* Skip queues that cannot request interrupts. */ > + if (!rxq || (rxq->fd <= 0)) { > + /* Use invalid intr_vec[] index to disable entry. */ > + intr_handle->intr_vec[i] = > + RTE_INTR_VEC_RXTX_OFFSET + > + RTE_MAX_RXTX_INTR_VEC_ID; > + continue; > If I'm not mistaken, the following if (count >=...) will never be reached. count starts from 0, and can only be increased at most n - 1 times. n being min(rxqsg, RTE_MAX_RXTX_INTR_VEC_ID), it won't exceed the max. I would remove that little condition. > + if (count >= RTE_MAX_RXTX_INTR_VEC_ID) { > + rte_errno = E2BIG; > + RTE_LOG(ERR, PMD, > + "too many Rx queues for interrupt vector size" > + " (%d), Rx interrupts cannot be enabled", > + RTE_MAX_RXTX_INTR_VEC_ID); > + tap_rx_intr_vec_uninstall(dev); > + return -rte_errno; > + } > + intr_handle->intr_vec[i] = RTE_INTR_VEC_RXTX_OFFSET + count; > + intr_handle->efds[count] = rxq->fd; > + count++; > + } > + if (!count) > + tap_rx_intr_vec_uninstall(dev); > + else > + intr_handle->nb_efd = count; > + return 0; > +} > + > +/** > + * Register or unregister the Rx interrupts. > + * > + * @param dev > + * Pointer to the tap rte_eth_dev device structure. > + * @param set > + * should the operation be register or unregister the interrupts. > + * > + * @return > + * 0 on success, negative errno value otherwise and rte_errno is set. > + */ > +int > +tap_rx_intr_vec_set(struct rte_eth_dev *dev, int set) > +{ > + tap_rx_intr_vec_uninstall(dev); > + if (set) > + return tap_rx_intr_vec_install(dev); > + return 0; > +}