[dpdk-stable] [PATCH 19.11] net/mlx4: fix device detach

[Slava Ovsiienko]

Thank you Christian for the checking,
I'm sorry, I forgot to enable mlx4 in the configuration (my default scripts do not)
while compiling , the v2 is sent.

With best regards,
Slava

> -----Original Message-----
> From: Christian Ehrhardt <christian.ehrhardt at canonical.com>
> Sent: Monday, February 15, 2021 18:24
> To: Slava Ovsiienko <viacheslavo at nvidia.com>
> Cc: dpdk stable <stable at dpdk.org>; Michael Baum <michaelba at nvidia.com>
> Subject: Re: [PATCH 19.11] net/mlx4: fix device detach
> 
> On Mon, Feb 15, 2021 at 4:32 PM Christian Ehrhardt
> <christian.ehrhardt at canonical.com> wrote:
> >
> > On Mon, Feb 15, 2021 at 1:20 PM Christian Ehrhardt
> > <christian.ehrhardt at canonical.com> wrote:
> > >
> > > On Fri, Feb 12, 2021 at 3:33 PM Viacheslav Ovsiienko
> > > <viacheslavo at nvidia.com> wrote:
> > > >
> > > > From: Michael Baum <michaelba at nvidia.com>
> > > >
> > > > [ upstream commit 8e1630e0f1985beb7e48429e3a7614f4732b0e68 ]
> > >
> > > Thanks, added
> > >
> > > > When mlx4 device is probed, 2 different ethdev ports may be
> > > > created for the 2 physical ports of the device.
> > > >
> > > > Wrongly, when the device is removed, the created ports are not
> released.
> > > >
> > > > Close and release the ethdev ports in remove process.
> > > >
> > > > Bugzilla ID: 488
> > > > Fixes: 7fae69eeff13 ("mlx4: new poll mode driver")
> > > > Cc: stable at dpdk.org
> > > >
> > > > Reported-by: David Marchand <david.marchand at redhat.com>
> > > > Signed-off-by: Michael Baum <michaelba at nvidia.com>
> > > > Acked-by: Matan Azrad <matan at nvidia.com>
> > > > Tested-by: David Marchand <david.marchand at redhat.com>
> > > > ---
> > > >  drivers/net/mlx4/mlx4.c | 35 +++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 35 insertions(+)
> > > >
> > > > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> > > > index 4479022a4..3d2cef335 100644
> > > > --- a/drivers/net/mlx4/mlx4.c
> > > > +++ b/drivers/net/mlx4/mlx4.c
> > > > @@ -376,6 +376,10 @@ mlx4_dev_close(struct rte_eth_dev *dev)
> > > >         struct mlx4_priv *priv = dev->data->dev_private;
> > > >         unsigned int i;
> > > >
> > > > +       if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> > > > +               rte_eth_dev_release_port(dev);
> > > > +               return 0;
> >
> >
> > I've found on tests that this won't work.
> > Back in 19.11 this was a void function and therefore "return 0" breaks
> > on some rather strict compiler settings.
> > I'll make it just a "return;" but wanted to ask you to double check if
> > that should overall still work fine in your opinion.
> >
> > > > +       }
> > > >         DEBUG("%p: closing device \"%s\"",
> > > >               (void *)dev,
> > > >               ((priv->ctx != NULL) ? priv->ctx->device->name :
> > > > "")); @@ -1131,6 +1135,36 @@ mlx4_pci_probe(struct rte_pci_driver
> *pci_drv, struct rte_pci_device *pci_dev)
> > > >         return -err;
> > > >  }
> > > >
> > > > +/**
> > > > + * DPDK callback to remove a PCI device.
> > > > + *
> > > > + * This function removes all Ethernet devices belong to a given PCI
> device.
> > > > + *
> > > > + * @param[in] pci_dev
> > > > + *   Pointer to the PCI device.
> > > > + *
> > > > + * @return
> > > > + *   0 on success, the function cannot fail.
> > > > + */
> > > > +static int
> > > > +mlx4_pci_remove(struct rte_pci_device *pci_dev) {
> > > > +       uint16_t port_id;
> > > > +       int ret = 0;
> > > > +
> > > > +       RTE_ETH_FOREACH_DEV_OF(port_id, &pci_dev->device) {
> > > > +               /*
> > > > +                * mlx4_dev_close() is not registered to secondary process,
> > > > +                * call the close function explicitly for secondary process.
> > > > +                */
> > > > +               if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> > > > +                       ret |=
> > > > + mlx4_dev_close(&rte_eth_devices[port_id]);
> 
> Oh there is more, this ^^ will also not work with a void function.
> I'll dequeue the patch for now, please if you would re-spin your backport that
> would be nice.
> 
> > > > +               else
> > > > +                       ret |= rte_eth_dev_close(port_id);
> > > > +       }
> > > > +       return ret == 0 ? 0 : -EIO; }
> > > > +
> > > >  static const struct rte_pci_id mlx4_pci_id_map[] = {
> > > >         {
> > > >                 RTE_PCI_DEVICE(PCI_VENDOR_ID_MELLANOX,
> > > > @@ -1155,6 +1189,7 @@ static struct rte_pci_driver mlx4_driver = {
> > > >         },
> > > >         .id_table = mlx4_pci_id_map,
> > > >         .probe = mlx4_pci_probe,
> > > > +       .remove = mlx4_pci_remove,
> > > >         .drv_flags = RTE_PCI_DRV_INTR_LSC | RTE_PCI_DRV_INTR_RMV,
> > > > };
> > > >
> > > > --
> > > > 2.18.1
> > > >
> > >
> > >
> > > --
> > > Christian Ehrhardt
> > > Staff Engineer, Ubuntu Server
> > > Canonical Ltd
> >
> >
> >
> > --
> > Christian Ehrhardt
> > Staff Engineer, Ubuntu Server
> > Canonical Ltd
> 
> 
> 
> --
> Christian Ehrhardt
> Staff Engineer, Ubuntu Server
> Canonical Ltd