From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <luca.boccassi@gmail.com>
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 <dev@dpdk.org>; Thu, 27 Sep 2018 10:39:49 +0200 (CEST)
Received: by mail-wm1-f68.google.com with SMTP id b19-v6so4986709wme.3
for <dev@dpdk.org>; Thu, 27 Sep 2018 01:39:49 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20161025;
h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to
:references:content-transfer-encoding:mime-version;
bh=RMfwJlkZIiVOTEhW6+P7eJvKn+zkotQlND1VGBIoN3U=;
b=Kz2YITx3dkDH+rwloCvoAZfT9Ne/rtXC0LdBSw7TqaRlLeGmggjz1IFB5IC14pWSD2
BO6ByirpguDYX2T2AQMfF1kU/hDv+5GhzAh5BSvAP6b6yhIEyJomrooCVy8RMtHVCsXN
QV3Tet/SurA9dezsgtFCnOGgo4Hk9N7VHFMuGoCnEk8FLwoJUcZ2ltWcJTVPSwGYdtDF
KP/oZzj3eAkiPwaItnnpn4WTG4qqBQ6l4C65hZaqSGsjLgsf3Li7ErTJe90RBP8bO2dQ
Q8dvai4G+8rDyXvdzr1OMTNyuFAg1NiYHvfpUlutksQzhj5fo8Xi0HrjiNw3qYMUqEzo
+ZJw==
X-Gm-Message-State: ABuFfois5SI9vd2dlZlo8ucvh99UZ+OHxDRBO/CG0E64H6FzvJ/yFlJd
21jezCLbHzeHFW09pq7IpUO7zIwj
X-Google-Smtp-Source: ACcGV6048/gPuLxtiMCFRwWdDhq++6BCLnphY6G5OVhyOMq25iZ+ZE9hHBGyxmdUy3Use+m6YiNxrg==
X-Received: by 2002:a1c:cf49:: with SMTP id
f70-v6mr6837127wmg.27.1538037588357;
Thu, 27 Sep 2018 01:39:48 -0700 (PDT)
Received: from localhost ([2a01:4b00:f419:6f00:8361:8946:ba2b:d556])
by smtp.gmail.com with ESMTPSA id 200-v6sm3680005wmv.6.2018.09.27.01.39.46
(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
Thu, 27 Sep 2018 01:39:46 -0700 (PDT)
Message-ID: <1538037585.8363.26.camel@debian.org>
From: Luca Boccassi <bluca@debian.org>
To: dev@dpdk.org
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
Date: Thu, 27 Sep 2018 09:39:45 +0100
In-Reply-To: <20180919125757.17938-2-bluca@debian.org>
References: <20180816135032.28283-1-bluca@debian.org>
<20180919125757.17938-1-bluca@debian.org>
<20180919125757.17938-2-bluca@debian.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Mailer: Evolution 3.22.6-1+deb9u1
Mime-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v2 2/3] net/vmxnet3: fix vmxnet3 dev_uninit()
hot-unplug
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 27 Sep 2018 08:39:49 -0000
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 <brussell@brocade.com>
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
> 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