From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tetsuya Mukawa <mukawa@igel.co.jp>
Subject: Re: [PATCH v13 2/2] vhost: Add VHOST PMD
Date: Tue, 22 Mar 2016 11:50:04 +0900
Message-ID: <56F0B2DC.3010602@igel.co.jp>
References: <1458030701-11487-3-git-send-email-mukawa@igel.co.jp>
 <1458539108-15686-3-git-send-email-mukawa@igel.co.jp>
 <74F120C019F4A64C9B78E802F6AD4CC24F84543E@IRSMSX106.ger.corp.intel.com>
 <56F0A5F7.2090205@igel.co.jp>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Cc: "Richardson, Bruce" <bruce.richardson@intel.com>,
 "ann.zhuangyanying@huawei.com" <ann.zhuangyanying@huawei.com>,
 "thomas.monjalon@6wind.com" <thomas.monjalon@6wind.com>
To: "Loftus, Ciara" <ciara.loftus@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-pf0-f182.google.com (mail-pf0-f182.google.com
 [209.85.192.182]) by dpdk.org (Postfix) with ESMTP id CC1B32B9C
 for <dev@dpdk.org>; Tue, 22 Mar 2016 03:50:07 +0100 (CET)
Received: by mail-pf0-f182.google.com with SMTP id 4so158285403pfd.0
 for <dev@dpdk.org>; Mon, 21 Mar 2016 19:50:07 -0700 (PDT)
In-Reply-To: <56F0A5F7.2090205@igel.co.jp>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 2016/03/22 10:55, Tetsuya Mukawa wrote:
> On 2016/03/22 0:40, Loftus, Ciara wrote:
>>> +
>>> +static void
>>> +eth_dev_info(struct rte_eth_dev *dev,
>>> +	     struct rte_eth_dev_info *dev_info)
>>> +{
>>> +	dev_info->driver_name = drivername;
>>> +	dev_info->max_mac_addrs = 1;
>>> +	dev_info->max_rx_pktlen = (uint32_t)-1;
>>> +	dev_info->max_rx_queues = dev->data->nb_rx_queues;
>>> +	dev_info->max_tx_queues = dev->data->nb_tx_queues;
>> I'm not entirely familiar with eth driver code so please correct me if I am wrong.
>>
>> I'm wondering if assigning the max queue values to dev->data->nb_*x_queues is correct.
>> A user could change the value of nb_*x_queues with a call to rte_eth_dev_configure(n_queues) which in turn calls rte_eth_dev_*x_queue_config(n_queues) which will set dev->data->nb_*x_queues to the value of n_queues which can be arbitrary and decided by the user. If this is the case, dev->data->nb_*x_queues will no longer reflect the max, rather the value the user chose in the call to rte_eth_dev_configure. And the max could potentially change with multiple calls to configure. Is this intended behaviour?
> Hi Ciara,
>
> Thanks for reviewing it. Here is a part of rte_eth_dev_configure().
>
> int
> rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>                       const struct rte_eth_conf *dev_conf)
> {
>         <snip>
>         /*
>          * Check that the numbers of RX and TX queues are not greater
>          * than the maximum number of RX and TX queues supported by the
>          * configured device.
>          */
>         (*dev->dev_ops->dev_infos_get)(dev, &dev_info);
>
>         if (nb_rx_q == 0 && nb_tx_q == 0) {
>                <snip>
>                 return -EINVAL;
>         }
>
>         if (nb_rx_q > dev_info.max_rx_queues) {
>                <snip>
>                 return -EINVAL;
>         }
>
>         if (nb_tx_q > dev_info.max_tx_queues) {
>                <snip>
>                 return -EINVAL;
>         }
>
>         <snip>
>
>         /*
>          * Setup new number of RX/TX queues and reconfigure device.
>          */
>         diag = rte_eth_dev_rx_queue_config(dev, nb_rx_q);
>         <snip>
>         diag = rte_eth_dev_tx_queue_config(dev, nb_tx_q);
>         <snip>
> }
>
> Anyway, rte_eth_dev_tx/rx_queue_config() will be called only after
> checking the current maximum number of queues.
> So the user cannot set the number of queues greater than current maximum
> number.
>
> Regards,
> Tetsuya

Hi Ciara,

Now, I understand what you say.
Probably you pointed out the case that the user specified a value
smaller than current maximum value.

For example, if we have 4 queues. Below code will be failed at last line.
rte_eth_dev_configure(portid, 4, 4, ...);
rte_eth_dev_configure(portid, 2, 2, ...);
rte_eth_dev_configure(portid, 4, 4, ...);

I will submit a patch to fix it. Could you please review and ack it?

Regards,
Tetsuya