From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luca Boccassi Subject: Re: [PATCH v2 2/3] net/vmxnet3: fix vmxnet3 dev_uninit() hot-unplug Date: Thu, 27 Sep 2018 09:39:45 +0100 Message-ID: <1538037585.8363.26.camel@debian.org> References: <20180816135032.28283-1-bluca@debian.org> <20180919125757.17938-1-bluca@debian.org> <20180919125757.17938-2-bluca@debian.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: maxime.coquelin@redhat.com, tiwei.bie@intel.com, yongwang@vmware.com, 3chas3@gmail.com, bruce.richardson@intel.com, jianfeng.tan@intel.com, anatoly.burakov@intel.com, llouis@vmware.com, brussell@vyatta.att-mail.com To: dev@dpdk.org Return-path: Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by dpdk.org (Postfix) with ESMTP id 2C98E1B107 for ; Thu, 27 Sep 2018 10:39:49 +0200 (CEST) Received: by mail-wm1-f68.google.com with SMTP id b19-v6so4986709wme.3 for ; Thu, 27 Sep 2018 01:39:49 -0700 (PDT) In-Reply-To: <20180919125757.17938-2-bluca@debian.org> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, 2018-09-19 at 13:57 +0100, Luca Boccassi wrote: > The vmxnet3 driver can't call back into dev_close(), and possibly > dev_stop(), in dev_uninit().=C2=A0=C2=A0When dev_uninit() is called, anyt= hing > that those routines would want to clean up has already been released. > Further, for complete cleanup, it is necessary to release any of the > queue resources during dev_close(). > This allows a vmxnet3 device to be hot-unplugged without leaking > queues. >=20 > Fixes: dfaff37fc46d ("vmxnet3: import new vmxnet3 poll mode driver > implementation") > Cc: stable@dpdk.org >=20 > Signed-off-by: Brian Russell > Signed-off-by: Luca Boccassi > --- > v2: add back extra close() call in uninit() for buggy applications as > =C2=A0=C2=A0=C2=A0=C2=A0requested by the reviewers, and add debug log not= ing the issue >=20 > =C2=A0drivers/net/vmxnet3/vmxnet3_ethdev.c | 35 +++++++++++++++++++++++--= - > -- > =C2=A01 file changed, 29 insertions(+), 6 deletions(-) >=20 > diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c > b/drivers/net/vmxnet3/vmxnet3_ethdev.c > index f1596ab19d..98e5d01890 100644 > --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c > +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c > @@ -354,8 +354,10 @@ eth_vmxnet3_dev_uninit(struct rte_eth_dev > *eth_dev) > =C2=A0 if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > =C2=A0 return 0; > =C2=A0 > - if (hw->adapter_stopped =3D=3D 0) > + if (hw->adapter_stopped =3D=3D 0) { > + PMD_INIT_LOG(DEBUG, "Device has not been closed."); > =C2=A0 vmxnet3_dev_close(eth_dev); > + } > =C2=A0 > =C2=A0 eth_dev->dev_ops =3D NULL; > =C2=A0 eth_dev->rx_pkt_burst =3D NULL; > @@ -802,7 +804,7 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev) > =C2=A0 PMD_INIT_FUNC_TRACE(); > =C2=A0 > =C2=A0 if (hw->adapter_stopped =3D=3D 1) { > - PMD_INIT_LOG(DEBUG, "Device already closed."); > + PMD_INIT_LOG(DEBUG, "Device already stopped."); > =C2=A0 return; > =C2=A0 } > =C2=A0 > @@ -826,7 +828,6 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev) > =C2=A0 /* reset the device */ > =C2=A0 VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, > VMXNET3_CMD_RESET_DEV); > =C2=A0 PMD_INIT_LOG(DEBUG, "Device reset."); > - hw->adapter_stopped =3D 0; > =C2=A0 > =C2=A0 vmxnet3_dev_clear_queues(dev); > =C2=A0 > @@ -836,6 +837,30 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev) > =C2=A0 link.link_speed =3D ETH_SPEED_NUM_10G; > =C2=A0 link.link_autoneg =3D ETH_LINK_FIXED; > =C2=A0 rte_eth_linkstatus_set(dev, &link); > + > + hw->adapter_stopped =3D 1; > +} > + > +static void > +vmxnet3_free_queues(struct rte_eth_dev *dev) > +{ > + int i; > + > + PMD_INIT_FUNC_TRACE(); > + > + for (i =3D 0; i < dev->data->nb_rx_queues; i++) { > + void *rxq =3D dev->data->rx_queues[i]; > + > + vmxnet3_dev_rx_queue_release(rxq); > + } > + dev->data->nb_rx_queues =3D 0; > + > + for (i =3D 0; i < dev->data->nb_tx_queues; i++) { > + void *txq =3D dev->data->tx_queues[i]; > + > + vmxnet3_dev_tx_queue_release(txq); > + } > + dev->data->nb_tx_queues =3D 0; > =C2=A0} > =C2=A0 > =C2=A0/* > @@ -844,12 +869,10 @@ vmxnet3_dev_stop(struct rte_eth_dev *dev) > =C2=A0static void > =C2=A0vmxnet3_dev_close(struct rte_eth_dev *dev) > =C2=A0{ > - struct vmxnet3_hw *hw =3D dev->data->dev_private; > - > =C2=A0 PMD_INIT_FUNC_TRACE(); > =C2=A0 > =C2=A0 vmxnet3_dev_stop(dev); > - hw->adapter_stopped =3D 1; > + vmxnet3_free_queues(dev); > =C2=A0} > =C2=A0 > =C2=A0static void Hi Louis, Are you happy with the diff as in v2 now for vmxnet3? Thanks --=20 Kind regards, Luca Boccassi