Fix Rx interrupt support in mlx4 and mlx5

PATCH In review Adrien Mazarguil 3 participants 13 messages Jun 14, 2017 – Jun 16, 2017 Network drivers

Share & tools

Adrien Mazarguil

author

2017-06-14 11:49 UTC #0 lore raw

cover Fix Rx interrupt support in mlx4 and mlx5

Several issues mainly related to error handling were found in both implementations as they share most of their Rx interrupts handling code.

Another problem with the mlx4 implementation is that it does not work properly with multiple ports adapters that share a common PCI device.

Adrien Mazarguil (7):
net/mlx4: fix typos from prior commit net/mlx4: fix Rx interrupts with multiple ports net/mlx4: fix Rx interrupts management net/mlx5: fix misplaced Rx interrupts functions net/mlx5: fix Rx interrupts support checks net/mlx5: fix return value in Rx interrupts code net/mlx5: fix Rx interrupts management

 drivers/net/mlx4/mlx4.c         | 179 ++++++++++++++++-------------------
 drivers/net/mlx4/mlx4.h         |   1 +
 drivers/net/mlx5/mlx5.c         |   2 +
 drivers/net/mlx5/mlx5_rxq.c     | 142 ++++++++++++++++++---------
 drivers/net/mlx5/mlx5_rxtx.c    |  73 --------------
 drivers/net/mlx5/mlx5_rxtx.h    |  12 +--
 drivers/net/mlx5/mlx5_trigger.c |  16 ++--
 7 files changed, 194 insertions(+), 231 deletions(-)

Signature · 1 line

2.1.4

Adrien Mazarguil

author

2017-06-14 11:49 UTC #1 lore raw

PATCH 1/7 net/mlx4: fix typos from prior commit

Fixes 9f05a4b81809 ("net/mlx4: support user space Rx interrupt event")

Signed-off-by Adrien Mazarguil <[email protected]>

Acked-by Nelio Laranjeiro <[email protected]>

Acked-by Moti Haimovsky <[email protected]>

---
 drivers/net/mlx4/mlx4.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/net/mlx4/mlx4.c +2 -2

