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 F118BA046B for ; Tue, 23 Jul 2019 15:34:44 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AE7E91BFF9; Tue, 23 Jul 2019 15:34:43 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 975901BFEA; Tue, 23 Jul 2019 15:34:42 +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-us4.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id E6E574C007D; Tue, 23 Jul 2019 13:34:40 +0000 (UTC) Received: from [192.168.1.11] (85.187.13.152) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 23 Jul 2019 14:34:35 +0100 To: Thomas Monjalon CC: , Ferruh Yigit , References: <1563873208-5096-1-git-send-email-arybchenko@solarflare.com> <1563883881-16258-1-git-send-email-arybchenko@solarflare.com> <1925153.tuYcfiiuYR@xps> From: Andrew Rybchenko Message-ID: Date: Tue, 23 Jul 2019 16:34:30 +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: <1925153.tuYcfiiuYR@xps> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [85.187.13.152] 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-24788.000 X-TM-AS-Result: No-4.208100-8.000000-10 X-TMASE-MatchedRID: QfHZjzml1E/mLzc6AOD8DfHkpkyUphL9IrMoP5XxqGfg91xayX4L81Zd F0VJxFbiJxk8LaZtmm3jIoyR5q0xua3U5Al7iV4j1bFtmBqqFLQEa8g1x8eqF0FwIhIhpx6TEAW d3hVwuW3dgm4gOEDtayxFZqaO1rGgJ610v/GCm771MIl9eZdLbyIk3dpe5X+hzP9LEqj2YniMvg H05QxjddOJFqzdXiLXu0ilAEqjhkfyvD+WC+3RekrM69p7lDSsOq7WO79QiaebKItl61J/ycnjL TA/UDoAA6QGdvwfwZa3sNbcHjySQd0H8LFZNFG7CKFCmhdu5cVmArchyPpIsqXRqIpsF52ijEvs Bf3fP9pZnVb+5ENCYhCj/yAWhrI9s1JcmGTZ5xzDWRYYsjxILm91ZMG8R24TajaoMkVCdHaigEH y7J4S6ylkreA5r24aYnCi5itk3iprD5+Qup1qU56oP1a0mRIj X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--4.208100-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24788.000 X-MDID: 1563888881-tptYYKjV6vXN Subject: Re: [dpdk-dev] [PATCH v2] ethdev: avoid usage of uninit device info in bad port case 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 7/23/19 4:14 PM, Thomas Monjalon wrote: > 23/07/2019 14:11, Andrew Rybchenko: >> rte_eth_dev_info_get() returns void and caller does know if the function >> does its job or not. Changing of the return value to int would be >> API/ABI breakage which requires deprecation process and cannot be >> backported to stable branches. For now, make sure that device info is >> initialized even in the case of invalid port ID. >> >> Fixes: a30268e9a2d0 ("ethdev: reset whole dev info structure before filling") >> Cc: stable@dpdk.org >> >> Signed-off-by: Andrew Rybchenko >> --- >> --- a/lib/librte_ethdev/rte_ethdev.c >> +++ b/lib/librte_ethdev/rte_ethdev.c >> + /* >> + * Init dev_info before port_id check since caller does not have >> + * return status and does not know if get is successful or not. >> + */ >> + memset(dev_info, 0, sizeof(struct rte_eth_dev_info)); > If someone was using a canary to detect failure, it will be resetted. I've not thought about such ways to check. I would expected check for not NULL device or driver_name. It is not defined behaviour of the function to not touch dev_info in the case of bad port ID. > Why is it urgent to have this workaround? Nothing really urgent, but I still think that it is a right fix to be applied and backported to stable branches. I really met calls with invalid port ID and it took some time to understand where uninitialized data come from. > Can we wait one more release for the definitive fix with error code? No strong opinion, but definitive fix will not be backported. >> + >> RTE_ETH_VALID_PORTID_OR_RET(port_id); >> dev = &rte_eth_devices[port_id]; >> >> - memset(dev_info, 0, sizeof(struct rte_eth_dev_info)); >> dev_info->rx_desc_lim = lim; >> dev_info->tx_desc_lim = lim; >> dev_info->device = dev->device; >>