[dpdk-dev] net/tap: add Rx interrupts
Checks
Commit Message
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 <motih@mellanox.com>
---
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
Comments
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 <motih@mellanox.com>
> ---
> 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 <assert.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <signal.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +#include <rte_eth_tap.h>
> +#include <rte_errno.h>
> +#include <rte_interrupts.h>
> +
> +
> +/**
> + * 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;
> +}
@@ -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
@@ -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
@@ -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++) {
@@ -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_ */
new file mode 100644
@@ -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 <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <rte_eth_tap.h>
+#include <rte_errno.h>
+#include <rte_interrupts.h>
+
+
+/**
+ * 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 (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;
+}