From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id D3F93A0540;
	Mon, 24 Oct 2022 20:57:48 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 4A3E042BB4;
	Mon, 24 Oct 2022 20:57:48 +0200 (CEST)
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
 by mails.dpdk.org (Postfix) with ESMTP id 031DF40A8B;
 Mon, 24 Oct 2022 20:57:46 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple;
 d=intel.com; i=@intel.com; q=dns/txt; s=Intel;
 t=1666637867; x=1698173867;
 h=from:to:cc:subject:date:message-id:references:
 in-reply-to:content-transfer-encoding:mime-version;
 bh=uWQlj9kPTdDrWpw4cYzo9PEG56/PnmQE4d5o974t3ZU=;
 b=Z8KEKsD7qX05ZH65BeoeT5RX91jqqS7Dak8rCyZnFz9Hu6uss1AXoGTj
 69IhDuQP0Xvww3dQfRo7SPvU1DkEHIB+wfyOa3SLE3+0x1jODIOHX6KkQ
 LdXL419ZioUkJEJ+4vFsqokCCccA2fUS5MEIPAjCvlx1cbpT71ZSF6Rln
 GvTU8bl6ilooKyXqFq98UlyqSdoNoT7wL+5XxNX72KRaJgFOV9U3EBdgR
 k0dV78QPnbs/b9uVOHnwfpDjon0ukKYIf1s8vxxXAyC3PzuA72bokIavB
 XOCDCyaOFZNUytgxLyEzsosQrnuU3IvwuzHiN1ZZkZcY5pI5cSoQMbnHq g==;
X-IronPort-AV: E=McAfee;i="6500,9779,10510"; a="308590121"
X-IronPort-AV: E=Sophos;i="5.95,210,1661842800"; d="scan'208";a="308590121"
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
 by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 24 Oct 2022 11:57:45 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=McAfee;i="6500,9779,10510"; a="773922693"
X-IronPort-AV: E=Sophos;i="5.95,210,1661842800"; d="scan'208";a="773922693"
Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16])
 by fmsmga001.fm.intel.com with ESMTP; 24 Oct 2022 11:57:44 -0700
Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) by
 ORSMSX603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2375.31; Mon, 24 Oct 2022 11:57:44 -0700
Received: from orsmsx602.amr.corp.intel.com (10.22.229.15) by
 ORSMSX611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2375.31; Mon, 24 Oct 2022 11:57:44 -0700
Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by
 orsmsx602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2375.31 via Frontend Transport; Mon, 24 Oct 2022 11:57:44 -0700
Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.101)
 by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.1.2375.31; Mon, 24 Oct 2022 11:57:42 -0700
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=R2b08HCv8gWEpJbf2PvkZ2DUmSVdX71mcGG2GdDadhw6VkLN8B4w1eH5exvibw7/j0xTRKq+9ws7xWZtVwWPljExI2nB/tPAzpgbW9Q7kDaFVm5i7CC09ze5rxZ2WJDrfOYSP0oIU/czbuE3yp21dt4vWADShGoc9JqYkE0ZVioHf6bFaK4pcJY8Hsr4ufh4mlhR7/zIgyQW8LkeULJjYtVs8CRHI05f9eUNoF719/ZMvXaDTUXOnOlROBhHaQUIlG3+2YXJsdHwWeWtiZgFzf3Frah/G8uKyzqe+FCzzwLlQHc3+6vmiEeEXh+DMcZfB1BUcA0+abcOGfdEeBTyYA==
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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;
 bh=jgZcW/6zCWBnDtRG36xwudrlEwNNXlV74Ne3heWUcxw=;
 b=j2ofhgZ5M01ZK4B3Z4AixKiz/Rv5g/iR7gGk3WVS95ZN10PfrJTe63t5xwAzMT8drhF7VSG2V8gQuM5DP/kq+H0nPJQcuin4P4Gdr4tRp8GG0U7ch5UmpLpMoFdQMPIYxYnIsBj5Yz3Dh69R4EGcfIN9Z66+++u9YlLfbs5XlgYhzT3f6CBxXQvKr1JHIIrqmJqA7B1RQBIJ4cCzaOiQDrA/JR1wIuELD7CIduN6rSAwtMyUb0gnia/toVSLOMl9IYKbtRNylBM84ajKIFb/yHkMN6S0j0JhET70mRU6RWgxyTAEaAfxhn7rc+uXwEoTf3g0EF4i5RoRU/DkXNfvZw==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
 smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com;
 dkim=pass header.d=intel.com; arc=none
