From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 117BAA046B
for <public@inbox.dpdk.org>; Tue, 23 Jul 2019 15:34:46 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id DF8721C02A;
Tue, 23 Jul 2019 15:34:45 +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 <thomas@monjalon.net>
CC: <dev@dpdk.org>, Ferruh Yigit <ferruh.yigit@intel.com>, <stable@dpdk.org>
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 <arybchenko@solarflare.com>
Message-ID: <ec2b6790-f8ce-70fc-3f53-6a999a51d904@solarflare.com>
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-stable] [dpdk-dev] [PATCH v2] ethdev: avoid usage of
uninit device info in bad port case
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
<mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
<mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
Sender: "stable" <stable-bounces@dpdk.org>
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 <arybchenko@solarflare.com>
>> ---
>> --- 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;
>>