From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7FD61A052A; Mon, 25 Jan 2021 18:57:31 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0833F1410BE; Mon, 25 Jan 2021 18:57:31 +0100 (CET) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id 7D1481410AB for ; Mon, 25 Jan 2021 18:57:29 +0100 (CET) IronPort-SDR: qi9eLKaIKSJc0W95V9nnMk4+vR243IGGdPM7r1EpczpH/fCzyh/v0Z6RjRWayBu7lXPhiyfBWL CwGrGMJStGaw== X-IronPort-AV: E=McAfee;i="6000,8403,9875"; a="179919127" X-IronPort-AV: E=Sophos;i="5.79,374,1602572400"; d="scan'208";a="179919127" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jan 2021 09:57:26 -0800 IronPort-SDR: EbHpSVq6Nfnzln1rUcf1VyW4N0dbprsyCpjmypcGJcI0vJnedyvqwt2/YWfl9Qc94sgMe7qL+Y +vC6FKCusBlA== X-IronPort-AV: E=Sophos;i="5.79,374,1602572400"; d="scan'208";a="361608321" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.243.89]) ([10.213.243.89]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Jan 2021 09:57:24 -0800 To: Lance Richardson , Steve Yang Cc: dev@dpdk.org, "Lu, Wenzhuo" , xiaoyun.li@intel.com, "Iremonger, Bernard" , Thomas Monjalon , Andrew Rybchenko , "Yang, Qiming" References: <20210122090110.50453-1-stevex.yang@intel.com> <20210125083202.38267-1-stevex.yang@intel.com> <20210125083202.38267-3-stevex.yang@intel.com> From: Ferruh Yigit Message-ID: Date: Mon, 25 Jan 2021 17:57:21 +0000 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 v4 2/2] app/testpmd: fix max-pkt-len option invalid X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 1/25/2021 3:46 PM, Lance Richardson wrote: > On Mon, Jan 25, 2021 at 3:35 AM Steve Yang wrote: >> >> Moved the setting of 'DEV_RX_OFFLOAD_JUMBO_FRAME' from >> 'cmd_config_max_pkt_len_parsed()' to 'init_config()' caused fail the case >> where 'max_rx_pkt_len' is changed from the command line via >> "port config all max-pkt-len". >> >> The 'init_config()' function is only called when testpmd is started, >> but the DEV_RX_OFFLOAD_JUMBO_FRAME setting needs to be recomputed whenever >> 'max_rx_pkt_len' changes. >> >> Define the 'update_jumbo_frame_offload()' function for both 'init_config()' >> and 'cmd_config_max_pkt_len_parsed()', and detect if 'max_rx_pkt_len' >> should be update to 1500 + overhead as default configuration. At the same >> time, the offloads of rx queue also should update the value once port >> offloads changed (e.g.: disabled JUMBO_FRAME offload via changed >> max-pkt-len, reproduce steps [1]), otherwise, it would cause unexpected >> result. >> >> [1] >> -------------------------------------------------------------------------- >> ./x86_64-native-linuxapp-gcc/app/dpdk-testpmd -c 0xf -n 4 -- -i >> --max-pkt-len=9000 --tx-offloads=0x8000 --rxq=4 --txq=4 --disable-rss >> testpmd> set verbose 3 >> testpmd> port stop all >> testpmd> port config all max-pkt-len 1518 port start all >> >> // Got fail error info without this patch >> Configuring Port 0 (socket 1) >> Ethdev port_id=0 rx_queue_id=0, new added offloads 0x800 must be >> within per-queue offload capabilities 0x0 in rte_eth_rx_queue_setup() >> Fail to configure port 0 rx queues //<-- Fail error info; >> -------------------------------------------------------------------------- >> >> Fixes: 761c4d6690 ("app/testpmd: fix max Rx packet length for VLAN packets") >> >> Signed-off-by: Steve Yang >> --- >> app/test-pmd/cmdline.c | 1 + >> app/test-pmd/parameters.c | 1 + >> app/test-pmd/testpmd.c | 63 ++++++++++++++++++++++++++++----------- >> app/test-pmd/testpmd.h | 2 ++ >> 4 files changed, 50 insertions(+), 17 deletions(-) >> >> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c >> index 89034c8b72..8b6b7b6206 100644 >> --- a/app/test-pmd/cmdline.c >> +++ b/app/test-pmd/cmdline.c >> @@ -1901,6 +1901,7 @@ cmd_config_max_pkt_len_parsed(void *parsed_result, >> printf("Unknown parameter\n"); >> return; >> } >> + update_jumbo_frame_offload(pid, false); > > I'm probably missing something, but why isn't this a matter of simply calling > port_mtu_set() here (with mtu computed from max pkt len) and keeping > init_config() as currently implemented? > They are very similar, calling the 'port_mtu_set()' can be an option, but it is headache to get dev->info first and calculate overhead to be able to call 'port_mtu_set()'; Since this is already done in 'init_config()', converting it to a function and reuse it also looks valid as done here. I am updating that function and sending a new version soon.