Received: from CH0PR11MB5724.namprd11.prod.outlook.com (2603:10b6:610:101::22)
 by SA1PR11MB6784.namprd11.prod.outlook.com (2603:10b6:806:24c::6)
 with Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5746.26; Mon, 24 Oct
 2022 18:57:40 +0000
Received: from CH0PR11MB5724.namprd11.prod.outlook.com
 ([fe80::b0b3:c55:745e:9e88]) by CH0PR11MB5724.namprd11.prod.outlook.com
 ([fe80::b0b3:c55:745e:9e88%5]) with mapi id 15.20.5723.036; Mon, 24 Oct 2022
 18:57:40 +0000
From: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
To: "Ajmera, Megha" <megha.ajmera@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
 "Singh, Jasvinder" <jasvinder.singh@intel.com>, "stephen@networkplumber.org"
 <stephen@networkplumber.org>
CC: "stable@dpdk.org" <stable@dpdk.org>
Subject: RE: [PATCH v4 3/3] sched: support for 100G+ rates in subport/pipe
 config
Thread-Topic: [PATCH v4 3/3] sched: support for 100G+ rates in subport/pipe
 config
Thread-Index: AQHY5Gl8jHvEHAv/jEORJ8ngqrz+G64d6NBw
Date: Mon, 24 Oct 2022 18:57:40 +0000
Message-ID: <CH0PR11MB57242F5F398B26ED07C188DCEB2E9@CH0PR11MB5724.namprd11.prod.outlook.com>
References: <20221019113714.1e949cd2@hermes.local>
 <20221020094747.605087-1-megha.ajmera@intel.com>
 <20221020094747.605087-3-megha.ajmera@intel.com>
