From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 86A5B1B4E3 for ; Fri, 13 Jul 2018 10:03:08 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 0CB46940060; Fri, 13 Jul 2018 08:03:07 +0000 (UTC) Received: from [192.168.1.16] (85.187.13.33) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Fri, 13 Jul 2018 09:03:01 +0100 To: Wenzhuo Lu , References: <1531373220-42150-1-git-send-email-wenzhuo.lu@intel.com> <1531449770-107772-1-git-send-email-wenzhuo.lu@intel.com> From: Andrew Rybchenko CC: Ferruh Yigit , Thomas Monjalon Message-ID: <25492396-a8ea-9fb8-553b-87d64a84feda@solarflare.com> Date: Fri, 13 Jul 2018 11:02:56 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <1531449770-107772-1-git-send-email-wenzhuo.lu@intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [85.187.13.33] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23964.003 X-TM-AS-Result: No--10.143400-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1531468988-7887UgOoy21N Subject: Re: [dpdk-dev] [PATCH v2] ethdev: fix device info getting 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, 13 Jul 2018 08:03:09 -0000 Hi, Wenzhuo, I'm sorry, but I have more even harder questions than the previous one. This questions are rather generic and mainly to ethdev maintainers. On 13.07.2018 05:42, Wenzhuo Lu wrote: > The device information cannot be gotten correctly before > the configuration is set. Because on some NICs the > information has dependence on the configuration. Thinking about it I have the following question. Is it valid behaviour of the dev_info if it changes after configuration? I always thought that the primary goal of the dev_info is to provide information to app about device capabilities to allow app configure device and queues correctly. Now we see the case when dev_info changes on configure. May be it is acceptable, but it is really suspicious. If we accept it, it should be documented. May be dev_info should be split into parts: part which is persistent and part which may depend on device configuration. > Fixes: 3be82f5cc5e3 ("ethdev: support PMD-tuned Tx/Rx parameters") > Signed-off-by: Wenzhuo Lu > --- > lib/librte_ethdev/rte_ethdev.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > index 3d556a8..6f606c1 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -1022,6 +1022,17 @@ struct rte_eth_dev * > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP); > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -ENOTSUP); > > + if (dev->data->dev_started) { > + RTE_ETHDEV_LOG(ERR, > + "Port %u must be stopped to allow configuration\n", > + port_id); > + return -EBUSY; > + } > + > + /* Copy the dev_conf parameter into the dev structure, > + * then get the info. > + */ > + memcpy(&dev->data->dev_conf, &local_conf, sizeof(dev->data->dev_conf)); It is not a problem of the patch, but I'd like to highlight it to Ferruh and Thomas. What we have in the case of below failures? State is really inconsistent in the case of reconfigure. We have applied new dev_conf, but we still have previous Rx/Tx queues which were setup before. > rte_eth_dev_info_get(port_id, &dev_info); > > /* If number of queues specified by application for both Rx and Tx is > @@ -1053,16 +1064,6 @@ struct rte_eth_dev * > return -EINVAL; > } > > - if (dev->data->dev_started) { > - RTE_ETHDEV_LOG(ERR, > - "Port %u must be stopped to allow configuration\n", > - port_id); > - return -EBUSY; > - } > - > - /* Copy the dev_conf parameter into the dev structure */ > - memcpy(&dev->data->dev_conf, &local_conf, sizeof(dev->data->dev_conf)); > - > /* > * Check that the numbers of RX and TX queues are not greater > * than the maximum number of RX and TX queues supported by the