From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 66F4DA034F;
Thu, 14 May 2020 17:11:25 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 4788C1D9D4;
Thu, 14 May 2020 17:11:25 +0200 (CEST)
Received: from EUR02-HE1-obe.outbound.protection.outlook.com
(mail-eopbgr10057.outbound.protection.outlook.com [40.107.1.57])
by dpdk.org (Postfix) with ESMTP id A1B4E1D9C0;
Thu, 14 May 2020 17:11:23 +0200 (CEST)
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
b=ezFX93fXt/kYop+kfsG9X8riSOGM2xYNBig5bU/pgnzsPHvsu1iSgIZ6UsnRnj1HhEFo137Rwgq7dYSHobgr8e4LlSz02pM61a473nzy80FX7teCmjuvAKZaeWMco63egkFFqhPT4BNIKWHXUcizF6dkeM/gvKLNYNCRQtB9T7cWoxJMk0poaTVet3AKbWn39ZubWGYD//mUpYfzfW1PvHiPo1LqFdTVgfk/JktD49rdOe29+SoZhKfS6vjsnnld0DQbOofmgGK+5kt2/vwlf/4PxZ5M6F0jxLxq8udAtsp/DL80gS9pZlzW78K3zW+kY22PqHitPIa6xIyz5ggEvg==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;
s=arcselector9901;
h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
bh=6XrHshZQWakzLueTE3+AWR8z9+lullwGisJp0ECnKy8=;
b=iX5jr1QWYJzyK1FOJJTrTXW9BGRbqfuUG6T9dJyf/ZVWKIaHugjaTghCW1wrmg+sjl/X2+++fiB+IcGepG2RBKeeEkfSMJJ4wh7T6g/KeFE9IlHF+QBrBvarnz1aXSkMbgw9q7IOKohNrUSGNoHX9yQkuaNO1GGd4RljxnD0M3MxNf2bFFauvkUxf7XxDvC1ZaC7IKzjw3cp5FgLaLKbA2lOiIhm9BK4P+0lkbxmXFq9DtFQaZhy9LHSmS5kCa7OC1MDs+zuOqm61aH9VzWn+Dj8S32vCiWdljRwFVvPqgKGLHi/GNZ7k/+kIgqTFDrrl4CJorobNuKebsXVaAnnUA==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
smtp.mailfrom=mellanox.com; dmarc=pass action=none header.from=mellanox.com;
dkim=pass header.d=mellanox.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;
s=selector1;
h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
bh=6XrHshZQWakzLueTE3+AWR8z9+lullwGisJp0ECnKy8=;
b=dcNMMKcqJ/EGPRhdYOJjOx6ktx6EEOIULfNJ74/oTPaZVJSl1tjVQt4O709lZ58QWEUTHZv917npLulIf6DLDWxG+/KPCTUi6FfpY699qzWd7febJxZVtvGoRUZSVcvdUnFRjlFm481RY/ttRLEYi+w2zQmcdNxhS/rw0LB8YVQ=
Received: from AM0PR05MB4561.eurprd05.prod.outlook.com (2603:10a6:208:ad::20)
by AM0PR05MB4898.eurprd05.prod.outlook.com (2603:10a6:208:c5::18)
with Microsoft SMTP Server (version=TLS1_2,
cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3000.26; Thu, 14 May
2020 15:11:22 +0000
Received: from AM0PR05MB4561.eurprd05.prod.outlook.com
([fe80::c4de:576a:33dd:554a]) by AM0PR05MB4561.eurprd05.prod.outlook.com
([fe80::c4de:576a:33dd:554a%5]) with mapi id 15.20.2979.033; Thu, 14 May 2020
15:11:22 +0000
From: Alexander Kozyrev <akozyrev@mellanox.com>
To: Slava Ovsiienko <viacheslavo@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>
CC: Matan Azrad <matan@mellanox.com>, Raslan Darawsheh <rasland@mellanox.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>,
"stable@dpdk.org" <stable@dpdk.org>
Thread-Topic: [PATCH v2] common/mlx5: fix bogus assert
Thread-Index: AQHWKb6mN5ECkxbx00y0rbQ+w9nKZ6inrtyg
Date: Thu, 14 May 2020 15:11:22 +0000
Message-ID: <AM0PR05MB4561F71A4AAA6D65A97B9158A2BC0@AM0PR05MB4561.eurprd05.prod.outlook.com>
References: <20200331060247.10954-1-stephen@networkplumber.org>
<1589440142-7197-1-git-send-email-viacheslavo@mellanox.com>
In-Reply-To: <1589440142-7197-1-git-send-email-viacheslavo@mellanox.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: mellanox.com; dkim=none (message not signed)
header.d=none;mellanox.com; dmarc=none action=none header.from=mellanox.com;
x-originating-ip: [174.112.69.51]
x-ms-publictraffictype: Email
x-ms-office365-filtering-ht: Tenant
x-ms-office365-filtering-correlation-id: 5c19e4fa-d8b5-4678-dbda-08d7f819100a
x-ms-traffictypediagnostic: AM0PR05MB4898:
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtFwd
x-ms-exchange-transport-forked: True
x-microsoft-antispam-prvs: <AM0PR05MB4898A7731D1A0F3D4D90BAB0A2BC0@AM0PR05MB4898.eurprd05.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:5516;
x-forefront-prvs: 040359335D
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: Q2aG9Iydk1mvTgJkjOgsEEYkogy5Q7bgEEMv4PmtIutGP1e8LN1NnTE+rVvBpqTsI76HxEqqfvD4mBJuSDoGDo+UxXpsDqmrMW9xwowy8vfT+zlVk8qavIrrjOXIpHAcp3jcK9mM6a/omC6KOHFtelIvbb9au5GtXGzxBNJ04oHxt1qhgzKzM0a7R1fxzEEewZM4VxzCGZj8NvuMypMCDriHgrtmvgHkW6+FlrKXrCDSZOXsJUiJwcvR9X6g1wRwD/v0/xscrB3mb5stbfiyeRvnWf5C7G+OWoXPDEb1kCDQQDV683PO1LlZOHFYHsVhPyEiVTLz1aFAGLiIcHliLZdnXzEBN/W2W36RC9iFNgagfVLdNOVzSfawCQIZ+3q0RaSc+bH/1LtCyCN4EEGUFm5QAeTIeaKMVGxpdb+7fRSWOh6APYmVvdPGtgRyG2e2bLb2qybFCydub9+KpSJvYeSAv9kzmNwqKHHacSXX59HSEylLT7bGxjwnkws9sGSYYXHug8j8HVhkRj2eNm25SQ==
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
IPV:NLI; SFV:NSPM; H:AM0PR05MB4561.eurprd05.prod.outlook.com; PTR:; CAT:NONE;
SFTY:;
SFS:(4636009)(39860400002)(136003)(346002)(396003)(366004)(376002)(2906002)(9686003)(55016002)(52536014)(7696005)(5660300002)(478600001)(76116006)(66446008)(66946007)(66476007)(66556008)(64756008)(186003)(45080400002)(4326008)(83080400001)(6506007)(71200400001)(8936002)(53546011)(8676002)(86362001)(110136005)(54906003)(26005)(33656002)(966005)(316002);
DIR:OUT; SFP:1101;
x-ms-exchange-antispam-messagedata: RPDLehV14tMurEbBe6jio3w7Rm7b3Zg3Ld+t9w/yTBsklzmGilepIMQNluZw5biitHnznV57b5xOmN2VkHatOwegLcNgTCdLEnBVqg4FJRYmFq1o4YaLS9yaeoni/JY7Lo/CX4BQJXgV3DwpsclZ7r0nUrg9jmD9Hd2llxt2F1Smwtt8qsx9Ix0LCDAHXm67JdcbTWLFLRE2hfohvpLj6ZBDkug8vA4nsk5aXEpjnkmJEANG45E7wL/6V0m37lq0+atZoskmEhD1HVrRNBWCZqCZCbFzpy7JIq1RHYoBojSHm/Hq1OjWSqc23J5B8BddZqYdeKSHfPy4KJTUaWKqinI1FKNUvRqzdqLeBoYXRC5Vs0/Y79dmCPED41CPf5LDSDOh+JSeSWIq3lg+9z9d3zMeRJ0hWGuexTRaA+DUFelueoPJ5Bzk86+Vm0i7FDOexQ8KNg9H5Bs5M3/b0FCwFaZW5Q/MvY+fNAxCl/LutRM=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5c19e4fa-d8b5-4678-dbda-08d7f819100a
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 May 2020 15:11:22.2412 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: DLNdTrS7qlcPdlptIYoGQNZhuDncA1MS6mJ2emZUDMtJNHn18aF2Dw4IeKTykQik0JbLEtuDjZpP88zRsjWJpQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR05MB4898
Subject: Re: [dpdk-dev] [PATCH v2] common/mlx5: fix bogus assert
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
These asserts seem redundant for me. Don't you think?
EINVAL is returned, why bother to assert the same condition?
Regards,
Alex =20
> -----Original Message-----
> From: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Sent: Thursday, May 14, 2020 3:09
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; stephen@networkplumber.org; Alexander Kozyrev
> <akozyrev@mellanox.com>; stable@dpdk.org
> Subject: [PATCH v2] common/mlx5: fix bogus assert
>=20
> The MLX5 device supports up to MLX5_MAX_MAC_ADDRESSES (256) MAC
> addresses.
> The code flushes all MAC devices.
>=20
> If DPDK is compiled with MLX5_DEBUG this would an assert.
> PANIC in mlx5_nl_mac_addr_flush():
> line 775 assert "(size_t)(i) < sizeof(mac_own) * 8" failed
>=20
> The root cause is that mac_own is a pointer and is being used as a bitmap=
array.
> The sizeof(mac_own) would therefore be 64 but the number of entries to be
> flushed would be 256.
>=20
> There is a whole set of asserts in MLX5 netlink code with the same bug; t=
hat
> should just be changed into proper error checks.
>=20
> Fixes: 8e46d4e18f09 ("common/mlx5: improve assert control")
> Cc: akozyrev@mellanox.com
> Cc: stable@dpdk.org
>=20
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
>=20
> ---
> v2: fix asserts
> v1:
> https://eur03.safelinks.protection.outlook.com/?url=3Dhttp%3A%2F%2Fpatche=
s.d
> pdk.org%2Fpatch%2F67453%2F&data=3D02%7C01%7Cakozyrev%40mellanox
> .com%7C4f8e2cb5aacd4a33e22a08d7f7d5c7c4%7Ca652971c7d2e4d9ba6a4d14
> 9256f461b%7C0%7C0%7C637250369858023357&sdata=3DZI7CTCQDnnmr6n
> pYXTOxOf4%2BBktSgmE%2F3rC4NG3QXxc%3D&reserved=3D0
> ---
> drivers/common/mlx5/mlx5_nl.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>=20
> diff --git a/drivers/common/mlx5/mlx5_nl.c b/drivers/common/mlx5/mlx5_nl.=
c
> index c144223..65efcd3 100644
> --- a/drivers/common/mlx5/mlx5_nl.c
> +++ b/drivers/common/mlx5/mlx5_nl.c
> @@ -671,7 +671,10 @@ struct mlx5_nl_ifindex_data {
>=20
> ret =3D mlx5_nl_mac_addr_modify(nlsk_fd, iface_idx, mac, 1);
> if (!ret) {
> - MLX5_ASSERT((size_t)(index) < sizeof(mac_own) * CHAR_BIT);
> + MLX5_ASSERT(index < MLX5_MAX_MAC_ADDRESSES);
> + if (index >=3D MLX5_MAX_MAC_ADDRESSES)
> + return -EINVAL;
> +
> BITFIELD_SET(mac_own, index);
> }
> if (ret =3D=3D -EEXIST)
> @@ -700,7 +703,10 @@ struct mlx5_nl_ifindex_data {
> mlx5_nl_mac_addr_remove(int nlsk_fd, unsigned int iface_idx, uint64_t
> *mac_own,
> struct rte_ether_addr *mac, uint32_t index) {
> - MLX5_ASSERT((size_t)(index) < sizeof(mac_own) * CHAR_BIT);
> + MLX5_ASSERT(index < MLX5_MAX_MAC_ADDRESSES);
> + if (index >=3D MLX5_MAX_MAC_ADDRESSES)
> + return -EINVAL;
> +
> BITFIELD_RESET(mac_own, index);
> return mlx5_nl_mac_addr_modify(nlsk_fd, iface_idx, mac, 0); } @@ -
> 769,10 +775,12 @@ struct mlx5_nl_ifindex_data { {
> int i;
>=20
> + if (n <=3D 0 || n >=3D MLX5_MAX_MAC_ADDRESSES)
> + return;
> +
> for (i =3D n - 1; i >=3D 0; --i) {
> struct rte_ether_addr *m =3D &mac_addrs[i];
>=20
> - MLX5_ASSERT((size_t)(i) < sizeof(mac_own) * CHAR_BIT);
> if (BITFIELD_ISSET(mac_own, i))
> mlx5_nl_mac_addr_remove(nlsk_fd, iface_idx,
> mac_own, m,
> i);
> --
> 1.8.3.1