index 5bc2a50..178562e 100644
@@ -5638,7 +5638,7 @@ priv_dev_removal_interrupt_handler_install(struct priv *priv,
  * Fill epoll fd list for rxq interrupts.
  *
  * @param priv
- *   Poinetr to private structure.
+ *   Pointer to private structure.
  *
  * @return
  *   0 on success, negative on failure.

@@ -5683,7 +5683,7 @@ priv_intr_efd_enable(struct priv *priv)
  * Clean epoll fd list for rxq interrupts.
  *
  * @param priv
- *   Ponter to private structure.
+ *   Pointer to private structure.
  */
 static void
 priv_intr_efd_disable(struct priv *priv)

Signature · 1 line

2.1.4

Adrien Mazarguil

author

2017-06-14 11:49 UTC #2 lore raw

PATCH 2/7 net/mlx4: fix Rx interrupts with multiple ports

In reply to Adrien Mazarguil

Several Ethernet device structures are allocated on top of a common PCI device for mlx4 adapters with multiple ports. These inherit a common interrupt handle from their parent PCI device, which prevents Rx interrupts from working properly on all ports as their configuration is overwritten.

Use a local interrupt handle to address this issue.

Fixes 9f05a4b81809 ("net/mlx4: support user space Rx interrupt event")

Signed-off-by Adrien Mazarguil <[email protected]>

Acked-by Moti Haimovsky <[email protected]>

---
 drivers/net/mlx4/mlx4.c | 9 +++++++++
 drivers/net/mlx4/mlx4.h | 1 +
 2 files changed, 10 insertions(+)

drivers/net/mlx4/mlx4.c +9 -0

index 178562e..2b4722f 100644
@@ -6207,6 +6207,15 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 
 		eth_dev->device->driver = &mlx4_driver.driver;
 
+		/*
+		 * Copy and override interrupt handle to prevent it from
+		 * being shared between all ethdev instances of a given PCI
+		 * device. This is required to properly handle Rx interrupts
+		 * on all ports.
+		 */
+		priv->intr_handle_dev = *eth_dev->intr_handle;
+		eth_dev->intr_handle = &priv->intr_handle_dev;
+
 		priv->dev = eth_dev;
 		eth_dev->dev_ops = &mlx4_dev_ops;

drivers/net/mlx4/mlx4.h +1 -0

index c46fc23..b74fbf8 100644
@@ -345,6 +345,7 @@ struct priv {
 	unsigned int txqs_n; /* TX queues array size. */
 	struct rxq *(*rxqs)[]; /* RX queues. */
 	struct txq *(*txqs)[]; /* TX queues. */
+	struct rte_intr_handle intr_handle_dev; /* Device interrupt handler. */
 	struct rte_intr_handle intr_handle; /* Interrupt handler. */
 	struct rte_flow_drop *flow_drop_queue; /* Flow drop queue. */
 	LIST_HEAD(mlx4_flows, rte_flow) flows;

Signature · 1 line

2.1.4

Ferruh Yigit

2017-06-16 13:07 UTC #3 lore raw

Re: [PATCH 2/7] net/mlx4: fix Rx interrupts with multiple ports

Quoted 6 lines · Adrien Mazarguil

Several Ethernet device structures are allocated on top of a common PCI device for mlx4 adapters with multiple ports. These inherit a common interrupt handle from their parent PCI device, which prevents Rx interrupts from working properly on all ports as their configuration is overwritten.
Use a local interrupt handle to address this issue.

Hi Adrien,

I am not clear why local copy required, and main concern from my point of view is if this is a common problem for all PMDs and should be addressed in higher level?

The variable is already per eth_dev, but this patch moves it the private data. What overwrites it within eth_dev?

Thanks,
ferruh

Quoted 43 lines

Fixes 9f05a4b81809 ("net/mlx4: support user space Rx interrupt event")

Signed-off-by Adrien Mazarguil <[email protected]>

Acked-by Moti Haimovsky <[email protected]>

---
 drivers/net/mlx4/mlx4.c | 9 +++++++++
 drivers/net/mlx4/mlx4.h | 1 +
 2 files changed, 10 insertions(+)

drivers/net/mlx4/mlx4.c +9 -0

index 178562e..2b4722f 100644
@@ -6207,6 +6207,15 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 
 		eth_dev->device->driver = &mlx4_driver.driver;
 
+		/*
+		 * Copy and override interrupt handle to prevent it from
+		 * being shared between all ethdev instances of a given PCI
+		 * device. This is required to properly handle Rx interrupts
+		 * on all ports.
+		 */
+		priv->intr_handle_dev = *eth_dev->intr_handle;
+		eth_dev->intr_handle = &priv->intr_handle_dev;
+
 		priv->dev = eth_dev;
 		eth_dev->dev_ops = &mlx4_dev_ops;

drivers/net/mlx4/mlx4.h +1 -0

index c46fc23..b74fbf8 100644
@@ -345,6 +345,7 @@ struct priv {
 	unsigned int txqs_n; /* TX queues array size. */
 	struct rxq *(*rxqs)[]; /* RX queues. */
 	struct txq *(*txqs)[]; /* TX queues. */
+	struct rte_intr_handle intr_handle_dev; /* Device interrupt handler. */
 	struct rte_intr_handle intr_handle; /* Interrupt handler. */
 	struct rte_flow_drop *flow_drop_queue; /* Flow drop queue. */
 	LIST_HEAD(mlx4_flows, rte_flow) flows;

Adrien Mazarguil

author

2017-06-16 13:40 UTC #4 lore raw

Re: [PATCH 2/7] net/mlx4: fix Rx interrupts with multiple ports

Quoted 6 lines · Ferruh Yigit

Quoted 5 lines · Adrien Mazarguil
Several Ethernet device structures are allocated on top of a common PCI device for mlx4 adapters with multiple ports. These inherit a common interrupt handle from their parent PCI device, which prevents Rx interrupts from working properly on all ports as their configuration is overwritten.

Quoted 7 lines

Quoted 1 line
Use a local interrupt handle to address this issue.
Hi Adrien,
I am not clear why local copy required, and main concern from my point of view is if this is a common problem for all PMDs and should be addressed in higher level?

This issue only affects PMDs that handle multiple Ethernet ports through a single PCI device. Such PMDs (like mlx4) identify themselves as PCI drivers that manually have to register multiple rte_eth_dev instances through rte_eth_dev_allocate(), which they then have to initialize.

Quoted 2 lines

The variable is already per eth_dev, but this patch moves it the private data. What overwrites it within eth_dev?

Calling rte_eth_copy_pci_info() makes the rte_eth_dev structure inherit the default interrupt handle of the underlying PCI device. By "inherit", I mean eth_dev->intr_handle points to it, in that sense it's not per eth_dev but per PCI device.

mlx4 Rx interrupts are associated with a given Verbs context, and each port has its own Verbs context, so they cannot be shared, while other PMDs using other methods for catching interrupts may be perfectly fine with a single vector associated with the PCI device. It depends on the PMD, for instance there is no such problem with mlx5 as exactly one PCI device is associated with a given port.

This patch merely allocates a specific interrupt handle associated with the eth_dev itself and makes the eth_dev handle point to that instead of the default PCI handle. This "local" handle is initialized using the PCI handle as a template before modifying the pointer. It's completely safe.

Quoted 43 lines

Quoted 42 lines

Fixes 9f05a4b81809 ("net/mlx4: support user space Rx interrupt event")

Signed-off-by Adrien Mazarguil <[email protected]>

Acked-by Moti Haimovsky <[email protected]>

---
 drivers/net/mlx4/mlx4.c | 9 +++++++++
 drivers/net/mlx4/mlx4.h | 1 +
 2 files changed, 10 insertions(+)

drivers/net/mlx4/mlx4.c +9 -0

index 178562e..2b4722f 100644
@@ -6207,6 +6207,15 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 
 		eth_dev->device->driver = &mlx4_driver.driver;
 
+		/*
+		 * Copy and override interrupt handle to prevent it from
+		 * being shared between all ethdev instances of a given PCI
+		 * device. This is required to properly handle Rx interrupts
+		 * on all ports.
+		 */
+		priv->intr_handle_dev = *eth_dev->intr_handle;
+		eth_dev->intr_handle = &priv->intr_handle_dev;
+
 		priv->dev = eth_dev;
 		eth_dev->dev_ops = &mlx4_dev_ops;

drivers/net/mlx4/mlx4.h +1 -0

index c46fc23..b74fbf8 100644
@@ -345,6 +345,7 @@ struct priv {
 	unsigned int txqs_n; /* TX queues array size. */
 	struct rxq *(*rxqs)[]; /* RX queues. */
 	struct txq *(*txqs)[]; /* TX queues. */
+	struct rte_intr_handle intr_handle_dev; /* Device interrupt handler. */
 	struct rte_intr_handle intr_handle; /* Interrupt handler. */
 	struct rte_flow_drop *flow_drop_queue; /* Flow drop queue. */
 	LIST_HEAD(mlx4_flows, rte_flow) flows;

Signature · 2 lines

Adrien Mazarguil
6WIND

Ferruh Yigit

2017-06-16 13:49 UTC #5 lore raw

Re: [PATCH 2/7] net/mlx4: fix Rx interrupts with multiple ports

Quoted 35 lines · Adrien Mazarguil

Quoted 6 lines · Ferruh Yigit
Quoted 5 lines · Adrien Mazarguil
Several Ethernet device structures are allocated on top of a common PCI device for mlx4 adapters with multiple ports. These inherit a common interrupt handle from their parent PCI device, which prevents Rx interrupts from working properly on all ports as their configuration is overwritten.
Quoted 7 lines
Quoted 1 line
Use a local interrupt handle to address this issue.
Hi Adrien,
I am not clear why local copy required, and main concern from my point of view is if this is a common problem for all PMDs and should be addressed in higher level?
This issue only affects PMDs that handle multiple Ethernet ports through a single PCI device. Such PMDs (like mlx4) identify themselves as PCI drivers that manually have to register multiple rte_eth_dev instances through rte_eth_dev_allocate(), which they then have to initialize.
Quoted 2 lines
The variable is already per eth_dev, but this patch moves it the private data. What overwrites it within eth_dev?
Calling rte_eth_copy_pci_info() makes the rte_eth_dev structure inherit the default interrupt handle of the underlying PCI device. By "inherit", I mean eth_dev->intr_handle points to it, in that sense it's not per eth_dev but per PCI device.
mlx4 Rx interrupts are associated with a given Verbs context, and each port has its own Verbs context, so they cannot be shared, while other PMDs using other methods for catching interrupts may be perfectly fine with a single vector associated with the PCI device. It depends on the PMD, for instance there is no such problem with mlx5 as exactly one PCI device is associated with a given port.

Quoted 4 lines

This patch merely allocates a specific interrupt handle associated with the eth_dev itself and makes the eth_dev handle point to that instead of the default PCI handle. This "local" handle is initialized using the PCI handle as a template before modifying the pointer. It's completely safe.

Thanks for clarification.

Quoted 45 lines

Quoted 44 lines

Quoted 43 lines

Fixes 9f05a4b81809 ("net/mlx4: support user space Rx interrupt event")

Signed-off-by Adrien Mazarguil <[email protected]>

Acked-by Moti Haimovsky <[email protected]>

---
 drivers/net/mlx4/mlx4.c | 9 +++++++++
 drivers/net/mlx4/mlx4.h | 1 +
 2 files changed, 10 insertions(+)

drivers/net/mlx4/mlx4.c +9 -0

index 178562e..2b4722f 100644
@@ -6207,6 +6207,15 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 
 		eth_dev->device->driver = &mlx4_driver.driver;
 
+		/*
+		 * Copy and override interrupt handle to prevent it from
+		 * being shared between all ethdev instances of a given PCI
+		 * device. This is required to properly handle Rx interrupts
+		 * on all ports.
+		 */
+		priv->intr_handle_dev = *eth_dev->intr_handle;
+		eth_dev->intr_handle = &priv->intr_handle_dev;
+
 		priv->dev = eth_dev;
 		eth_dev->dev_ops = &mlx4_dev_ops;

drivers/net/mlx4/mlx4.h +1 -0

index c46fc23..b74fbf8 100644
@@ -345,6 +345,7 @@ struct priv {
 	unsigned int txqs_n; /* TX queues array size. */
 	struct rxq *(*rxqs)[]; /* RX queues. */
 	struct txq *(*txqs)[]; /* TX queues. */
+	struct rte_intr_handle intr_handle_dev; /* Device interrupt handler. */
 	struct rte_intr_handle intr_handle; /* Interrupt handler. */
 	struct rte_flow_drop *flow_drop_queue; /* Flow drop queue. */
 	LIST_HEAD(mlx4_flows, rte_flow) flows;

Adrien Mazarguil

author

2017-06-14 11:49 UTC #6 lore raw

PATCH 3/7 net/mlx4: fix Rx interrupts management

In reply to Adrien Mazarguil

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 9f05a4b81809 ("net/mlx4: support user space Rx interrupt event")

Signed-off-by Adrien Mazarguil <[email protected]>

Acked-by Moti Haimovsky <[email protected]>

---
 drivers/net/mlx4/mlx4.c | 166 +++++++++++++++++++------------------------
 1 file changed, 72 insertions(+), 94 deletions(-)

drivers/net/mlx4/mlx4.c +72 ~~-94~~

index 2b4722f..050d646 100644
@@ -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));
 }

@@ -5635,7 +5627,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.

@@ -5644,108 +5636,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.

@@ -5755,22 +5728,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.

@@ -5780,20 +5755,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;
 }
 
 /**

Signature · 1 line

2.1.4

Adrien Mazarguil

author

2017-06-14 11:49 UTC #7 lore raw

PATCH 4/7 net/mlx5: fix misplaced Rx interrupts functions

In reply to Adrien Mazarguil

These functions do not belong to the data path. Their prototypes are also misplaced.

Fixes 3c7d44af252a ("net/mlx5: support user space Rx interrupt event")

Cc [email protected]

Signed-off-by Adrien Mazarguil <[email protected]>

---
 drivers/net/mlx5/mlx5_rxq.c  | 73 +++++++++++++++++++++++++++++++++++++++
 drivers/net/mlx5/mlx5_rxtx.c | 73 ---------------------------------------
 drivers/net/mlx5/mlx5_rxtx.h | 12 +++----
 3 files changed, 79 insertions(+), 79 deletions(-)

drivers/net/mlx5/mlx5_rxq.c +73 -0

index 2a26839..5421d1f 100644
@@ -1418,3 +1418,76 @@ priv_destroy_intr_vec(struct priv *priv)
 
 	rte_free(intr_handle->intr_vec);
 }
+
+/**
+ * DPDK callback for rx queue interrupt enable.
+ *
+ * @param dev
+ *   Pointer to Ethernet device structure.
+ * @param rx_queue_id
+ *   RX queue number.
+ *
+ * @return
+ *   0 on success, negative on failure.
+ */
+int
+mlx5_rx_intr_enable(struct rte_eth_dev *dev, uint16_t rx_queue_id)
+{
+#ifdef HAVE_UPDATE_CQ_CI
+	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;
+
+	ibv_mlx5_exp_update_cq_ci(cq, ci);
+	ret = ibv_req_notify_cq(cq, 0);
+#else
+	int ret = -1;
+	(void)dev;
+	(void)rx_queue_id;
+#endif
+	if (ret)
+		WARN("unable to arm interrupt on rx queue %d", rx_queue_id);
+	return ret;
+}
+
+/**
+ * DPDK callback for rx queue interrupt disable.
+ *
+ * @param dev
+ *   Pointer to Ethernet device structure.
+ * @param rx_queue_id
+ *   RX queue number.
+ *
+ * @return
+ *   0 on success, negative on failure.
+ */
+int
+mlx5_rx_intr_disable(struct rte_eth_dev *dev, uint16_t rx_queue_id)
+{
+#ifdef HAVE_UPDATE_CQ_CI
+	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;
+
+	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);
+#else
+	int ret = -1;
+	(void)dev;
+	(void)rx_queue_id;
+#endif
+	if (ret)
+		WARN("unable to disable interrupt on rx queue %d",
+		     rx_queue_id);
+	return ret;
+}

