From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id 4CA8D5B32 for ; Fri, 2 Nov 2018 12:14:22 +0100 (CET) Received: by mail-wr1-f66.google.com with SMTP id y16so1596692wrw.3 for ; Fri, 02 Nov 2018 04:14:22 -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=1wwhaDBoX6Iqei81y4co8aVPThtw5s4ya6e1P9K/clQ=; b=Zz1saGxTexgApmt4XQkajr8eF7l/9G4B4selsVblHoX2oNJHegWUhhisvnhIy6tokC jYG+B3j4u7jSKZ0/4VCk75izg5QU4M9E9p83E3Fx6ZAd8isXiGOunSW9oYeSneB7SuTc zmv7Yszh4/LDJJy/fhV3A8gT5i77Mj5d7kSi0xt2A346Rolq/QDb82ltTLX4yXcZ0qvt G7FzRRm9aejibUVFwOMlpKSQ/8AYzxV4roIA+f2ivcrsGoE5/0lAiaryt1GxcUVt6gWG k2ASQ3ueFfsENsgvEfFnetoM0/jG0Ocv7OwOoh1llvE6e18oSRm8D88HrbZxXFANvTzs z8VQ== X-Gm-Message-State: AGRZ1gJmiyr+Zb94JkIMZbE6pOPywkg5II53wWHnED3is+2pfs4TOMYT ZAF8LugFTGXkV8I9M5Nqz4w= X-Google-Smtp-Source: AJdET5e4YOi1n9wkmxhnKVyYSJIlGbl/acvHRhkFFLtANZrc9lGdZO9yFwvuIGmt+xO98YacAZlNGw== X-Received: by 2002:adf:e14b:: with SMTP id f11-v6mr9381519wri.42.1541157261738; Fri, 02 Nov 2018 04:14:21 -0700 (PDT) Received: from localhost ([88.98.246.218]) by smtp.gmail.com with ESMTPSA id z6-v6sm12453160wrs.19.2018.11.02.04.14.20 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 02 Nov 2018 04:14:20 -0700 (PDT) Message-ID: <1541157258.4849.17.camel@debian.org> From: Luca Boccassi To: Maxime Coquelin , dev@dpdk.org Cc: ferruh.yigit@intel.com, 3chas3@gmail.com Date: Fri, 02 Nov 2018 11:14:18 +0000 In-Reply-To: <54c2ea74-17ec-ee3a-4b32-a55599f43413@redhat.com> References: <1500332723-10812-1-git-send-email-ciwillia@brocade.com> <1500332723-10812-2-git-send-email-ciwillia@brocade.com> <1541083532.4849.8.camel@debian.org> <4cb5c58b-b0e8-b00c-b302-70f4cb6cb2ee@redhat.com> <54c2ea74-17ec-ee3a-4b32-a55599f43413@redhat.com> 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 1/2] net/virtio: do not re-enter clean up routines X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Nov 2018 11:14:22 -0000 On Fri, 2018-11-02 at 12:03 +0100, Maxime Coquelin wrote: >=20 > On 11/2/18 10:57 AM, Maxime Coquelin wrote: > >=20 > >=20 > > On 11/1/18 3:45 PM, Luca Boccassi wrote: > > > On Mon, 2017-07-17 at 19:05 -0400, Charles (Chas) Williams wrote: > > > > .dev_uninit calls .dev_stop and .dev_close.=C2=A0=C2=A0The work tha= t is > > > > done in > > > > those routines doesn't need repeated.=C2=A0=C2=A0Use started and op= ened > > > > to > > > > track > > > > the adapter's status. > > > >=20 > > > > Signed-off-by: Chas Williams > > > > --- > > > > =C2=A0=C2=A0drivers/net/virtio/virtio_ethdev.c | 15 ++++++++++++--- > > > > =C2=A0=C2=A0drivers/net/virtio/virtio_pci.h=C2=A0=C2=A0=C2=A0=C2=A0= |=C2=A0=C2=A04 +++- > > > > =C2=A0=C2=A02 files changed, 15 insertions(+), 4 deletions(-) > > > >=20 > > > > diff --git a/drivers/net/virtio/virtio_ethdev.c > > > > b/drivers/net/virtio/virtio_ethdev.c > > > > index 00a3122..eff0545 100644 > > > > --- a/drivers/net/virtio/virtio_ethdev.c > > > > +++ b/drivers/net/virtio/virtio_ethdev.c > > > > @@ -609,6 +609,10 @@ virtio_dev_close(struct rte_eth_dev *dev) > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 PMD_INIT_LOG(DEBUG, "virtio_dev_clos= e"); > > > > +=C2=A0=C2=A0=C2=A0 if (!hw->opened) > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return; > > > > +=C2=A0=C2=A0=C2=A0 hw->opened =3D false; > > > > + > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* reset the NIC */ > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (dev->data->dev_flags & RTE_ETH_D= EV_INTR_LSC) > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 VTPCI_OPS(hw= )->set_config_irq(hw, > > > > VIRTIO_MSI_NO_VECTOR); > > > > @@ -1696,6 +1700,8 @@ virtio_dev_configure(struct rte_eth_dev > > > > *dev) > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 return -EBUSY; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } > > > > +=C2=A0=C2=A0=C2=A0 hw->opened =3D true; > > > > + > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return 0; > > > > =C2=A0=C2=A0} > > > > @@ -1759,7 +1765,7 @@ virtio_dev_start(struct rte_eth_dev *dev) > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 VIRTQUEUE_DU= MP(txvq->vq); > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } > > > > -=C2=A0=C2=A0=C2=A0 hw->started =3D 1; > > > > +=C2=A0=C2=A0=C2=A0 hw->started =3D true; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Initialize Link state */ > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 virtio_dev_link_update(dev, 0); > > > > @@ -1824,10 +1830,13 @@ virtio_dev_stop(struct rte_eth_dev > > > > *dev) > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 PMD_INIT_LOG(DEBUG, "stop"); > > > > +=C2=A0=C2=A0=C2=A0 if (!hw->started) > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return; > > > > +=C2=A0=C2=A0=C2=A0 hw->started =3D false; > > > > + > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (intr_conf->lsc || intr_conf->rxq= ) > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 rte_intr_dis= able(dev->intr_handle); > > > > -=C2=A0=C2=A0=C2=A0 hw->started =3D 0; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 memset(&link, 0, sizeof(link)); > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 virtio_dev_atomic_write_link_status(= dev, &link); > > > > =C2=A0=C2=A0} > > > > @@ -1844,7 +1853,7 @@ virtio_dev_link_update(struct rte_eth_dev > > > > *dev, > > > > __rte_unused int wait_to_complet > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 link.link_duplex =3D ETH_LINK_FULL_D= UPLEX; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 link.link_speed=C2=A0=C2=A0=3D SPEED= _10G; > > > > -=C2=A0=C2=A0=C2=A0 if (hw->started =3D=3D 0) { > > > > +=C2=A0=C2=A0=C2=A0 if (!hw->started) { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 link.link_st= atus =3D ETH_LINK_DOWN; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } else if (vtpci_with_feature(hw, VI= RTIO_NET_F_STATUS)) { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 PMD_INIT_LOG= (DEBUG, "Get link status from hw"); > > > > diff --git a/drivers/net/virtio/virtio_pci.h > > > > b/drivers/net/virtio/virtio_pci.h > > > > index 18caebd..65bad2d 100644 > > > > --- a/drivers/net/virtio/virtio_pci.h > > > > +++ b/drivers/net/virtio/virtio_pci.h > > > > @@ -35,6 +35,7 @@ > > > > =C2=A0=C2=A0#define _VIRTIO_PCI_H_ > > > > =C2=A0=C2=A0#include > > > > +#include > > > > =C2=A0=C2=A0#include > > > > =C2=A0=C2=A0#include > > > > @@ -253,7 +254,7 @@ struct virtio_hw { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 uint64_t=C2=A0=C2=A0=C2=A0=C2=A0req_= guest_features; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 uint64_t=C2=A0=C2=A0=C2=A0=C2=A0gues= t_features; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 uint32_t=C2=A0=C2=A0=C2=A0=C2=A0max_= queue_pairs; > > > > -=C2=A0=C2=A0=C2=A0 uint16_t=C2=A0=C2=A0=C2=A0=C2=A0started; > > > > +=C2=A0=C2=A0=C2=A0 bool=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0started; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 uint16_t=C2=A0=C2=A0=C2=A0 max_mtu; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 uint16_t=C2=A0=C2=A0=C2=A0=C2=A0vtne= t_hdr_size; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 uint8_t=C2=A0=C2=A0=C2=A0 =C2=A0=C2= =A0=C2=A0=C2=A0vlan_strip; > > > > @@ -268,6 +269,7 @@ struct virtio_hw { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct virtio_pci_common_cfg *common= _cfg; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct virtio_net_config *dev_cfg; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 void=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0= =C2=A0=C2=A0*virtio_user_dev; > > > > +=C2=A0=C2=A0=C2=A0 bool=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0opened; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct virtqueue **vqs; > > > > =C2=A0=C2=A0}; > > >=20 > > > Hi Maxime, > > >=20 > > > Any chance this could be reviewed and taken care of, please? It's > > > been > > > forgotten in the queue for a year, but we still use it. > > > I have sent 2/2 separately so ignore that (I didn't notice this > > > series > > > was still pending review). > > >=20 > > > Thanks! > > >=20 > >=20 > >=20 > > Reviewed-by: Maxime Coquelin > >=20 > > I'll certainly have to rebase it, but it should be trivial. > > And as discussed on IRC, it fixes a real issue, so will have to be > > backported to stable. I'll take care of it. > >=20 > > Regards, > > Maxime >=20 > Applied to dpdk-next-virtio/master >=20 > If you have some time, please have a look at the branch to ensure the > rebase is correct. >=20 > Thanks, > Maxime Looks good to me, thank you very much! --=20 Kind regards, Luca Boccassi