From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: dev@dpdk.org
Cc: stable@dpdk.org
Subject: [dpdk-stable] [PATCH 7/7] net/mlx5: fix Rx interrupts management
Date: Wed, 14 Jun 2017 13:49:17 +0200 [thread overview]
Message-ID: <f29447cd7e2b6f8cbf87d2712fc2caf435d3550d.1497439616.git.adrien.mazarguil@6wind.com> (raw)
In-Reply-To: <cover.1497439616.git.adrien.mazarguil@6wind.com>
This commit addresses various issues that may lead to undefined behavior
when configuring Rx interrupts.
While failure to create a Rx queue completion channel in rxq_ctrl_setup()
prevents that queue from being created, existing queues still have theirs.
Since the error handler disables dev_conf.intr_conf.rxq as well, subsequent
calls to rxq_ctrl_setup() create Rx queues without interrupts. This leads
to a scenario where not all Rx queues support interrupts; missing checks on
the presence of completion channels may crash the application.
Considering that the PMD is not supposed to disable user-provided
configuration parameters (dev_conf.intr_conf.rxq), and that these can
change for subsequent rxq_ctrl_setup() calls anyway, properly supporting a
mixed mode where not all Rx queues have interrupts enabled is a better
approach.
To do so with a minimum set of changes, priv_intr_efd_enable() and
priv_create_intr_vec() are first refactored as a single
priv_rx_intr_vec_enable() function (same for their "disable" counterparts).
Since they had to be used together, there was no point in keeping them
separate.
Remaining changes:
- Always clean up before reconfiguring interrupts to avoid memory leaks.
- Always clean up when closing the device.
- Use malloc()/free() instead of their rte_*() counterparts since there is
no need to store the vector in huge pages-backed memory.
- Allow more Rx queues than the size of the event file descriptor array as
long as Rx interrupts are not requested on all of them.
- Properly clean up interrupt handle when disabling Rx interrupts (nb_efd
and intr_vec reset to 0).
- Check completion channel presence while toggling Rx interrupts on a given
queue.
Fixes: 3c7d44af252a ("net/mlx5: support user space Rx interrupt event")
Cc: stable@dpdk.org
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
drivers/net/mlx5/mlx5_rxq.c | 155 ++++++++++++++++-------------------
drivers/net/mlx5/mlx5_rxtx.h | 6 +-
drivers/net/mlx5/mlx5_trigger.c | 16 ++--
3 files changed, 78 insertions(+), 99 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index 46d566b..5aa8121 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -978,10 +978,10 @@ rxq_ctrl_setup(struct rte_eth_dev *dev, struct rxq_ctrl *rxq_ctrl,
if (dev->data->dev_conf.intr_conf.rxq) {
tmpl.channel = ibv_create_comp_channel(priv->ctx);
if (tmpl.channel == NULL) {
- dev->data->dev_conf.intr_conf.rxq = 0;
ret = ENOMEM;
- ERROR("%p: Comp Channel creation failure: %s",
- (void *)dev, strerror(ret));
+ ERROR("%p: Rx interrupt completion channel creation"
+ " failure: %s",
+ (void *)dev, strerror(ret));
goto error;
}
}
@@ -1310,124 +1310,102 @@ mlx5_rx_burst_secondary_setup(void *dpdk_rxq, struct rte_mbuf **pkts,
}
/**
- * Fill epoll fd list for rxq interrupts.
+ * Allocate queue vector and fill epoll fd list for Rx interrupts.
*
* @param priv
- * Private structure.
+ * Pointer to private structure.
*
* @return
* 0 on success, negative on failure.
*/
int
-priv_intr_efd_enable(struct priv *priv)
+priv_rx_intr_vec_enable(struct priv *priv)
{
unsigned int i;
unsigned int rxqs_n = priv->rxqs_n;
unsigned int n = RTE_MIN(rxqs_n, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
+ unsigned int count = 0;
struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
- if (n == 0)
+ if (!priv->dev->data->dev_conf.intr_conf.rxq)
return 0;
- if (n < rxqs_n) {
- WARN("rxqs num is larger than EAL max interrupt vector "
- "%u > %u unable to supprt rxq interrupts",
- rxqs_n, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
- return -EINVAL;
+ priv_rx_intr_vec_disable(priv);
+ intr_handle->intr_vec = malloc(sizeof(intr_handle->intr_vec[rxqs_n]));
+ if (intr_handle->intr_vec == NULL) {
+ ERROR("failed to allocate memory for interrupt vector,"
+ " Rx interrupts will not be supported");
+ return -ENOMEM;
}
intr_handle->type = RTE_INTR_HANDLE_EXT;
for (i = 0; i != n; ++i) {
struct rxq *rxq = (*priv->rxqs)[i];
struct rxq_ctrl *rxq_ctrl =
container_of(rxq, struct rxq_ctrl, rxq);
- int fd = rxq_ctrl->channel->fd;
+ int fd;
int flags;
int rc;
+ /* Skip queues that cannot request interrupts. */
+ if (!rxq || !rxq_ctrl->channel) {
+ /* 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) {
+ ERROR("too many Rx queues for interrupt vector size"
+ " (%d), Rx interrupts cannot be enabled",
+ RTE_MAX_RXTX_INTR_VEC_ID);
+ priv_rx_intr_vec_disable(priv);
+ return -1;
+ }
+ fd = rxq_ctrl->channel->fd;
flags = fcntl(fd, F_GETFL);
rc = fcntl(fd, F_SETFL, flags | O_NONBLOCK);
if (rc < 0) {
- WARN("failed to change rxq interrupt file "
- "descriptor %d for queue index %d", fd, i);
+ ERROR("failed to make Rx interrupt file descriptor"
+ " %d non-blocking for queue index %d", fd, i);
+ priv_rx_intr_vec_disable(priv);
return -1;
}
- intr_handle->efds[i] = fd;
+ intr_handle->intr_vec[i] = RTE_INTR_VEC_RXTX_OFFSET + count;
+ intr_handle->efds[count] = fd;
+ count++;
}
- intr_handle->nb_efd = n;
+ if (!count)
+ priv_rx_intr_vec_disable(priv);
+ else
+ intr_handle->nb_efd = count;
return 0;
}
/**
- * Clean epoll fd list for rxq interrupts.
+ * Clean up Rx interrupts handler.
*
* @param priv
- * Private structure.
+ * Pointer to private structure.
*/
void
-priv_intr_efd_disable(struct priv *priv)
+priv_rx_intr_vec_disable(struct priv *priv)
{
struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
rte_intr_free_epoll_fd(intr_handle);
-}
-
-/**
- * Create and init interrupt vector array.
- *
- * @param priv
- * Private structure.
- *
- * @return
- * 0 on success, negative on failure.
- */
-int
-priv_create_intr_vec(struct priv *priv)
-{
- unsigned int rxqs_n = priv->rxqs_n;
- unsigned int i;
- struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
-
- if (rxqs_n == 0)
- return 0;
- intr_handle->intr_vec = (int *)
- rte_malloc("intr_vec", rxqs_n * sizeof(int), 0);
- if (intr_handle->intr_vec == NULL) {
- WARN("Failed to allocate memory for intr_vec "
- "rxq interrupt will not be supported");
- return -ENOMEM;
- }
- for (i = 0; i != rxqs_n; ++i) {
- /* 1:1 mapping between rxq and interrupt. */
- intr_handle->intr_vec[i] = RTE_INTR_VEC_RXTX_OFFSET + i;
- }
- return 0;
-}
-
-/**
- * Destroy init interrupt vector array.
- *
- * @param priv
- * Private structure.
- *
- * @return
- * 0 on success, negative on failure.
- */
-void
-priv_destroy_intr_vec(struct priv *priv)
-{
- struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
-
- rte_free(intr_handle->intr_vec);
+ free(intr_handle->intr_vec);
+ intr_handle->nb_efd = 0;
+ intr_handle->intr_vec = NULL;
}
#ifdef HAVE_UPDATE_CQ_CI
/**
- * DPDK callback for rx queue interrupt enable.
+ * DPDK callback for Rx queue interrupt enable.
*
* @param dev
* Pointer to Ethernet device structure.
* @param rx_queue_id
- * RX queue number.
+ * Rx queue number.
*
* @return
* 0 on success, negative on failure.
@@ -1438,24 +1416,26 @@ mlx5_rx_intr_enable(struct rte_eth_dev *dev, uint16_t rx_queue_id)
struct priv *priv = mlx5_get_priv(dev);
struct rxq *rxq = (*priv->rxqs)[rx_queue_id];
struct rxq_ctrl *rxq_ctrl = container_of(rxq, struct rxq_ctrl, rxq);
- struct ibv_cq *cq = rxq_ctrl->cq;
- uint16_t ci = rxq->cq_ci;
- int ret = 0;
+ int ret;
- ibv_mlx5_exp_update_cq_ci(cq, ci);
- ret = ibv_req_notify_cq(cq, 0);
+ if (!rxq || !rxq_ctrl->channel) {
+ ret = EINVAL;
+ } else {
+ ibv_mlx5_exp_update_cq_ci(rxq_ctrl->cq, rxq->cq_ci);
+ ret = ibv_req_notify_cq(rxq_ctrl->cq, 0);
+ }
if (ret)
WARN("unable to arm interrupt on rx queue %d", rx_queue_id);
return -ret;
}
/**
- * DPDK callback for rx queue interrupt disable.
+ * DPDK callback for Rx queue interrupt disable.
*
* @param dev
* Pointer to Ethernet device structure.
* @param rx_queue_id
- * RX queue number.
+ * Rx queue number.
*
* @return
* 0 on success, negative on failure.
@@ -1466,20 +1446,23 @@ mlx5_rx_intr_disable(struct rte_eth_dev *dev, uint16_t rx_queue_id)
struct priv *priv = mlx5_get_priv(dev);
struct rxq *rxq = (*priv->rxqs)[rx_queue_id];
struct rxq_ctrl *rxq_ctrl = container_of(rxq, struct rxq_ctrl, rxq);
- struct ibv_cq *cq = rxq_ctrl->cq;
struct ibv_cq *ev_cq;
void *ev_ctx;
- int ret = 0;
+ int ret;
- ret = ibv_get_cq_event(cq->channel, &ev_cq, &ev_ctx);
- if (ret || ev_cq != cq)
- ret = -1;
- else
- ibv_ack_cq_events(cq, 1);
+ if (!rxq || !rxq_ctrl->channel) {
+ ret = EINVAL;
+ } else {
+ ret = ibv_get_cq_event(rxq_ctrl->cq->channel, &ev_cq, &ev_ctx);
+ if (ret || ev_cq != rxq_ctrl->cq)
+ ret = EINVAL;
+ }
if (ret)
WARN("unable to disable interrupt on rx queue %d",
rx_queue_id);
- return ret;
+ else
+ ibv_ack_cq_events(rxq_ctrl->cq, 1);
+ return -ret;
}
#endif /* HAVE_UPDATE_CQ_CI */
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 1a3ede4..450a569 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -307,10 +307,8 @@ int mlx5_rx_queue_setup(struct rte_eth_dev *, uint16_t, uint16_t, unsigned int,
const struct rte_eth_rxconf *, struct rte_mempool *);
void mlx5_rx_queue_release(void *);
uint16_t mlx5_rx_burst_secondary_setup(void *, struct rte_mbuf **, uint16_t);
-int priv_intr_efd_enable(struct priv *priv);
-void priv_intr_efd_disable(struct priv *priv);
-int priv_create_intr_vec(struct priv *priv);
-void priv_destroy_intr_vec(struct priv *priv);
+int priv_rx_intr_vec_enable(struct priv *priv);
+void priv_rx_intr_vec_disable(struct priv *priv);
#ifdef HAVE_UPDATE_CQ_CI
int mlx5_rx_intr_enable(struct rte_eth_dev *dev, uint16_t rx_queue_id);
int mlx5_rx_intr_disable(struct rte_eth_dev *dev, uint16_t rx_queue_id);
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 8c5aa69..40f23da 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -94,12 +94,13 @@ mlx5_dev_start(struct rte_eth_dev *dev)
(void *)priv, strerror(err));
goto error;
}
- priv_dev_interrupt_handler_install(priv, dev);
- if (dev->data->dev_conf.intr_conf.rxq) {
- err = priv_intr_efd_enable(priv);
- if (!err)
- err = priv_create_intr_vec(priv);
+ err = priv_rx_intr_vec_enable(priv);
+ if (err) {
+ ERROR("%p: RX interrupt vector creation failed",
+ (void *)priv);
+ goto error;
}
+ priv_dev_interrupt_handler_install(priv, dev);
priv_xstats_init(priv);
priv_unlock(priv);
return 0;
@@ -140,11 +141,8 @@ mlx5_dev_stop(struct rte_eth_dev *dev)
priv_destroy_hash_rxqs(priv);
priv_fdir_disable(priv);
priv_flow_stop(priv);
+ priv_rx_intr_vec_disable(priv);
priv_dev_interrupt_handler_uninstall(priv, dev);
- if (priv->dev->data->dev_conf.intr_conf.rxq) {
- priv_destroy_intr_vec(priv);
- priv_intr_efd_disable(priv);
- }
priv->started = 0;
priv_unlock(priv);
}
--
2.1.4
prev parent reply other threads:[~2017-06-14 11:49 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1497439616.git.adrien.mazarguil@6wind.com>
2017-06-14 11:49 ` [dpdk-stable] [PATCH 4/7] net/mlx5: fix misplaced Rx interrupts functions Adrien Mazarguil
2017-06-14 11:49 ` [dpdk-stable] [PATCH 5/7] net/mlx5: fix Rx interrupts support checks Adrien Mazarguil
2017-06-14 11:49 ` [dpdk-stable] [PATCH 6/7] net/mlx5: fix return value in Rx interrupts code Adrien Mazarguil
2017-06-14 11:49 ` Adrien Mazarguil [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f29447cd7e2b6f8cbf87d2712fc2caf435d3550d.1497439616.git.adrien.mazarguil@6wind.com \
--to=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--cc=stable@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).