* [dpdk-dev] [PATCH 1/7] net/mlx4: fix typos from prior commit
2017-06-14 11:49 [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Adrien Mazarguil
@ 2017-06-14 11:49 ` Adrien Mazarguil
2017-06-14 11:49 ` [dpdk-dev] [PATCH 2/7] net/mlx4: fix Rx interrupts with multiple ports Adrien Mazarguil
` (7 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-14 11:49 UTC (permalink / raw)
To: dev
Fixes: 9f05a4b81809 ("net/mlx4: support user space Rx interrupt event")
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Moti Haimovsky <motih@mellanox.com>
---
drivers/net/mlx4/mlx4.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 5bc2a50..178562e 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -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)
--
2.1.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH 2/7] net/mlx4: fix Rx interrupts with multiple ports
2017-06-14 11:49 [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Adrien Mazarguil
2017-06-14 11:49 ` [dpdk-dev] [PATCH 1/7] net/mlx4: fix typos from prior commit Adrien Mazarguil
@ 2017-06-14 11:49 ` Adrien Mazarguil
2017-06-16 13:07 ` Ferruh Yigit
2017-06-14 11:49 ` [dpdk-dev] [PATCH 3/7] net/mlx4: fix Rx interrupts management Adrien Mazarguil
` (6 subsequent siblings)
8 siblings, 1 reply; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-14 11:49 UTC (permalink / raw)
To: dev
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 <adrien.mazarguil@6wind.com>
Acked-by: Moti Haimovsky <motih@mellanox.com>
---
drivers/net/mlx4/mlx4.c | 9 +++++++++
drivers/net/mlx4/mlx4.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 178562e..2b4722f 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -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;
diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
index c46fc23..b74fbf8 100644
--- a/drivers/net/mlx4/mlx4.h
+++ b/drivers/net/mlx4/mlx4.h
@@ -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;
--
2.1.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH 2/7] net/mlx4: fix Rx interrupts with multiple ports
2017-06-14 11:49 ` [dpdk-dev] [PATCH 2/7] net/mlx4: fix Rx interrupts with multiple ports Adrien Mazarguil
@ 2017-06-16 13:07 ` Ferruh Yigit
2017-06-16 13:39 ` Adrien Mazarguil
0 siblings, 1 reply; 13+ messages in thread
From: Ferruh Yigit @ 2017-06-16 13:07 UTC (permalink / raw)
To: Adrien Mazarguil, dev
On 6/14/2017 12:49 PM, Adrien Mazarguil wrote:
> 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
>
> Fixes: 9f05a4b81809 ("net/mlx4: support user space Rx interrupt event")
>
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Acked-by: Moti Haimovsky <motih@mellanox.com>
> ---
> drivers/net/mlx4/mlx4.c | 9 +++++++++
> drivers/net/mlx4/mlx4.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index 178562e..2b4722f 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -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;
>
> diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
> index c46fc23..b74fbf8 100644
> --- a/drivers/net/mlx4/mlx4.h
> +++ b/drivers/net/mlx4/mlx4.h
> @@ -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;
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH 2/7] net/mlx4: fix Rx interrupts with multiple ports
2017-06-16 13:07 ` Ferruh Yigit
@ 2017-06-16 13:39 ` Adrien Mazarguil
2017-06-16 13:49 ` Ferruh Yigit
0 siblings, 1 reply; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-16 13:39 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev
On Fri, Jun 16, 2017 at 02:07:54PM +0100, Ferruh Yigit wrote:
> On 6/14/2017 12:49 PM, Adrien Mazarguil wrote:
> > 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?
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.
> 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.
> > Fixes: 9f05a4b81809 ("net/mlx4: support user space Rx interrupt event")
> >
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Acked-by: Moti Haimovsky <motih@mellanox.com>
> > ---
> > drivers/net/mlx4/mlx4.c | 9 +++++++++
> > drivers/net/mlx4/mlx4.h | 1 +
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> > index 178562e..2b4722f 100644
> > --- a/drivers/net/mlx4/mlx4.c
> > +++ b/drivers/net/mlx4/mlx4.c
> > @@ -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;
> >
> > diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
> > index c46fc23..b74fbf8 100644
> > --- a/drivers/net/mlx4/mlx4.h
> > +++ b/drivers/net/mlx4/mlx4.h
> > @@ -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
6WIND
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH 2/7] net/mlx4: fix Rx interrupts with multiple ports
2017-06-16 13:39 ` Adrien Mazarguil
@ 2017-06-16 13:49 ` Ferruh Yigit
0 siblings, 0 replies; 13+ messages in thread
From: Ferruh Yigit @ 2017-06-16 13:49 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: dev
On 6/16/2017 2:39 PM, Adrien Mazarguil wrote:
> On Fri, Jun 16, 2017 at 02:07:54PM +0100, Ferruh Yigit wrote:
>> On 6/14/2017 12:49 PM, Adrien Mazarguil wrote:
>>> 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?
>
> 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.
>
>> 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.
Thanks for clarification.
>
>>> Fixes: 9f05a4b81809 ("net/mlx4: support user space Rx interrupt event")
>>>
>>> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>>> Acked-by: Moti Haimovsky <motih@mellanox.com>
>>> ---
>>> drivers/net/mlx4/mlx4.c | 9 +++++++++
>>> drivers/net/mlx4/mlx4.h | 1 +
>>> 2 files changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
>>> index 178562e..2b4722f 100644
>>> --- a/drivers/net/mlx4/mlx4.c
>>> +++ b/drivers/net/mlx4/mlx4.c
>>> @@ -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;
>>>
>>> diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
>>> index c46fc23..b74fbf8 100644
>>> --- a/drivers/net/mlx4/mlx4.h
>>> +++ b/drivers/net/mlx4/mlx4.h
>>> @@ -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;
>>>
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH 3/7] net/mlx4: fix Rx interrupts management
2017-06-14 11:49 [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Adrien Mazarguil
2017-06-14 11:49 ` [dpdk-dev] [PATCH 1/7] net/mlx4: fix typos from prior commit Adrien Mazarguil
2017-06-14 11:49 ` [dpdk-dev] [PATCH 2/7] net/mlx4: fix Rx interrupts with multiple ports Adrien Mazarguil
@ 2017-06-14 11:49 ` Adrien Mazarguil
2017-06-14 11:49 ` [dpdk-dev] [PATCH 4/7] net/mlx5: fix misplaced Rx interrupts functions Adrien Mazarguil
` (5 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-14 11:49 UTC (permalink / raw)
To: dev
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 <adrien.mazarguil@6wind.com>
Acked-by: Moti Haimovsky <motih@mellanox.com>
---
drivers/net/mlx4/mlx4.c | 166 +++++++++++++++++++------------------------
1 file changed, 72 insertions(+), 94 deletions(-)
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 2b4722f..050d646 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -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;
}
/**
--
2.1.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH 4/7] net/mlx5: fix misplaced Rx interrupts functions
2017-06-14 11:49 [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Adrien Mazarguil
` (2 preceding siblings ...)
2017-06-14 11:49 ` [dpdk-dev] [PATCH 3/7] net/mlx4: fix Rx interrupts management Adrien Mazarguil
@ 2017-06-14 11:49 ` Adrien Mazarguil
2017-06-14 11:49 ` [dpdk-dev] [PATCH 5/7] net/mlx5: fix Rx interrupts support checks Adrien Mazarguil
` (4 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-14 11:49 UTC (permalink / raw)
To: dev; +Cc: stable
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: stable@dpdk.org
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
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(-)
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index 2a26839..5421d1f 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -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;
+}
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 3f6fb70..cade625 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -2142,76 +2142,3 @@ removed_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
(void)pkts_n;
return 0;
}
-
-/**
- * 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;
-}
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 8db8eb1..cd79b5d 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -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 */
--
2.1.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH 5/7] net/mlx5: fix Rx interrupts support checks
2017-06-14 11:49 [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Adrien Mazarguil
` (3 preceding siblings ...)
2017-06-14 11:49 ` [dpdk-dev] [PATCH 4/7] net/mlx5: fix misplaced Rx interrupts functions Adrien Mazarguil
@ 2017-06-14 11:49 ` Adrien Mazarguil
2017-06-14 11:49 ` [dpdk-dev] [PATCH 6/7] net/mlx5: fix return value in Rx interrupts code Adrien Mazarguil
` (3 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-14 11:49 UTC (permalink / raw)
To: dev; +Cc: stable
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: stable@dpdk.org
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
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(-)
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index bcb2c1b..49d4dba 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -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 {
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index 5421d1f..fa350bf 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -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 */
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index cd79b5d..1a3ede4 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -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 */
--
2.1.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH 6/7] net/mlx5: fix return value in Rx interrupts code
2017-06-14 11:49 [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Adrien Mazarguil
` (4 preceding siblings ...)
2017-06-14 11:49 ` [dpdk-dev] [PATCH 5/7] net/mlx5: fix Rx interrupts support checks Adrien Mazarguil
@ 2017-06-14 11:49 ` Adrien Mazarguil
2017-06-14 11:49 ` [dpdk-dev] [PATCH 7/7] net/mlx5: fix Rx interrupts management Adrien Mazarguil
` (2 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-14 11:49 UTC (permalink / raw)
To: dev; +Cc: stable
A negative return value is documented for that function in case of error.
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 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index fa350bf..46d566b 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -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;
}
/**
--
2.1.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH 7/7] net/mlx5: fix Rx interrupts management
2017-06-14 11:49 [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Adrien Mazarguil
` (5 preceding siblings ...)
2017-06-14 11:49 ` [dpdk-dev] [PATCH 6/7] net/mlx5: fix return value in Rx interrupts code Adrien Mazarguil
@ 2017-06-14 11:49 ` Adrien Mazarguil
2017-06-14 14:28 ` [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Nélio Laranjeiro
2017-06-16 13:51 ` Ferruh Yigit
8 siblings, 0 replies; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-14 11:49 UTC (permalink / raw)
To: dev; +Cc: stable
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5
2017-06-14 11:49 [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Adrien Mazarguil
` (6 preceding siblings ...)
2017-06-14 11:49 ` [dpdk-dev] [PATCH 7/7] net/mlx5: fix Rx interrupts management Adrien Mazarguil
@ 2017-06-14 14:28 ` Nélio Laranjeiro
2017-06-16 13:51 ` Ferruh Yigit
8 siblings, 0 replies; 13+ messages in thread
From: Nélio Laranjeiro @ 2017-06-14 14:28 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: dev
On Wed, Jun 14, 2017 at 01:49:10PM +0200, Adrien Mazarguil wrote:
> 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(-)
>
> --
> 2.1.4
For the series:
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
--
Nélio Laranjeiro
6WIND
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5
2017-06-14 11:49 [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Adrien Mazarguil
` (7 preceding siblings ...)
2017-06-14 14:28 ` [dpdk-dev] [PATCH 0/7] Fix Rx interrupt support in mlx4 and mlx5 Nélio Laranjeiro
@ 2017-06-16 13:51 ` Ferruh Yigit
8 siblings, 0 replies; 13+ messages in thread
From: Ferruh Yigit @ 2017-06-16 13:51 UTC (permalink / raw)
To: Adrien Mazarguil, dev
On 6/14/2017 12:49 PM, Adrien Mazarguil wrote:
> 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(-)
Series applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread