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 C6A50A04B1; Tue, 24 Nov 2020 13:23:58 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1E2ADC910; Tue, 24 Nov 2020 13:23:57 +0100 (CET) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 5FAD0C902 for ; Tue, 24 Nov 2020 13:23:54 +0100 (CET) IronPort-SDR: 66QMbMZSUSpFwl2zbe5DlQDbU07W4+dddaUGGLACuCOARK28bOnF2/XMK4aKvqMfaYkq9KdX3Q bHEjZXdvlqyQ== X-IronPort-AV: E=McAfee;i="6000,8403,9814"; a="171153612" X-IronPort-AV: E=Sophos;i="5.78,366,1599548400"; d="scan'208";a="171153612" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Nov 2020 04:23:53 -0800 IronPort-SDR: 1ek/fasbbxMj1XoIYjScicU5MXRXeZvIhd3yJHUOf5qdb6EWyqn9pR82VfLQbjj7jA686+/QZv ctOgTSvuGBKw== X-IronPort-AV: E=Sophos;i="5.78,366,1599548400"; d="scan'208";a="370352542" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.251.85.132]) ([10.251.85.132]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Nov 2020 04:23:51 -0800 To: Thomas Monjalon , "Wei Hu (Xavier)" Cc: "dev@dpdk.org" , "xavier.huwei@huawei.com" , Slava Ovsiienko References: <20200818120254.72792-1-huwei013@chinasoftinc.com> <20200925124719.26001-4-huwei013@chinasoftinc.com> <3600731.79S79Y1HSu@thomas> From: Ferruh Yigit Message-ID: Date: Tue, 24 Nov 2020 12:23:47 +0000 MIME-Version: 1.0 In-Reply-To: <3600731.79S79Y1HSu@thomas> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v4 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 11/24/2020 10:27 AM, Thomas Monjalon wrote: > Is it OK to keep this regression? > > Ferruh, what do you suggest? > I confirm the '--txpkts' parameter is broken now, I suggest submitting a defect for it and continue with the regression. We have alternative for the parameter, "set txpkts " command. The parameter was only working when hardcoded '--txd=N' parameter is provided, the command is more dynamic and works however queue size is configured. We can fix the '--txpkts' in next release. > > 23/11/2020 12:50, Slava Ovsiienko: >> Hi, Wei >> >> It was found this patch rejects the --txpkts command line settings. >> set_tx_pkt_segments() is called before device started and >> we have failure chain: >> >> set_tx_pkt_segments() >> nb_segs_is_invalid() >> get_tx_ring_size () >> rte_eth_tx_queue_info_get() >> >> It causes --txpkts testpmd command line option is ignored. >> >> With best regards, Slava >> >>> -----Original Message----- >>> From: dev On Behalf Of Wei Hu (Xavier) >>> Sent: Friday, September 25, 2020 15:47 >>> To: dev@dpdk.org >>> Cc: xavier.huwei@huawei.com >>> Subject: [dpdk-dev] [PATCH v4 3/6] app/testpmd: remove restriction on txpkts >>> set >>> >>> 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) >>> --- >>> v3 -> v4: >>> add check 'rte_eth_rx_queue_info_get()' return value and >>> if it is '-ENOSTUP' calculate the 'ring_size'. >>> v3: initial version. >>> --- >>> app/test-pmd/config.c | 64 >>> +++++++++++++++++++++++++++++++++++++++++++++++---- >>> 1 file changed, 60 insertions(+), 4 deletions(-) >>> >>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index >>> 6496d2f..8ebb927 100644 >>> --- a/app/test-pmd/config.c >>> +++ b/app/test-pmd/config.c >>> @@ -1893,6 +1893,38 @@ tx_queue_id_is_invalid(queueid_t txq_id) } >>> >>> static int >>> +get_tx_ring_size(portid_t port_id, queueid_t txq_id, uint16_t >>> +*ring_size) { >>> + struct rte_port *port = &ports[port_id]; >>> + struct rte_eth_txq_info tx_qinfo; >>> + int ret; >>> + >>> + ret = rte_eth_tx_queue_info_get(port_id, txq_id, &tx_qinfo); >>> + if (ret == 0) { >>> + *ring_size = tx_qinfo.nb_desc; >>> + return ret; >>> + } >>> + >>> + if (ret != -ENOTSUP) >>> + return ret; >>> + /* >>> + * If the rte_eth_tx_queue_info_get is not support for this PMD, >>> + * ring_size stored in testpmd will be used for validity verification. >>> + * When configure the txq by rte_eth_tx_queue_setup with >>> nb_tx_desc >>> + * being 0, it will use a default value provided by PMDs to setup this >>> + * txq. If the default value is 0, it will use the >>> + * RTE_ETH_DEV_FALLBACK_TX_RINGSIZE to setup this txq. >>> + */ >>> + if (port->nb_tx_desc[txq_id]) >>> + *ring_size = port->nb_tx_desc[txq_id]; >>> + else if (port->dev_info.default_txportconf.ring_size) >>> + *ring_size = port->dev_info.default_txportconf.ring_size; >>> + else >>> + *ring_size = RTE_ETH_DEV_FALLBACK_TX_RINGSIZE; >>> + return 0; >>> +} >>> + >>> +static int >>> rx_desc_id_is_invalid(uint16_t rxdesc_id) { >>> if (rxdesc_id < nb_rxd) >>> @@ -2986,17 +3018,41 @@ show_tx_pkt_segments(void) >>> printf("Split packet: %s\n", split); >>> } >>> >>> +static bool >>> +nb_segs_is_invalid(unsigned int nb_segs) { >>> + uint16_t ring_size; >>> + uint16_t queue_id; >>> + uint16_t port_id; >>> + int ret; >>> + >>> + RTE_ETH_FOREACH_DEV(port_id) { >>> + for (queue_id = 0; queue_id < nb_txq; queue_id++) { >>> + ret = get_tx_ring_size(port_id, queue_id, &ring_size); >>> + >>> + if (ret) >>> + return true; >>> + >>> + if (ring_size < nb_segs) { >>> + printf("nb segments per TX packets=%u >= " >>> + "TX queue(%u) ring_size=%u - ignored\n", >>> + nb_segs, queue_id, ring_size); >>> + return true; >>> + } >>> + } >>> + } >>> + >>> + return false; >>> +} >>> + >>> void >>> set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs) { >>> uint16_t tx_pkt_len; >>> unsigned i; >>> >>> - if (nb_segs >= (unsigned) nb_txd) { >>> - printf("nb segments per TX packets=%u >= nb_txd=%u - >>> ignored\n", >>> - nb_segs, (unsigned int) nb_txd); >>> + if (nb_segs_is_invalid(nb_segs)) >>> return; >>> - } >>> >>> /* >>> * Check that each segment length is greater or equal than >>> -- >>> 2.9.5 >> >> > > > > >