From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DAAF3A04B1; Thu, 24 Sep 2020 14:25:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9DC1B1DEC0; Thu, 24 Sep 2020 14:19:37 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 44CC31DEC0 for ; Thu, 24 Sep 2020 14:19:35 +0200 (CEST) IronPort-SDR: getWrazl7abAgq490vVGIi0lyHEzi5DHcl+BRsPjv7XyTmvq3CTebwWTn8kByv8d+4GiSdJCWO f2uF0qwClDrQ== X-IronPort-AV: E=McAfee;i="6000,8403,9753"; a="148946383" X-IronPort-AV: E=Sophos;i="5.77,297,1596524400"; d="scan'208";a="148946383" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Sep 2020 05:19:34 -0700 IronPort-SDR: BdFL+y2TMeBKh3Ef157lBvwglehONtUautGC780vlhk4LP/uCQHDyqkdZSz43DG5G0ssjOsQRv UOA1aV6r9HEg== X-IronPort-AV: E=Sophos;i="5.77,297,1596524400"; d="scan'208";a="512135597" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.251.85.112]) ([10.251.85.112]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Sep 2020 05:19:32 -0700 To: Chengchang Tang , "Wei Hu (Xavier)" Cc: dev@dpdk.org, xavier.huwei@huawei.com References: <20200818120254.72792-1-huwei013@chinasoftinc.com> <20200919104708.58451-1-xavier_huwei@163.com> <20200919104708.58451-4-xavier_huwei@163.com> <72499939-f3c1-1caa-cded-b46b489c1863@intel.com> <887fa0c9-73f4-401c-85ca-ad8a1f1b5a1c@chinasoftinc.com> <5fbb620a-498c-a404-4183-f78190890a78@chinasoftinc.com> <8179ac78-9706-0bc5-e1da-ecda000532f7@intel.com> From: Ferruh Yigit Message-ID: Date: Thu, 24 Sep 2020 13:19:29 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.2.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 3/6] app/testpmd: remove restriction on txpkts set 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 9/24/2020 7:08 AM, Chengchang Tang wrote: > > > On 2020/9/24 0:59, Ferruh Yigit wrote: >> On 9/23/2020 12:57 PM, Wei Hu (Xavier) wrote: >>> Hi, Ferruh Yigit >>> >>> On 2020/9/23 11:14, Wei Hu (Xavier) wrote: >>>> Hi, Ferruh Yigit >>>> >>>> On 2020/9/22 22:51, Ferruh Yigit wrote: >>>>> On 9/19/2020 11:47 AM, Wei Hu (Xavier) wrote: >>>>>> From: Chengchang Tang >>>>>> >>>>>> Currently, if nb_txd is not set, the txpkts is not allowed to be set >>>>>> because the nb_txd is used to avoid the numer of segments exceed the Tx >>>>>> ring size and the default value of nb_txd is 0. And there is a bug that >>>>>> nb_txd is the global configuration for Tx ring size and the ring size >>>>>> could be changed by some command per queue. So these valid check is >>>>>> unreliable and introduced unnecessary constraints. >>>>>> >>>>>> This patch adds a valid check function to use the real Tx ring size to >>>>>> check the validity of txpkts. >>>>>> >>>>>> Fixes: af75078fece3 ("first public release") >>>>>> Cc: stable@dpdk.org >>>>>> >>>>>> Signed-off-by: Chengchang Tang >>>>>> Signed-off-by: Wei Hu (Xavier) >>>>>> --- > > <...> > >>>>> What do you think calling 'rte_eth_rx_queue_info_get()' & 'rte_eth_tx_queue_info_get()' to get the 'nb_desc'? >>>> >>>> Currently not all PMD driver implement the .rxq_info_get and >>>> >>>> .txq_info_get hook function. If calling rte_eth_rx_queue_info_get >>>> >>>> return -ENOSTUP, we still need to obtain the ring_size in this way. >>>> >>> If calling rte_eth_rx_queue_info_get function get ring_size, because not all PMDS implement the relevant the related hook function, we need to >>> check the return value and if the return value is -ENOSTUP, we must obtain the ring_size in this way. >>> Do you prefer this method, right? >>> >> >> Do we really need this check? > > IMHO, these checks are needed. There are two patches use the same method to obtain the ring_size to implement the > verification in the patch set. One is used to verify the validity of the descriptor ID in the ring. Another is used > to avoid the number of segments configuration won't exceed the ring_size. For the first case, if no check is > performed, memory out of bound may occur. For the another one, if no check is performed, the sending may fail when > the number of segment exceed the ring size. >> >> What about verifying 'ring_size' if 'rte_eth_rx_queue_info_get()' returns a valid info, if not ignore the check? >> > > if we ignore the check when the 'rte_eth_rx_queue_info_get()' returns -ENOTSUP, it may still cause an illegal memory > access or sending failure. > Ok, thanks for clarification, agree to your suggestion. (to check 'rte_eth_rx_queue_info_get()' return value and if it is '-ENOSTUP' calculate the 'ring_size' as above.)