In-Reply-To: <20221020094747.605087-3-megha.ajmera@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
dlp-version: 11.6.500.17
dlp-reaction: no-action
dlp-product: dlpe-windows
authentication-results: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=intel.com;
x-ms-publictraffictype: Email
x-ms-traffictypediagnostic: CH0PR11MB5724:EE_|SA1PR11MB6784:EE_
x-ms-office365-filtering-correlation-id: b76c00b1-b26e-4983-208b-08dab5f1a020
x-ms-exchange-senderadcheck: 1
x-ms-exchange-antispam-relay: 0
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: 2XRYlyH+rZ4iiet/rhGYzA7eCjMr3LithLKAqeBGsxzamsuji31x0E05DbDUUTPhyDQ749dmcuNlQ/pSL8lf6KJA5bu0XPOUkRzYp+JzlZ7Y/Di20mwcZWk6YfJybxIgDk0MlXTNBED86/fP4gmA7C8uwt5Seext0FlTLWtFTReUl52UmQlECAjawiJIngtHRdRTMAxFCLjqEaDULZQybuh+BP1o2rN04x/XPJNqGgH8Kfipv3fR65xgHNzaJzZ27pj5kVgVYnIxUX3aPg/7UxZivVZGqeyUT8CnJbCZHOtrEIfC1jJ20ToWqbSWUpSoNR7yVqhUq+PZKGOEgrNR4GUrpq+qumnE6AIabP/D7oZpBLGsOTkuM6fqP2+sEGw1bIPuLXzBxRCicZdh18ovQuc3EMeGrbc6vjShKl3U7ETdhfX6VyyMlpm3TiJw3IvJft23J8QXW3fMzdUTgSw42O6lX3+qXLdZuJQ3SbYwvxbFAumo5O8e21w9QOz95NqWduLzf1qK48Yd94OI0jyspN+4HnUCVPj9qRyaIAQ8qrZGhsvUkZHHLXLOu8F6AkYHsl+YSPdS3trzTE24j2zxr8CbFYIJaViKct1Z9m8RZ+QhNWSGdj18wUcpM2ELBY4PFlXcy07X52cG+DG7bP9HGhs9q3qauTErGUp8sNq+BxFChULG9CdHzjdzqxWas0ZPr3ebRuizOcIyaOFFd7adq4qQQQ7fIQn7PCXsn19H3mggBMeZTI3tNlLU5kxf/xre2Do8K+k9l849YCVSnd5yTQ==
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
 IPV:NLI; SFV:NSPM; H:CH0PR11MB5724.namprd11.prod.outlook.com; PTR:; CAT:NONE;
 SFS:(13230022)(39860400002)(366004)(136003)(396003)(376002)(346002)(451199015)(76116006)(82960400001)(66556008)(66946007)(38100700002)(86362001)(316002)(64756008)(66446008)(66476007)(110136005)(8676002)(4326008)(122000001)(38070700005)(55016003)(33656002)(478600001)(83380400001)(9686003)(6506007)(7696005)(71200400001)(26005)(53546011)(52536014)(41300700001)(8936002)(2906002)(5660300002)(186003);
 DIR:OUT; SFP:1102; 
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?jV2j89YCRanoeYoR4aV1Wp5tADk4XtgvgIhvnMHzhnSOfUldagb97bXxg0ZA?=
 =?us-ascii?Q?aaIUi6djzkdXNauabOABS/laQ8oDbOI2q/mycbTbjTqXNPwi7Nv57ww0OPMn?=
 =?us-ascii?Q?wMyC/AI3f5CX4kKO3qrCSqfIyF3Ai1UPCn+DxGMbsDZjFoZplRz89hcN4wHt?=
 =?us-ascii?Q?lLcHaFVlIlXjiebDkbU7bePoC9H9FsaF9e58smK231T1zfUA6tEPydWfSheC?=
 =?us-ascii?Q?jNZqgtfdC05b3XjB+1lT95qzCzC8HMZ/cmr8SyPFVGCcmPY5P9TFeA+tAvyw?=
 =?us-ascii?Q?R3p9pxD2iKRh6zyW5GHMfPOEFX0R7DR2ucnM8Z3cUgRahqxkFQgwR9jM9OTG?=
 =?us-ascii?Q?NkSzSjLR4TAIsSpYY6txNrTj8SxPqxtAMaIzVVz2uImXdrH1hJNR8zz9C7am?=
 =?us-ascii?Q?bxXHDInDW7D3+TaVA4OcCDpr++GssKN/CoaQXm6FAMcT6/yuDLNlhVuRttjX?=
 =?us-ascii?Q?ULt0YLAPiXEiYTzuxpfE1YN5xrNZoOcjNUk3r7QyHmJY+GUn69YDC9bFJf8r?=
 =?us-ascii?Q?0I19DlgcKfd2HRMnYc+vu5Hz33d26ImVCTw0zpkYchfJ7VKRf3cRIGlw7lCU?=
 =?us-ascii?Q?HWBb8I7LtVhsTuYMJdI82bUpaJNpA057npvuv6ascxMMVv3S9uqV5iBz4fGW?=
 =?us-ascii?Q?KtoRsne11XH9Ft31LrzAfB0WlDhdC02HdaYSHAciim5518BExqcJqXtgqjyX?=
 =?us-ascii?Q?ILiUP2/1XSAkXADGLgN9vW+dYRkaq+o/ODIfXGlg5wa+c5QpvpZaii5Whxj6?=
 =?us-ascii?Q?V+yRoAQr1+KZkMcuIqS88f1eVthNLlRJyfR09vp3fdPI6DNZOS+GSaZPK1kR?=
 =?us-ascii?Q?C4QzOkAtsqfOeCuC6dFuKbjmQM++2e3MHYQ8cBMm3MMKHg8WJzwKk+7+7EyX?=
 =?us-ascii?Q?g7MTAR3pYgqRqa8kG9uM0t0TPF84fXrlxDYni9tYHuxqsoFy300C3WzscdNq?=
 =?us-ascii?Q?K534KTp6D3ntUfePMjhsSji5+cNLG9tFTvQ4GR/D+c2KIzF6L4Q63TMdNmuD?=
 =?us-ascii?Q?Qsyn9h27yqLOi/uay7Z6Sb3iaL1W3GqgMZSnzXEtlE/Ws14c6fzz92uyJxNA?=
 =?us-ascii?Q?WI/KHyx7WeSjQYygPBFbQ72X7EiFi3C8ALcRmeR8gITYgcxFUZ5PzFE09xDH?=
 =?us-ascii?Q?SEFapORkxRK/5/HzAfoTxAYwS1vmGQ8Zgnnu4hOVCI3kTRL7TrglNDq7S1vu?=
 =?us-ascii?Q?SB2pXna0JXgl+3vQ/dZgvIm1Z0RVFfDq+P87WGXAHrPwoUEAd5/k+LM1Fh4q?=
 =?us-ascii?Q?hNx4iqm1vUU1EWbjVdVOIK1EPtIquM0HWNRdSZ5DPrcE/ktlwIoGOO0j/jg0?=
 =?us-ascii?Q?hJJYwy918JAH5rheFCn2uzOrdwYzG1zckjbA7eIeJ97+FyH82dDrJnmGL2AA?=
 =?us-ascii?Q?kdfh6C/crj+glDwan7ZkjCsUy5pUB581/UhfgIk0fjv63k0TURwBewREaC6T?=
 =?us-ascii?Q?rBr1TLM48U7RV3T1Af9kCp5kNq0cCBrIIYs+olYx+EMJRzD58DYUJk73XW1N?=
 =?us-ascii?Q?n6n8SWlSJvTK8DxkK+S3umzKQap8jB86lBOkG3nv71+0Wl0jbA2O4AJb7MAF?=
 =?us-ascii?Q?+umq3I29PMqxtOSbDMVBIeOR6/PTsvCzKx0MOYPZ2P9eYAxSBIwxTsg2JImR?=
 =?us-ascii?Q?Sw=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CH0PR11MB5724.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b76c00b1-b26e-4983-208b-08dab5f1a020
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Oct 2022 18:57:40.4239 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: qetrplPsBTp7jWRw79AtVDPkAbpQ+ueBq2dIxANUIgByozBFziAoq/hnzM0CXzUs6vE6QsuJONf0bscOaIKLhT554k6ay/Ndxgd/9tVKgmA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6784
X-OriginatorOrg: intel.com
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
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

Hi Megha,

Comments inline below.

> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Thursday, October 20, 2022 10:48 AM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> stephen@networkplumber.org
> Cc: stable@dpdk.org
> Subject: [PATCH v4 3/3] sched: support for 100G+ rates in subport/pipe
> config
>=20
> Config load functions updated to support 100G rates
> for subport and pipes.
>=20
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> ---
>  examples/qos_sched/cfg_file.c | 294 ++++++++++++++++++++++++++------
> --
>  1 file changed, 230 insertions(+), 64 deletions(-)
>=20
> diff --git a/examples/qos_sched/cfg_file.c b/examples/qos_sched/cfg_file.=
c
> index ca871d3287..9fcdf5eeb6 100644
> --- a/examples/qos_sched/cfg_file.c
> +++ b/examples/qos_sched/cfg_file.c
> @@ -60,71 +60,154 @@ cfg_load_pipe(struct rte_cfgfile *cfg, struct
> rte_sched_pipe_params *pipe_params
>=20
>  	for (j =3D 0; j < profiles; j++) {
>  		char pipe_name[32];
> +		/* Convert to decimal */
> +		int base =3D 10;

We should set base to 0 to allow multiple basis (8, 10, 16), I see no reaso=
n to limit to base 10, let's take the benefits out of strtoull. I suggest w=
e use the value 0 directly in the function, no need to have a local variabl=
e just for this.
> +
>  		snprintf(pipe_name, sizeof(pipe_name), "pipe profile %d",
> j);
>=20
>  		entry =3D rte_cfgfile_get_entry(cfg, pipe_name, "tb rate");
> -		if (entry)
> -			pipe_params[j].tb_rate =3D (uint64_t)atoi(entry);
> +		if (entry) {
> +			pipe_params[j].tb_rate =3D (uint64_t)strtoull(entry,
> NULL, base);
> +			if (errno =3D=3D EINVAL || errno =3D=3D ERANGE) {
> +				/* cannot convert string to unsigned long
> long */
> +				return -1;
> +			}
> +		}

Some comments for this recurring pattern:

1. Why are you still doing conversion to uint64_t, as strtoull already retu=
rns a 64-bit unsigned integer?
2. If you are checking errno after the call, you should first set errno to =
0 before the call, right? See man strtoull, please.
3. The errno check does not fully handle the error cases, you should also u=
se the second argument (as opposed to setting it to NULL) to check that aft=
er the call the value referenced by the second argument is 0. This ensures =
that there are no illegal characters after the digits, such as in the case =
of "1234qwerty".
4. The comment just before the return does not bring any value, as it is ob=
vious that an error took place if you're returning an error.
5. I suggest you wrap the strtoull() usage into local function, such as par=
se_u64() or similar.


<snip>

Regards,
Cristian