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 903B4A2E1B for ; Tue, 3 Sep 2019 13:59:36 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 10CFE1C1DE; Tue, 3 Sep 2019 13:59:35 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 3EE7C1C1CE for ; Tue, 3 Sep 2019 13:59:33 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us5.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id CD4394C005C; Tue, 3 Sep 2019 11:59:30 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 3 Sep 2019 12:59:24 +0100 To: Ferruh Yigit , Wenzhuo Lu , Jingjing Wu , Bernard Iremonger , Adrien Mazarguil CC: , Ivan Ilchenko References: <1566915962-5472-1-git-send-email-arybchenko@solarflare.com> <1566915962-5472-3-git-send-email-arybchenko@solarflare.com> <932aee2b-0ecb-a5d9-1dff-7b16bafd2d61@intel.com> From: Andrew Rybchenko Message-ID: <3897a598-62dc-e046-801c-ad44677591f4@solarflare.com> Date: Tue, 3 Sep 2019 14:59:21 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <932aee2b-0ecb-a5d9-1dff-7b16bafd2d61@intel.com> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24886.003 X-TM-AS-Result: No-17.883500-8.000000-10 X-TMASE-MatchedRID: Jm7Yxmmj9Om8rRvefcjeTSZ6hERbr2kd69aS+7/zbj+qvcIF1TcLYIaA TSpziR3NuRWBL1zIHKCX9moRF8wW9xL/12vY1jd3BcaL/tyWL2NfAXPuWnqbj7lmMfLNiukaJeL xnuELHSg9vVjJjTiisfxJpelXs/hKCN0Xjgarp5xFeAr5guIYJp61dsZ2f28J4ilGqCIXmu26PD 0Fdea44OpQ4rsHyTSjk9yCAJhmQwXyZJjykKilZUf49ONH0RaSb1d/zpzApVqEAZ+8YfT0PUCQj +h+9wPkhnvtDFEmZaHGmVW+QJKlUVDqf+tE0BQqGUlF/M3Dxp80AJe3B5qfBnxO7py+RBC8ETmT E0q+rtyhTrpajzy4rQQbFCKnTbI4T/iRFWG2G/rBtFDYGmaWKjcy5Kr1d07TDC/Vm90If4UVcJR 4XudE5kXMeswpFkgKHcN4x+ltYL1zySdTkJm1SnEFgt4Wa1LtqmRbFDx9ddDZhqPLU/1bVd8mfx iZsPBGWrDkp7pQgx//voIzhn8wZuVHGbcDbAq6OX/V8P8ail1ZDL1gLmoa/PoA9r2LThYYvJqH1 oJhQIf7WtrE3WKWZOblyQ9FIjqxnShvUr3UB1+A9MQp1OY41wANlplwoR624D6uVMkVBSSV9UXz CAO1c7+TgT8yd0Wya+gahEj+7tM= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--17.883500-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24886.003 X-MDID: 1567511971-PZrJ5ZMxMl2k Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH 02/51] app/testpmd: check status of getting ethdev info 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/2/19 3:40 PM, Ferruh Yigit wrote: > On 8/27/2019 3:25 PM, Andrew Rybchenko wrote: >> From: Ivan Ilchenko >> >> Add eth_dev_info_get_print_err() which is a wrapper for >> rte_eth_dev_info_get() printing error if rte_eth_dev_info_get() >> fails and returning its status code. >> >> Signed-off-by: Ivan Ilchenko >> Signed-off-by: Andrew Rybchenko > <...> > >> @@ -2410,9 +2414,13 @@ struct cmd_config_rss_hash_key { >> struct rte_eth_dev_info dev_info; >> uint8_t hash_key_size; >> uint32_t key_len; >> + int ret; >> >> memset(&dev_info, 0, sizeof(dev_info)); > It should be in different most probably, but the 'memset' before > 'rte_eth_dev_info_get()' looks redundant, since API does the same. I'll add pre-patch to fix it everywhere I found. > <...> > >> @@ -4746,7 +4780,8 @@ struct cmd_tunnel_tso_set_result { >> { >> struct rte_eth_dev_info dev_info; >> >> - rte_eth_dev_info_get(port_id, &dev_info); >> + eth_dev_info_get_print_err(port_id, &dev_info); > Should we check the return value here too? And return 'dev_info' immediately on > error instead of going on. Thanks, missed it. Also there are two more cases in testpmd to get maximum number of Rx/Tx queue. I'll skip port and continue if device info get fails. > <...> > >> @@ -1136,7 +1139,10 @@ struct extmem_param { >> /* Apply default TxRx configuration for all ports */ >> port->dev_conf.txmode = tx_mode; >> port->dev_conf.rxmode = rx_mode; >> - rte_eth_dev_info_get(pid, &port->dev_info); >> + >> + ret = eth_dev_info_get_print_err(pid, &port->dev_info); >> + if (ret != 0) >> + return; > What do you think 'rte_exit()' here instead of 'return', 'init_config()' doesn't > return error, and returning from init will assume everyting setup, 0 number of > queues for any port eventually will cause an exit in the app, better to fail > where error occurs. I agree. I've considered to skip the port and continue with other port, but it is better to catch such problems earlier instead of hiding behind complex logic. Thanks, Andrew.