drivers/net/mlx5/mlx5_rxtx.c +0 ~~-73~~

76 lines not on this page · raw message

drivers/net/mlx5/mlx5_rxtx.h +6 -6

index 8db8eb1..cd79b5d 100644
@@ -298,10 +298,6 @@ int priv_create_hash_rxqs(struct priv *);
 void priv_destroy_hash_rxqs(struct priv *);
 int priv_allow_flow_type(struct priv *, enum hash_rxq_flow_type);
 int priv_rehash_flows(struct priv *);
-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);
 void rxq_cleanup(struct rxq_ctrl *);
 int rxq_rehash(struct rte_eth_dev *, struct rxq_ctrl *);
 int rxq_ctrl_setup(struct rte_eth_dev *, struct rxq_ctrl *, uint16_t,

@@ -311,6 +307,12 @@ 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 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);
 
 /* mlx5_txq.c */

@@ -333,8 +335,6 @@ uint16_t removed_tx_burst(void *, struct rte_mbuf **, uint16_t);
 uint16_t removed_rx_burst(void *, struct rte_mbuf **, uint16_t);
 int mlx5_rx_descriptor_status(void *, uint16_t);
 int mlx5_tx_descriptor_status(void *, uint16_t);
-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);
 
 /* mlx5_mr.c */

