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 BDF72A0613 for ; Tue, 30 Jul 2019 18:39:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 074191BEB6; Tue, 30 Jul 2019 18:39:56 +0200 (CEST) Received: from EUR04-HE1-obe.outbound.protection.outlook.com (mail-eopbgr70084.outbound.protection.outlook.com [40.107.7.84]) by dpdk.org (Postfix) with ESMTP id 73D0D1BEB0 for ; Tue, 30 Jul 2019 18:39:54 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=a2Ku+/S1afApxjJUu84tSMLM10rVaTViOaPFLQ6VTYn2hSe0SxHzSzzvpUC58SCjlPcoKI0LOmWLTvNk9Z4r3mZGXEmWuesU4Siava8sxlejRVVhajedPrMQ/DP8sNfUyr891yURfO2B3uRrk7T9GKdAKl4nyZagzooemoDG4kM0HbXYJayljO2tRH9zh2dzCdTd0sIr5NAh1qervnzw+7c+hGZKj3LjI3wIGKkdOvDp5QWH/9GRz1gWHss3q1CJQ6mLgxkS8tF0edCzSzxCfH100QyfVSIJ0xtx/5qGGB5AXDM8qVqVNLUYSY9SjL4Y3TmFSy9pcy3v7of/HM0AVA== 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=pMWC4lv6evgaWes1ANlmFnwR7qgWwcZqKwDmyP2w+8o=; b=Aek79Q7QkzTqlRJDLdqhjPVBWgbRDnBT7KiwJOdFuDv189BHvq9PrUosZdbCWWouWwyg0yDTBA35x8JrG3Bb/61JAaVWkTw/lHVIzQSqIejU5nVi6RQw8HJt5A5AXmbEAiFS073paJPiXcGJ8Js/qbMllRUYRpP+GAA87hj4HTiR7pMYStk919L6zLStTxjHQHSY5WIhtgjGxfR716o5lI0bJDJ9RUeLyVmBeQ3wcFb4MDr8Wvq3I5zLq6Rn5Gt1QBvw9q6E5OWtlp3oO33Rj+6Wq2PL3FRUtylP9CKA5+2pnnshzmyrZ6Cgs/c5lNUo13ZrdzkJWLYvJ976E6ZNYw== 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=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=pMWC4lv6evgaWes1ANlmFnwR7qgWwcZqKwDmyP2w+8o=; b=EDTzBgZRWeozhSKaU3A1KWwveyairoj4gMatllW8DhIbbGkwt7DXlfdFE4UP69s5NgKNhZB6VWIKYzFh47d5FPthOrv/qba4qlLI9+7sHrolTNOlRZfbA0hDTCTZJHCKOdAK57Crk8MC/2XK6lQCx8ZGSCtJ3vGhGhqR5m4bFPU= Received: from AM0PR0502MB4019.eurprd05.prod.outlook.com (52.133.39.139) by AM0PR0502MB3601.eurprd05.prod.outlook.com (52.133.46.154) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2115.15; Tue, 30 Jul 2019 16:39:52 +0000 Received: from AM0PR0502MB4019.eurprd05.prod.outlook.com ([fe80::ccc2:2dd4:ca86:7639]) by AM0PR0502MB4019.eurprd05.prod.outlook.com ([fe80::ccc2:2dd4:ca86:7639%3]) with mapi id 15.20.2115.005; Tue, 30 Jul 2019 16:39:52 +0000 From: Matan Azrad To: Stephen Hemminger CC: "dev@dpdk.org" , Stephen Hemminger Thread-Topic: [dpdk-dev] [PATCH v4 1/4] examples/multi_process/client_server_mp: check port validity Thread-Index: AQHVRu9wnL3Q0yVBh0WjoEq/NJb4HabjWt7A Date: Tue, 30 Jul 2019 16:39:52 +0000 Message-ID: References: <20190709150939.31338-1-stephen@networkplumber.org> <20190726165054.24078-1-stephen@networkplumber.org> <20190726165054.24078-2-stephen@networkplumber.org> <20190730085656.7c720f28@hermes.lan> In-Reply-To: <20190730085656.7c720f28@hermes.lan> Accept-Language: en-US, he-IL Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=matan@mellanox.com; x-originating-ip: [77.126.64.114] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 93eece48-5f81-4484-d4cd-08d7150c8bde x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(5600148)(711020)(4605104)(1401327)(4618075)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7193020); SRVR:AM0PR0502MB3601; x-ms-traffictypediagnostic: AM0PR0502MB3601: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:4714; x-forefront-prvs: 0114FF88F6 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(4636009)(39860400002)(346002)(136003)(376002)(396003)(366004)(189003)(199004)(4326008)(6116002)(11346002)(305945005)(68736007)(8676002)(86362001)(7696005)(476003)(81156014)(486006)(446003)(74316002)(8936002)(3846002)(33656002)(25786009)(2906002)(81166006)(6246003)(256004)(6916009)(229853002)(6506007)(55016002)(5660300002)(186003)(9686003)(76176011)(14444005)(7736002)(316002)(76116006)(66946007)(71200400001)(66066001)(66476007)(66446008)(478600001)(6436002)(52536014)(66556008)(102836004)(99286004)(71190400001)(26005)(64756008)(54906003)(53936002)(14454004); DIR:OUT; SFP:1101; SCL:1; SRVR:AM0PR0502MB3601; H:AM0PR0502MB4019.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: oaLJYFXZ0ote2mQ5/X91e7hPyPrh4ANTP3QC1Bn3vwX82cpMMEGzX2nZu9wxFvwUzWpTqT2f01RYhYEcaMsXDeQ2Zir0/EzxbBQJFBgkWKjElkCSsxWY7FdsBxy8N2o9zSfLYs5DrcRo4ezJjSHqkxkXLzQqMDaZll20NH+ZSJtjykHEfPCq769FoPuDkAJzxAeC2VIAP++a4u9E/+5KdtZ697A86ZHhyVhAumv/UnoYCgaYNS9Erm2kEikWthEjAiXG/ugQMhMuf6OczZEpQxGSDNncstaFp8pqaT8iAy7LyPXrRNcLaMiKY6wwv8ol7+S7hA+L5UiKwJfxL3+wtUTMrjfw/5c0MRVOWri208noI+uANK/9raDGePqrPhtPEg83RyqFA7BSwVaNS3vPhP/iOPaxtyH8JUjzLyiY4qo= 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: 93eece48-5f81-4484-d4cd-08d7150c8bde X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Jul 2019 16:39:52.4892 (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: matan@mellanox.com X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR0502MB3601 Subject: Re: [dpdk-dev] [PATCH v4 1/4] examples/multi_process/client_server_mp: check port validity 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 From: Stephen Hemminger > On Tue, 30 Jul 2019 09:21:02 +0000 > Matan Azrad wrote: >=20 > > Hi Stephen > > > > From: Stephen Hemminger > > > From: Stephen Hemminger > > > > > > The mp_server incorrectly allows a port mask that included hidden > > > ports and which later caused either lost packets or failed initializa= tion. > > > > > > This fixes explicitly checking that each bit in portmask is a valid > > > port before using it. > > > > > > Fixes: 5b7ba31148a8 ("ethdev: add port ownership") > > > Signed-off-by: Stephen Hemminger > > > --- > > > .../client_server_mp/mp_server/args.c | 46 +++++++++++++----= -- > > > .../client_server_mp/mp_server/args.h | 2 +- > > > .../client_server_mp/mp_server/init.c | 7 +-- > > > 3 files changed, 35 insertions(+), 20 deletions(-) > > > > > > diff --git > > > a/examples/multi_process/client_server_mp/mp_server/args.c > > > b/examples/multi_process/client_server_mp/mp_server/args.c > > > index b0d8d7665c85..c1ab12ad00d1 100644 > > > --- a/examples/multi_process/client_server_mp/mp_server/args.c > > > +++ b/examples/multi_process/client_server_mp/mp_server/args.c > > > @@ -10,6 +10,7 @@ > > > #include > > > > > > #include > > > +#include > > > #include > > > > > > #include "common.h" > > > @@ -34,6 +35,22 @@ usage(void) > > > , progname); > > > } > > > > > > +/** > > > + * Check if port is present in the system > > > + * It maybe owned by a device and should not be used. > > > + */ > > > +static int > > > +port_is_present(uint16_t portid) > > > +{ > > > + uint16_t id; > > > + > > > + RTE_ETH_FOREACH_DEV(id) { > > > + if (id =3D=3D portid) > > > + return 1; > > > + } > > > + return 0; > > > +} > > > + > > > /** > > > * The ports to be used by the application are passed in > > > * the form of a bitmask. This function parses the bitmask @@ > > > -41,31 +58,32 @@ usage(void) > > > * array variable > > > */ > > > static int > > > -parse_portmask(uint8_t max_ports, const char *portmask) > > > +parse_portmask(const char *portmask) > > > { > > > char *end =3D NULL; > > > unsigned long pm; > > > - uint16_t count =3D 0; > > > + uint16_t count; > > > > > > if (portmask =3D=3D NULL || *portmask =3D=3D '\0') > > > return -1; > > > > > > /* convert parameter to a number and verify */ > > > pm =3D strtoul(portmask, &end, 16); > > > - if (end =3D=3D NULL || *end !=3D '\0' || pm =3D=3D 0) > > > + if (end =3D=3D NULL || *end !=3D '\0' || pm > UINT16_MAX || pm =3D= =3D 0) > > > return -1; > > > > > > /* loop through bits of the mask and mark ports */ > > > - while (pm !=3D 0){ > > > - if (pm & 0x01){ /* bit is set in mask, use port */ > > > - if (count >=3D max_ports) > > > - printf("WARNING: requested port %u not > > > present" > > > - " - ignoring\n", (unsigned)count); > > > - else > > > - ports->id[ports->num_ports++] =3D count; > > > + for (count =3D 0; pm !=3D 0; pm >>=3D 1, ++count) { > > > + if ((pm & 0x1) =3D=3D 0) > > > + continue; > > > + > > > + if (!port_is_present(count)) { > > > + printf("WARNING: requested port %u not present - > > > ignoring\n", > > > + count); > > > + continue; > > > } > > > - pm =3D (pm >> 1); > > > - count++; > > > + > > > + ports->id[ports->num_ports++] =3D count; > > > } > > > > Why not just to do something like: > > > > RTE_ETH_FOREACH_DEV(id) { > > If (pm & (1 << id)) { > > pm &=3D ~(1 << id) > > put in list > > } > > } > > If (pm) > > Warning > > >=20 > No real reason, was just trying to keep existing logic flow This is an example for the users\developers, and you on it, I think this is a good chance to show how to do it in the correct way - O(n= ) instead O(n^2). Don't you think so? =20 I suggest to adjust.=20