[PATCH] net/failsafe: fix interrupt handle leak
David Marchand
david.marchand at redhat.com
Mon Aug 29 12:23:16 CEST 2022
On Fri, Apr 29, 2022 at 3:56 PM Ferruh Yigit <ferruh.yigit at xilinx.com> wrote:
>
> On 3/24/2022 3:09 PM, David Marchand wrote:
> > A intr_handle is being allocated as a hack to get a (proxy) eventfd from
> > the Linux interrupt implementation.
> > But this handle is never freed.
> >
> > Remove this convoluted hack and create an eventfd in Linux case.
> >
> > Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle")
> > Cc: stable at dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand at redhat.com>
> > ---
> > drivers/net/failsafe/failsafe_ops.c | 32 ++++++++++-------------------
> > 1 file changed, 11 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
> > index 55e21d635c..2c23d0e70a 100644
> > --- a/drivers/net/failsafe/failsafe_ops.c
> > +++ b/drivers/net/failsafe/failsafe_ops.c
> > @@ -6,6 +6,9 @@
> > #include <stdbool.h>
> > #include <stdint.h>
> > #include <unistd.h>
> > +#ifdef RTE_EXEC_ENV_LINUX
> > +#include <sys/eventfd.h>
> > +#endif
> >
> > #include <rte_debug.h>
> > #include <rte_atomic.h>
> > @@ -387,28 +390,11 @@ fs_rx_queue_setup(struct rte_eth_dev *dev,
> > const struct rte_eth_rxconf *rx_conf,
> > struct rte_mempool *mb_pool)
> > {
> > - /*
> > - * FIXME: Add a proper interface in rte_eal_interrupts for
> > - * allocating eventfd as an interrupt vector.
> > - * For the time being, fake as if we are using MSIX interrupts,
> > - * this will cause rte_intr_efd_enable to allocate an eventfd for us.
> > - */
> > - struct rte_intr_handle *intr_handle;
> > struct sub_device *sdev;
> > struct rxq *rxq;
> > uint8_t i;
> > int ret;
> >
> > - intr_handle = rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
> > - if (intr_handle == NULL)
> > - return -ENOMEM;
> > -
> > - if (rte_intr_type_set(intr_handle, RTE_INTR_HANDLE_VFIO_MSIX))
> > - return -rte_errno;
> > -
> > - if (rte_intr_efds_index_set(intr_handle, 0, -1))
> > - return -rte_errno;
> > -
> > fs_lock(dev, 0);
> > if (rx_conf->rx_deferred_start) {
> > FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
> > @@ -442,12 +428,16 @@ fs_rx_queue_setup(struct rte_eth_dev *dev,
> > rxq->info.nb_desc = nb_rx_desc;
> > rxq->priv = PRIV(dev);
> > rxq->sdev = PRIV(dev)->subs;
> > - ret = rte_intr_efd_enable(intr_handle, 1);
> > - if (ret < 0) {
> > +#ifdef RTE_EXEC_ENV_LINUX
> > + rxq->event_fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> > + if (rxq->event_fd < 0) {
> > + ERROR("Failed to create an eventfd: %s", strerror(errno));
> > fs_unlock(dev, 0);
> > - return ret;
> > + return -errno;
> > }
> > - rxq->event_fd = rte_intr_efds_index_get(intr_handle, 0);
> > +#else
> > + rxq->event_fd = -1;
> > +#endif
>
> How this impacts the BSD? I don't know if driver used on BSD but
> technically it looks supported.
>
> @Gaetan, any objection to the change?
There was no feedback for months, can we get this merged?
Thanks.
--
David Marchand
More information about the stable
mailing list