Signature · 1 line

2.1.4

Adrien Mazarguil

author

2017-06-14 11:49 UTC #8 lore raw

PATCH 5/7 net/mlx5: fix Rx interrupts support checks

In reply to Adrien Mazarguil

Not exposing Rx interrupts callbacks when this feature is unsupported is less intrusive than having two different versions for these functions.

Fixes 3c7d44af252a ("net/mlx5: support user space Rx interrupt event")

Cc [email protected]

Signed-off-by Adrien Mazarguil <[email protected]>

---
 drivers/net/mlx5/mlx5.c      |  2 ++
 drivers/net/mlx5/mlx5_rxq.c  | 16 ++++------------
 drivers/net/mlx5/mlx5_rxtx.h |  2 ++
 3 files changed, 8 insertions(+), 12 deletions(-)

drivers/net/mlx5/mlx5.c +2 -0

index bcb2c1b..49d4dba 100644
@@ -246,8 +246,10 @@ static const struct eth_dev_ops mlx5_dev_ops = {
 	.filter_ctrl = mlx5_dev_filter_ctrl,
 	.rx_descriptor_status = mlx5_rx_descriptor_status,
 	.tx_descriptor_status = mlx5_tx_descriptor_status,
+#ifdef HAVE_UPDATE_CQ_CI
 	.rx_queue_intr_enable = mlx5_rx_intr_enable,
 	.rx_queue_intr_disable = mlx5_rx_intr_disable,
+#endif
 };
 
 static struct {

drivers/net/mlx5/mlx5_rxq.c +4 ~~-12~~

index 5421d1f..fa350bf 100644
@@ -1419,6 +1419,8 @@ priv_destroy_intr_vec(struct priv *priv)
 	rte_free(intr_handle->intr_vec);
 }
 
+#ifdef HAVE_UPDATE_CQ_CI
+
 /**
  * DPDK callback for rx queue interrupt enable.
  *

@@ -1433,7 +1435,6 @@ priv_destroy_intr_vec(struct priv *priv)
 int
 mlx5_rx_intr_enable(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 {
-#ifdef HAVE_UPDATE_CQ_CI
 	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);

@@ -1443,11 +1444,6 @@ mlx5_rx_intr_enable(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 
 	ibv_mlx5_exp_update_cq_ci(cq, ci);
 	ret = ibv_req_notify_cq(cq, 0);
-#else
-	int ret = -1;
-	(void)dev;
-	(void)rx_queue_id;
-#endif
 	if (ret)
 		WARN("unable to arm interrupt on rx queue %d", rx_queue_id);
 	return ret;

@@ -1467,7 +1463,6 @@ 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)
 {
-#ifdef HAVE_UPDATE_CQ_CI
 	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);

@@ -1481,13 +1476,10 @@ mlx5_rx_intr_disable(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 		ret = -1;
 	else
 		ibv_ack_cq_events(cq, 1);
-#else
-	int ret = -1;
-	(void)dev;
-	(void)rx_queue_id;
-#endif
 	if (ret)
 		WARN("unable to disable interrupt on rx queue %d",
 		     rx_queue_id);
 	return ret;
 }
+
+#endif /* HAVE_UPDATE_CQ_CI */

