net/mlx4: fix Rx interrupts management
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_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_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_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: 9f05a4b818
("net/mlx4: support user space Rx interrupt event")
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Moti Haimovsky <motih@mellanox.com>
This commit is contained in:
parent
3c560ec3ea
commit
0a2ae70319
@ -135,16 +135,10 @@ static int
|
||||
mlx4_rx_intr_disable(struct rte_eth_dev *dev, uint16_t idx);
|
||||
|
||||
static int
|
||||
priv_intr_efd_enable(struct priv *priv);
|
||||
priv_rx_intr_vec_enable(struct priv *priv);
|
||||
|
||||
static void
|
||||
priv_intr_efd_disable(struct priv *priv);
|
||||
|
||||
static int
|
||||
priv_create_intr_vec(struct priv *priv);
|
||||
|
||||
static void
|
||||
priv_destroy_intr_vec(struct priv *priv);
|
||||
priv_rx_intr_vec_disable(struct priv *priv);
|
||||
|
||||
/**
|
||||
* Check if running as a secondary process.
|
||||
@ -3720,9 +3714,9 @@ rxq_setup(struct rte_eth_dev *dev, struct rxq *rxq, uint16_t desc,
|
||||
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",
|
||||
ERROR("%p: Rx interrupt completion channel creation"
|
||||
" failure: %s",
|
||||
(void *)dev, strerror(ret));
|
||||
goto error;
|
||||
}
|
||||
@ -4037,10 +4031,11 @@ mlx4_dev_start(struct rte_eth_dev *dev)
|
||||
(void *)dev);
|
||||
goto err;
|
||||
}
|
||||
if (dev->data->dev_conf.intr_conf.rxq) {
|
||||
ret = priv_intr_efd_enable(priv);
|
||||
if (!ret)
|
||||
ret = priv_create_intr_vec(priv);
|
||||
ret = priv_rx_intr_vec_enable(priv);
|
||||
if (ret) {
|
||||
ERROR("%p: Rx interrupt vector creation failed",
|
||||
(void *)dev);
|
||||
goto err;
|
||||
}
|
||||
ret = mlx4_priv_flow_start(priv);
|
||||
if (ret) {
|
||||
@ -4234,10 +4229,7 @@ mlx4_dev_close(struct rte_eth_dev *dev)
|
||||
assert(priv->ctx == NULL);
|
||||
priv_dev_removal_interrupt_handler_uninstall(priv, dev);
|
||||
priv_dev_link_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_rx_intr_vec_disable(priv);
|
||||
priv_unlock(priv);
|
||||
memset(priv, 0, sizeof(*priv));
|
||||
}
|
||||
@ -5637,7 +5629,7 @@ priv_dev_removal_interrupt_handler_install(struct priv *priv,
|
||||
}
|
||||
|
||||
/**
|
||||
* Fill epoll fd list for rxq interrupts.
|
||||
* Allocate queue vector and fill epoll fd list for Rx interrupts.
|
||||
*
|
||||
* @param priv
|
||||
* Pointer to private structure.
|
||||
@ -5646,108 +5638,89 @@ priv_dev_removal_interrupt_handler_install(struct priv *priv,
|
||||
* 0 on success, negative on failure.
|
||||
*/
|
||||
static 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];
|
||||
int fd = rxq->channel->fd;
|
||||
int fd;
|
||||
int flags;
|
||||
int rc;
|
||||
|
||||
/* Skip queues that cannot request interrupts. */
|
||||
if (!rxq || !rxq->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->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 rc;
|
||||
}
|
||||
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
|
||||
* Pointer to private structure.
|
||||
*/
|
||||
static 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);
|
||||
free(intr_handle->intr_vec);
|
||||
intr_handle->nb_efd = 0;
|
||||
intr_handle->intr_vec = NULL;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create and init interrupt vector array.
|
||||
*
|
||||
* @param priv
|
||||
* Pointer to private structure.
|
||||
*
|
||||
* @return
|
||||
* 0 on success, negative on failure.
|
||||
*/
|
||||
static 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
|
||||
* Pointer to private structure.
|
||||
*/
|
||||
static void
|
||||
priv_destroy_intr_vec(struct priv *priv)
|
||||
{
|
||||
struct rte_intr_handle *intr_handle = priv->dev->intr_handle;
|
||||
|
||||
rte_free(intr_handle->intr_vec);
|
||||
}
|
||||
|
||||
/**
|
||||
* DPDK callback for rx queue interrupt enable.
|
||||
* DPDK callback for Rx queue interrupt enable.
|
||||
*
|
||||
* @param dev
|
||||
* Pointer to Ethernet device structure.
|
||||
* @param idx
|
||||
* RX queue index.
|
||||
* Rx queue index.
|
||||
*
|
||||
* @return
|
||||
* 0 on success, negative on failure.
|
||||
@ -5757,22 +5730,24 @@ mlx4_rx_intr_enable(struct rte_eth_dev *dev, uint16_t idx)
|
||||
{
|
||||
struct priv *priv = dev->data->dev_private;
|
||||
struct rxq *rxq = (*priv->rxqs)[idx];
|
||||
struct ibv_cq *cq = rxq->cq;
|
||||
int ret = 0;
|
||||
int ret;
|
||||
|
||||
ret = ibv_req_notify_cq(cq, 0);
|
||||
if (!rxq || !rxq->channel)
|
||||
ret = EINVAL;
|
||||
else
|
||||
ret = ibv_req_notify_cq(rxq->cq, 0);
|
||||
if (ret)
|
||||
WARN("unable to arm interrupt on rx queue %d", idx);
|
||||
return -ret;
|
||||
}
|
||||
|
||||
/**
|
||||
* DPDK callback for rx queue interrupt disable.
|
||||
* DPDK callback for Rx queue interrupt disable.
|
||||
*
|
||||
* @param dev
|
||||
* Pointer to Ethernet device structure.
|
||||
* @param idx
|
||||
* RX queue index.
|
||||
* Rx queue index.
|
||||
*
|
||||
* @return
|
||||
* 0 on success, negative on failure.
|
||||
@ -5782,20 +5757,23 @@ mlx4_rx_intr_disable(struct rte_eth_dev *dev, uint16_t idx)
|
||||
{
|
||||
struct priv *priv = dev->data->dev_private;
|
||||
struct rxq *rxq = (*priv->rxqs)[idx];
|
||||
struct ibv_cq *cq = rxq->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->channel) {
|
||||
ret = EINVAL;
|
||||
} else {
|
||||
ret = ibv_get_cq_event(rxq->cq->channel, &ev_cq, &ev_ctx);
|
||||
if (ret || ev_cq != rxq->cq)
|
||||
ret = EINVAL;
|
||||
}
|
||||
if (ret)
|
||||
WARN("unable to disable interrupt on rx queue %d",
|
||||
idx);
|
||||
return ret;
|
||||
else
|
||||
ibv_ack_cq_events(rxq->cq, 1);
|
||||
return -ret;
|
||||
}
|
||||
|
||||
/**
|
||||
|
Loading…
Reference in New Issue
Block a user