[dpdk-dev] [PATCH v3 2/2] ethdev: complete closing of port
Thomas Monjalon
thomas at monjalon.net
Tue Oct 16 14:25:31 CEST 2018
16/10/2018 13:24, Andrew Rybchenko:
> On 10/15/18 2:20 AM, Thomas Monjalon wrote:
> > After closing a port, it cannot be restarted.
> > So there is no reason to not free all associated resources.
> >
> > The last step was done with rte_eth_dev_detach() which is deprecated.
> > Instead of blindly removing the associated rte_device, the driver should
> > check if no more port (ethdev, cryptodev, etc) is open for the device.
> >
> > The last ethdev freeing which were done by rte_eth_dev_detach(),
> > are now done at the end of rte_eth_dev_close().
> >
> > If the driver is trying to free the port again, the function
> > rte_eth_dev_release_port() will abort with -ENODEV error.
> >
> > Signed-off-by: Thomas Monjalon <thomas at monjalon.net>
> > ---
> > drivers/net/bnxt/bnxt_ethdev.c | 4 ----
> > drivers/net/vhost/rte_eth_vhost.c | 4 ----
> > lib/librte_ethdev/rte_ethdev.c | 9 +++------
> > lib/librte_ethdev/rte_ethdev.h | 3 +--
> > 4 files changed, 4 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> > index ff41cb0fa..183b40821 100644
> > --- a/drivers/net/bnxt/bnxt_ethdev.c
> > +++ b/drivers/net/bnxt/bnxt_ethdev.c
> > @@ -712,10 +712,6 @@ static void bnxt_dev_close_op(struct rte_eth_dev *eth_dev)
> > if (bp->dev_stopped == 0)
> > bnxt_dev_stop_op(eth_dev);
> >
> > - if (eth_dev->data->mac_addrs != NULL) {
> > - rte_free(eth_dev->data->mac_addrs);
> > - eth_dev->data->mac_addrs = NULL;
> > - }
> > if (bp->grp_info != NULL) {
> > rte_free(bp->grp_info);
> > bp->grp_info = NULL;
> > diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
> > index 986bf9633..b9cefbbe5 100644
> > --- a/drivers/net/vhost/rte_eth_vhost.c
> > +++ b/drivers/net/vhost/rte_eth_vhost.c
> > @@ -998,12 +998,8 @@ eth_dev_close(struct rte_eth_dev *dev)
> > for (i = 0; i < dev->data->nb_tx_queues; i++)
> > rte_free(dev->data->tx_queues[i]);
> >
> > - rte_free(dev->data->mac_addrs);
> > free(internal->dev_name);
> > free(internal->iface_name);
> > - rte_free(internal);
> > -
> > - dev->data->dev_private = NULL;
> > }
> >
> > static int
>
> It looks like above snippets should be a part of previous changeset.
No, this is related to close function.
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -367,6 +367,8 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
> > {
> > if (eth_dev == NULL)
> > return -EINVAL;
> > + if (eth_dev->state == RTE_ETH_DEV_UNUSED)
> > + return -ENODEV;
> >
> > rte_eth_dev_shared_data_prepare();
> >
> > @@ -1384,12 +1386,7 @@ rte_eth_dev_close(uint16_t port_id)
> > dev->data->dev_started = 0;
> > (*dev->dev_ops->dev_close)(dev);
> >
> > - dev->data->nb_rx_queues = 0;
> > - rte_free(dev->data->rx_queues);
> > - dev->data->rx_queues = NULL;
> > - dev->data->nb_tx_queues = 0;
> > - rte_free(dev->data->tx_queues);
> > - dev->data->tx_queues = NULL;
> > + rte_eth_dev_release_port(dev);
> > }
> >
> > int
>
> Right now it introduces resource leak for resources which are
> freed by PCI drivers in uninit.
No, these resources are freed by rte_eth_dev_release_port.
More information about the dev
mailing list