drivers/net/mlx5/mlx5_rxtx.h +2 -0

index cd79b5d..1a3ede4 100644
@@ -311,8 +311,10 @@ 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);
+#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);
+#endif /* HAVE_UPDATE_CQ_CI */
 
 /* mlx5_txq.c */

Signature · 1 line

2.1.4

Adrien Mazarguil

author

2017-06-14 11:49 UTC #9 lore raw

PATCH 6/7 net/mlx5: fix return value in Rx interrupts code

In reply to Adrien Mazarguil

A negative return value is documented for that function in case of error.

Fixes 3c7d44af252a ("net/mlx5: support user space Rx interrupt event")

Cc [email protected]

Signed-off-by Adrien Mazarguil <[email protected]>

---
 drivers/net/mlx5/mlx5_rxq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/mlx5/mlx5_rxq.c +1 -1

index fa350bf..46d566b 100644
@@ -1446,7 +1446,7 @@ mlx5_rx_intr_enable(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 	ret = ibv_req_notify_cq(cq, 0);
 	if (ret)
 		WARN("unable to arm interrupt on rx queue %d", rx_queue_id);
-	return ret;
+	return -ret;
 }
 
 /**

Signature · 1 line

2.1.4

Adrien Mazarguil

author

2017-06-14 11:49 UTC #10 lore raw

PATCH 7/7 net/mlx5: fix Rx interrupts management

In reply to Adrien Mazarguil

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.

Remaining changes:

Fixes 3c7d44af252a ("net/mlx5: support user space Rx interrupt event")

Cc [email protected]

Signed-off-by Adrien Mazarguil <[email protected]>

---
 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(-)

drivers/net/mlx5/mlx5_rxq.c +69 ~~-86~~

index 46d566b..5aa8121 100644
@@ -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 */

