From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-it0-f52.google.com (mail-it0-f52.google.com [209.85.214.52]) by dpdk.org (Postfix) with ESMTP id 497471BA56 for ; Mon, 29 Jan 2018 17:49:25 +0100 (CET) Received: by mail-it0-f52.google.com with SMTP id 196so10030863iti.5 for ; Mon, 29 Jan 2018 08:49:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=xJD8p7G32PSJ6IYr2mBhUZ39RFSE3/wfegv0gmA6mVs=; b=Lc5W8qNtnbJQ1bNwA9EkYxi6eC5bjCC4m5BtSHxT+KjbRHifIF4KgtbMCMSW7gfTvK wBzCcaZQYbwBt0RLirEgjA/RRxQLqaU5UJinisqGIaC848lc69IyLhptx7BBvFhugN87 JuNygFLCuoLo7uqrQeOPYW+5pvOWEd5QaZ6VC4TtBzycdRgKNbQJIqYEt8/2AnIlMX7H vTSAcRbbugw5XcAt+TseiSRVkk/ZaSyUHh3MiuV8neRwL0Ab46GbcszOZ242v6kHzj+M 3jzLxTu2dz3A5FXt57SSLzgbsuXboahCwHvj9nOJilSHTDcDSruyX/JUM0k1OAgHrLsY HwYQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=xJD8p7G32PSJ6IYr2mBhUZ39RFSE3/wfegv0gmA6mVs=; b=ool2iTzqZ45NcASGf7IewdAd/FtVItq4Cz5KLeqz5xomhpMP3gdscfJy19ZsEwLZiR lYSdA9nroeMRh/wQehNMktLcBk7HAK1un9IHq46nrCVokUe72nfLdwZS8ZHLnv4RQVtr 0rA+W7cSPRO0BynrPv3ukQ7tmhuHjE4ojH8xm6tqo0WN2wJFUmNmSIyRnQHOuwNpYVDM pC+dt1bkcwioIgIIvSIfv18fOULsoEjt4AaqhTjGSKrowLgN9wU1r0uOa7hqgNPDGl1e ix1GvWUujquKN5Gj5NqncTjpU+pqv1hSt0hDAWPRgNPdKMpe2Aza3TG0iUsj/EdxhmXn Co3Q== X-Gm-Message-State: AKwxyte+Ir2FrdQA25vTmzXEJnF8Ik/eOtX/YKDBVNAHLvwBCIL96O75 G2N8IJ3Zl/G/+2F8FLWIu9piJTg8gxXckJzOr7s= X-Google-Smtp-Source: AH8x224PIX44vjFyWNEp28ioSU4z7vr91xzZXGySSquySrU9QKFGe1QT/36bp+W+OB23VjMHx5ecNLHvslhnKn554OI= X-Received: by 10.36.155.197 with SMTP id o188mr26955370itd.84.1517244564070; Mon, 29 Jan 2018 08:49:24 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.200.5 with HTTP; Mon, 29 Jan 2018 08:49:23 -0800 (PST) In-Reply-To: <2601191342CEEE43887BDE71AB977258905653D7@IRSMSX103.ger.corp.intel.com> References: <20180129152115.26359-1-3chas3@gmail.com> <2601191342CEEE43887BDE71AB977258905653D7@IRSMSX103.ger.corp.intel.com> From: Chas Williams <3chas3@gmail.com> Date: Mon, 29 Jan 2018 11:49:23 -0500 Message-ID: To: "Ananyev, Konstantin" Cc: "dev@dpdk.org" , "Lu, Wenzhuo" , "Charles (Chas) Williams" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: fix reconfiguration of rx queues 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: Mon, 29 Jan 2018 16:49:25 -0000 On Mon, Jan 29, 2018 at 11:25 AM, Ananyev, Konstantin < konstantin.ananyev@intel.com> wrote: > > > -----Original Message----- > > From: Chas Williams [mailto:3chas3@gmail.com] > > Sent: Monday, January 29, 2018 3:21 PM > > To: dev@dpdk.org > > Cc: Lu, Wenzhuo ; Ananyev, Konstantin < > konstantin.ananyev@intel.com>; Charles (Chas) Williams > > > > Subject: [PATCH] net/ixgbe: fix reconfiguration of rx queues > > > > From: "Charles (Chas) Williams" > > > > .dev_configure() may be called again after RX queues have been setup. > > This has the effect of clearing whatever setting the RX queues made for > > rx_bulk_alloc_allowed or rx_vec_allowed. Only reset this configuration > > is there aren't any currently allocated queues. > > > > Fixes: 01fa1d6215fa ("ixgbe: unify Rx setup") > > > > Signed-off-by: Chas Williams > > --- > > drivers/net/ixgbe/ixgbe_ethdev.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ > ethdev.c > > index 37eb668..b39249a 100644 > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > > @@ -2348,6 +2348,7 @@ ixgbe_dev_configure(struct rte_eth_dev *dev) > > struct ixgbe_adapter *adapter = > > (struct ixgbe_adapter *)dev->data->dev_private; > > int ret; > > + uint16_t i; > > > > PMD_INIT_FUNC_TRACE(); > > /* multipe queue mode checking */ > > @@ -2363,11 +2364,17 @@ ixgbe_dev_configure(struct rte_eth_dev *dev) > > > > /* > > * Initialize to TRUE. If any of Rx queues doesn't meet the bulk > > - * allocation or vector Rx preconditions we will reset it. > > + * allocation or vector Rx preconditions we will reset it. We > > + * can only do this is there aren't any existing RX queues. > > */ > > + for (i = 0; i < dev->data->nb_rx_queues; i++) { > > + if (dev->data->rx_queues[i]) > > + goto out; > > + } > > I don't see why this is needed. > It surely should be possible to reconfigure device with different > number of queues. > Konstantin > Yes, you can add new queues but you shouldn't reset the bulk and vec settings that have already been chosen by the previously allocated queues. If those queues set rx_bulk_alloc_allowed to be false, then this is going to cause an issue with queue release later on. This breaks: rte_eth_dev_configure(..., 1, 1, ...); rx_queue_setup(1) [rx_queue_setup decides that it can't support rx_bulk_alloc_allowed] .. Later, you want to add some more queues, you call eth_eth_dev_configure(..., 2, 2, ...); rx_queue_setup(2) [rx_queue_setup hopefully makes the same choice as rxqid = 1?] ... Is one supposed to release all queues before calling rte_eth_dev_configure()? If that is true, it seems like the change_mtu examples I see are possibly wrong. As suggested in kenrel_nic_interface.rst: ret = rte_eth_dev_configure(port_id, 1, 1, &conf); if (ret < 0) { RTE_LOG(ERR, APP, "Fail to reconfigure port %d\n", port_id); return ret; } /* Restart specific port */ ret = rte_eth_dev_start(port_id); if (ret < 0) { RTE_LOG(ERR, APP, "Fail to restart port %d\n", port_id); return ret; } This is will obviously reset the rx_bulk_alloc_allowed and not reallocated the RX queues. > > > adapter->rx_bulk_alloc_allowed = true; > > adapter->rx_vec_allowed = true; > > > > +out: > > return 0; > > } > > > > @@ -4959,6 +4966,7 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev) > > struct rte_eth_conf *conf = &dev->data->dev_conf; > > struct ixgbe_adapter *adapter = > > (struct ixgbe_adapter *)dev->data->dev_private; > > + uint16_t i; > > > > PMD_INIT_LOG(DEBUG, "Configured Virtual Function port id: %d", > > dev->data->port_id); > > @@ -4981,11 +4989,17 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev) > > > > /* > > * Initialize to TRUE. If any of Rx queues doesn't meet the bulk > > - * allocation or vector Rx preconditions we will reset it. > > + * allocation or vector Rx preconditions we will reset it. We > > + * can only do this is there aren't any existing RX queues. > > */ > > + for (i = 0; i < dev->data->nb_rx_queues; i++) { > > + if (dev->data->rx_queues[i]) > > + goto out; > > + } > > adapter->rx_bulk_alloc_allowed = true; > > adapter->rx_vec_allowed = true; > > > > +out: > > return 0; > > } > > > > -- > > 2.9.5 > >