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 454E8A0562; Tue, 31 Mar 2020 09:31:52 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 72BAC1BFC1; Tue, 31 Mar 2020 09:31:51 +0200 (CEST) Received: from EUR04-DB3-obe.outbound.protection.outlook.com (mail-eopbgr60081.outbound.protection.outlook.com [40.107.6.81]) by dpdk.org (Postfix) with ESMTP id 24F842C6D for ; Tue, 31 Mar 2020 09:31:50 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UPj4TpCwN6wRO1JX39/p8ABSUC74fFREuXWz+1YRcJkmTjhfa9VJjfaoyVxylzmFjc8EqmGWa5wkSly5Bm9qr+801mvGeUPbQjkXRaY6nxYq/Uk7fJkj6IshzWT/9bpDg9sipX2Med8iVQ9eGuB+TzRd7dqRlzQQVErr5OWvQDhkuowWdJ10DxidVdYppsGCKab6x92eVSnXoSIIYJeTwbYD9zg4U0N4SrZ8rFlEuXdoUEubM9nleyJghfv4qaqZfP3E5w8qPVfyupiFvgv4t7FRA4+Rz1v8h5E0gXczT1TsyISZuxeo0lLKUJn3IdK7bw73JuJBi1Lxat1DaPJvaw== 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=dyD6oaWcbC9iVPVfexaYPdkhkT+vaXFKM8OTYRJ/zRM=; b=YE1JbbC9o/a8ha161R2qSGMyIFDVrLdzsHu4qoPuh9dCUEURW5lToSMJOCpY7+K0UgetUfN9PI4jEUzyQ6cpZw4cjCisz+WpSShM3IbcCSKzDiPObIm7FmwjXvC1OInpUrS6/3ZzOvtu/PEP2wiXO/ADqfi9vMazqfh5C8QugRzwl4438sb/554YuefqsI5qTQHg6rYDqrKleVsKHEyZvD5t9lVen8f8CX8F1ebwdvhcadVJ+H179cVh6Kky/IwjIzrA/0hLwcQahckXah0sMmTRk072Q8JDPXFaVdiyBhDiygzUvwByRCYHlh/9akObjZSlX0XZQglUoKoK4fcG5w== 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=dyD6oaWcbC9iVPVfexaYPdkhkT+vaXFKM8OTYRJ/zRM=; b=Hl89KTIMn6qrWObm96aF5P+O0v6ev/7MIKwiBJmAs0cWCi3ofcyHjazisMPWKuEiNMxI0J5FUxvm2UiFwu4iXzc+ChloIBGKVnJUcW5p+mOLeNj7wZlSffEMU/FsF/5h/IqEcEZrrFTZRC/j+WDrrIIC5lNinJxofBHVBXlKoaI= Received: from AM4PR05MB3265.eurprd05.prod.outlook.com (10.171.188.154) by AM4PR05MB3379.eurprd05.prod.outlook.com (10.171.187.28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2835.22; Tue, 31 Mar 2020 07:31:48 +0000 Received: from AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::da5:7919:35c1:894]) by AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::da5:7919:35c1:894%6]) with mapi id 15.20.2856.019; Tue, 31 Mar 2020 07:31:48 +0000 From: Slava Ovsiienko To: Stephen Hemminger , Matan Azrad , Shahaf Shuler CC: "dev@dpdk.org" , Alexander Kozyrev Thread-Topic: [PATCH] common/mlx5: fix bogus assert Thread-Index: AQHWByIH06kShlcoU0ea+M+TfF8VY6hiSb4g Date: Tue, 31 Mar 2020 07:31:48 +0000 Message-ID: References: <20200331060247.10954-1-stephen@networkplumber.org> In-Reply-To: <20200331060247.10954-1-stephen@networkplumber.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=viacheslavo@mellanox.com; x-originating-ip: [95.164.10.10] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: ef73adc6-9860-4ee5-b433-08d7d54592a3 x-ms-traffictypediagnostic: AM4PR05MB3379:|AM4PR05MB3379: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 0359162B6D x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AM4PR05MB3265.eurprd05.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10009020)(4636009)(346002)(376002)(366004)(396003)(136003)(39860400002)(8676002)(81156014)(81166006)(33656002)(66446008)(52536014)(64756008)(6636002)(478600001)(66946007)(66556008)(8936002)(66476007)(76116006)(5660300002)(26005)(186003)(107886003)(2906002)(9686003)(6506007)(53546011)(71200400001)(7696005)(4326008)(55016002)(86362001)(54906003)(110136005)(316002); DIR:OUT; SFP:1101; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 1UVCmh5VQA1yNDpbr+qiRIfTOsbspf1ewpVwyw8nwO0VF2FVdvRy3vWtMM5msS7LFPdpVXGGDebCkI/rbB3yJksQNAQ7wjoOgWAU3qsR+MQJP4E3O1LX8YvzUzISfeNdiDy/K5cC10H424GrIXNir0BC+xAdQ6+fOLLe/9LDUssJjeJyZqELrdjLkRFMDizm45THgl9xulGXlEUIKoP8qPv/4t5U3AeFfzSHoc0oHYi/XMMWWQuaCq9XjpQaUh5vgOoXnskKPFOdDHxCSQZz3YFgLYfb3VnnRbQQxAqHO5jaI6JeYxE18dyLjTJRlO5Rec3ACNKxqcxl9UMFTNkJh/pN8Is2TuQHBN0ZGqcjauZylGiCx+I5I3Ym4ZOsT+xzcr3A0kfXG9+CvMH+ispsbgR/nYaLVpcvhGZ70IU3fiKYWD379kURCFNu2v8ZdlO+ x-ms-exchange-antispam-messagedata: lJHWsT29iwZOSU1cGfv6jhTqoQxyZXbGoL+Z7KRd8YnEZQ37maivudbrApjf6MAZ4U1X9/Qw0hODtMxyi6l7sbpMk7rGlw/sPHroORCKRZWRMR6H7qX1qcdvdKgVk4pIbT8I2RWnKCykCLI9VWUU/g== 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: ef73adc6-9860-4ee5-b433-08d7d54592a3 X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Mar 2020 07:31:48.5056 (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: wcV9L3gqU3aZqsy3YN02IoDkbDNvk8yU4SnV0nZcN0eCbuQVdxJ+a2XBt2LeliM8gHp0DvD4bKeXsUNWgllXaMEEpf9a4fsgTovczZ3jWGY= X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR05MB3379 Subject: Re: [dpdk-dev] [PATCH] common/mlx5: fix bogus assert 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" Hi, Stephen Thank you for the fix. The exposed API to set MAC addresses: - mlx5_mac_addr_set (invoked by rte_mac_addr_set ()) - mlx5_set_mc_addr_list (invoked by rte_eth_dev_set_mc_addr_list()) Both routines call mlx5_internal_mac_addr_add(), it in its turn calls mlx5_nl_mac_addr_add() (that is subject of the patch). mlx5_nl_mac_addr_add is internal function, not exposed external API, the wrong parameter means the critical internal bug, so assert looks to be = relevant here. I would not remove MLX5_ASSERT at all but fix just it.=20 Adding the parameter check and return an error is nice. What do you think? With best regards, Slava > -----Original Message----- > From: Stephen Hemminger > Sent: Tuesday, March 31, 2020 9:03 > To: Matan Azrad ; Shahaf Shuler > ; Slava Ovsiienko > Cc: dev@dpdk.org; Stephen Hemminger ; > Alexander Kozyrev > Subject: [PATCH] common/mlx5: fix bogus assert >=20 > The MLX5 device supports up to 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 therfore 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 >=20 > Signed-off-by: Stephen Hemminger > --- > drivers/common/mlx5/mlx5_nl.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) >=20 > diff --git a/drivers/common/mlx5/mlx5_nl.c > b/drivers/common/mlx5/mlx5_nl.c index 549e787b04bf..69f5efa50aa8 100644 > --- a/drivers/common/mlx5/mlx5_nl.c > +++ b/drivers/common/mlx5/mlx5_nl.c > @@ -671,7 +671,9 @@ mlx5_nl_mac_addr_add(int nlsk_fd, unsigned int > iface_idx, >=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); > + if (index >=3D MLX5_MAX_MAC_ADDRESSES) > + return -EINVAL; > + > BITFIELD_SET(mac_own, index); > } > if (ret =3D=3D -EEXIST) > @@ -700,7 +702,9 @@ int > 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); > + 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 +773,12 @@ mlx5_nl_mac_addr_flush(int nlsk_fd, unsigned int > iface_idx, { > 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); > -- > 2.20.1