drivers/net/mlx5/mlx5_rxtx.h +2 -4

index 1a3ede4..450a569 100644
@@ -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);

drivers/net/mlx5/mlx5_trigger.c +7 -9

index 8c5aa69..40f23da 100644
@@ -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);
 }

Signature · 1 line

2.1.4

Nélio Laranjeiro

Acked-by on the v1 series

2017-06-14 14:28 UTC #11 lore raw

In reply to Adrien Mazarguil

Quoted 15 lines · Adrien Mazarguil

Several issues mainly related to error handling were found in both implementations as they share most of their Rx interrupts handling code.
Another problem with the mlx4 implementation is that it does not work properly with multiple ports adapters that share a common PCI device.
Adrien Mazarguil (7):
net/mlx4: fix typos from prior commit net/mlx4: fix Rx interrupts with multiple ports net/mlx4: fix Rx interrupts management net/mlx5: fix misplaced Rx interrupts functions net/mlx5: fix Rx interrupts support checks net/mlx5: fix return value in Rx interrupts code net/mlx5: fix Rx interrupts management

Quoted 11 lines

 drivers/net/mlx4/mlx4.c         | 179 ++++++++++++++++-------------------
 drivers/net/mlx4/mlx4.h         |   1 +
 drivers/net/mlx5/mlx5.c         |   2 +
 drivers/net/mlx5/mlx5_rxq.c     | 142 ++++++++++++++++++---------
 drivers/net/mlx5/mlx5_rxtx.c    |  73 --------------
 drivers/net/mlx5/mlx5_rxtx.h    |  12 +--
 drivers/net/mlx5/mlx5_trigger.c |  16 ++--
 7 files changed, 194 insertions(+), 231 deletions(-)

Signature · 1 line

2.1.4

For the series:

Acked-by Nelio Laranjeiro <[email protected]>

Signature · 2 lines

Nélio Laranjeiro
6WIND

Ferruh Yigit

2017-06-16 13:51 UTC #12 lore raw

In reply to Adrien Mazarguil

Quoted 15 lines · Adrien Mazarguil

Several issues mainly related to error handling were found in both implementations as they share most of their Rx interrupts handling code.
Another problem with the mlx4 implementation is that it does not work properly with multiple ports adapters that share a common PCI device.
Adrien Mazarguil (7):
net/mlx4: fix typos from prior commit net/mlx4: fix Rx interrupts with multiple ports net/mlx4: fix Rx interrupts management net/mlx5: fix misplaced Rx interrupts functions net/mlx5: fix Rx interrupts support checks net/mlx5: fix return value in Rx interrupts code net/mlx5: fix Rx interrupts management

Quoted 8 lines

 drivers/net/mlx4/mlx4.c         | 179 ++++++++++++++++-------------------
 drivers/net/mlx4/mlx4.h         |   1 +
 drivers/net/mlx5/mlx5.c         |   2 +
 drivers/net/mlx5/mlx5_rxq.c     | 142 ++++++++++++++++++---------
 drivers/net/mlx5/mlx5_rxtx.c    |  73 --------------
 drivers/net/mlx5/mlx5_rxtx.h    |  12 +--
 drivers/net/mlx5/mlx5_trigger.c |  16 ++--
 7 files changed, 194 insertions(+), 231 deletions(-)

Series applied to dpdk-next-net/master, thanks.