From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 6389FA0547
	for <public@inbox.dpdk.org>; Tue,  9 Feb 2021 03:36:04 +0100 (CET)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 4AF931606BA;
	Tue,  9 Feb 2021 03:36:04 +0100 (CET)
Received: from mga04.intel.com (mga04.intel.com [192.55.52.120])
 by mails.dpdk.org (Postfix) with ESMTP id 373A840147;
 Tue,  9 Feb 2021 03:36:01 +0100 (CET)
IronPort-SDR: k2RDdDrD7pNhgsoUkNYEjvmM8sI078vl7+GG4RFOIXytpOngudcyd/XC14E3TWxuE2DA7doxiC
 /2Gic/GBbmyQ==
X-IronPort-AV: E=McAfee;i="6000,8403,9889"; a="179258555"
X-IronPort-AV: E=Sophos;i="5.81,163,1610438400"; d="scan'208";a="179258555"
Received: from orsmga002.jf.intel.com ([10.7.209.21])
 by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 08 Feb 2021 18:35:59 -0800
IronPort-SDR: 8hEbdSPNXgx8kHGRDsiMRITQWhuoX04Pu7r/y24NSp1Z/b6ypl/jo9kY3afTwSw1JMV1eGO741
 Vl7scc3A0fpQ==
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.81,163,1610438400"; d="scan'208";a="374795164"
Received: from orsmsx604.amr.corp.intel.com ([10.22.229.17])
 by orsmga002.jf.intel.com with ESMTP; 08 Feb 2021 18:35:59 -0800
Received: from orsmsx609.amr.corp.intel.com (10.22.229.22) by
 ORSMSX604.amr.corp.intel.com (10.22.229.17) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2106.2; Mon, 8 Feb 2021 18:35:59 -0800
Received: from orsmsx603.amr.corp.intel.com (10.22.229.16) by
 ORSMSX609.amr.corp.intel.com (10.22.229.22) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2106.2; Mon, 8 Feb 2021 18:35:58 -0800
Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) 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.2106.2
 via Frontend Transport; Mon, 8 Feb 2021 18:35:58 -0800
Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.170)
 by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.1.1713.5; Mon, 8 Feb 2021 18:35:58 -0800
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=TYOOagS4+eVZPXtH3QGwKceMmfdO9xzKOAkEFfpF/F5AxQfczlVrcAHKyOrb9YuH1zgEV5RytCdvPAZLW8kKPMJCN6b4aYp5HUunkbQgLNCx7kr0yMHYZlceqXaRxWhyyeWV5kGB89of4NNxfcOUU9gtVm8fZSLoJA+eF6SLT8UYtQHGv2YI8jYg/5W2XQtBrQe9ajkfa851CYTXIK1z3gp3A3jS607FTsVwbcBndpGddq0H4WS/9L1gSzWcnqgW8suhhpa7pYEORB7lucQqA7cCNEanCY/LEInd83/MylaX5kefNGrWvikKnU0SJd21QBAYaRsGcYaFGhR6DodOsg==
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=E+n2PtnhyqVnUaqC7qnFk8eEGjvlsc02wE0eTJAoVjQ=;
 b=Liyi7G5bf00MkYmBBMAzj9ICe6RTlyb1D2ZD+SU/QHsFvuNSo+ewp0Dd0Nm8rvmIW31wJM2Qu5i28buGmX0HhfwDgOl+Wq3Lko34arj1Kcf3+R1n8S1gm+/sqUeo0dQlujP5Irc4SjFKLuXjY8ia2mHUVioY537OW5yzsGFcHPPBUi6QJmv6WPDM4TiuYuCznj/bDZ4eX36MaTLiy8ywFLmrKVsO8oGGkVC98tGJYFKhwjxF+xv+iZNiHAZLoLH435FW+gKVaTeLoZ4+H8VQCo6OTv6lYOFeSgPnpTgpBAT86Kmp7kJ7W4GXNd3nT3WizHTHjrD+1jKISQD+qVgjHg==
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
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; 
 s=selector2-intel-onmicrosoft-com;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=E+n2PtnhyqVnUaqC7qnFk8eEGjvlsc02wE0eTJAoVjQ=;
 b=VEgl2OxKcHjdFwRJe65Le5Uz1oji2LRZWcRURBpr74DntUBPN7AxeBYdLTsSaKLyZkLaHjG4VDaHND1Db0kx3z9m8zjNM7L6P30sdBefWB59NsNTHZ25u2G2WWfCJTyyvRPYJ26i0W747IVjmdb5GDzANexeckpDUOHkJumkt90=
Received: from CY4PR11MB1750.namprd11.prod.outlook.com (2603:10b6:903:126::8)
 by CY4PR11MB1336.namprd11.prod.outlook.com (2603:10b6:903:2f::18)
 with Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3825.17; Tue, 9 Feb
 2021 02:35:43 +0000
Received: from CY4PR11MB1750.namprd11.prod.outlook.com
 ([fe80::8fd:c082:f2bc:f6ae]) by CY4PR11MB1750.namprd11.prod.outlook.com
 ([fe80::8fd:c082:f2bc:f6ae%12]) with mapi id 15.20.3825.030; Tue, 9 Feb 2021
 02:35:43 +0000
From: "Li, Xiaoyun" <xiaoyun.li@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>, "Singh, Jasvinder"
 <jasvinder.singh@intel.com>, "Wu, Jingjing" <jingjing.wu@intel.com>, "Adrien
 Mazarguil" <adrien.mazarguil@6wind.com>, "Dumitrescu, Cristian"
 <cristian.dumitrescu@intel.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
Thread-Topic: [PATCH v2] app/testpmd: fix meter commands help strings
Thread-Index: AQHW/i1Ubb/tljhcA0GE+w5gQxbg4qpPFoxw
Date: Tue, 9 Feb 2021 02:35:43 +0000
Message-ID: <CY4PR11MB1750B5972FDE7C07C40A072B998E9@CY4PR11MB1750.namprd11.prod.outlook.com>
References: <20210205133926.779938-1-ferruh.yigit@intel.com>
 <20210208151520.1048763-1-ferruh.yigit@intel.com>
In-Reply-To: <20210208151520.1048763-1-ferruh.yigit@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
authentication-results: intel.com; dkim=none (message not signed)
 header.d=none;intel.com; dmarc=none action=none header.from=intel.com;
x-originating-ip: [192.102.204.37]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 52978feb-43cf-44f1-bfa1-08d8cca3662e
x-ms-traffictypediagnostic: CY4PR11MB1336:
x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr
x-ms-exchange-transport-forked: True
x-microsoft-antispam-prvs: <CY4PR11MB1336526AA97EE99F4BD1A45B998E9@CY4PR11MB1336.namprd11.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:10000;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: 0s+zAPqKAWJRMCd3zdRgbiM3X3JuEw5kkmvNGUmO1uP9A8m8RPKDh9+YYvod+wMTCt1KQI22AfvJqVy852UJ+ZCw1hqDF+DpMVx+K9nlGC69mGe757mbbdD/DRhhMOIU3JNBZ25IKLn3pqKGaanSDnhOFh7mHVVRVx0GU09Ffx9lQsAhwxfwFDzWR7IdNmShSmTZAl1ARwwfC6i/9Rsmj/CN7k9pNp+q49czw06Gb+cqW4xDTE0m/Jo1FIUHa041E/VLRs75WfZx27UobqqXeDYDbNXu/zqG+6Ohe4qiDjEefXJBzkIal2utTtqcq4njAcMla7MEobIACPID1gJqIk9/acRwfEUDUH+YCu5cWncevdMvALGdj53iADJEJNEKUH9jPmdnWG3jA9ggl0eyZV48ZNAahiTP+hDKKgcaA6hQr+lnTczyTNQtPisbSrqChqRrMbXDKIXK8WUPp9Mp6FohdrEnKNo8PmA1mZxCokyVoL+v1N21Jilu6XdehPByJTcQC1jqeTy3ZRaI/kNjuQ==
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
 IPV:NLI; SFV:NSPM; H:CY4PR11MB1750.namprd11.prod.outlook.com; PTR:; CAT:NONE;
 SFS:(376002)(396003)(346002)(366004)(136003)(39860400002)(6506007)(71200400001)(53546011)(52536014)(6636002)(26005)(76116006)(8936002)(7696005)(478600001)(66946007)(5660300002)(8676002)(4326008)(55016002)(9686003)(33656002)(186003)(86362001)(54906003)(110136005)(66446008)(66476007)(83380400001)(66556008)(64756008)(2906002)(316002);
 DIR:OUT; SFP:1102; 
x-ms-exchange-antispam-messagedata: =?us-ascii?Q?nBpZgyaDnoQsFMjHvUzwDoZJCS8nKz5WOPuvh1eoPtC4xJjPLQKpdiwqAsOc?=
 =?us-ascii?Q?GVrRoEBF68uCXCQA4DeOniUwS2kqirxDaJPYqu4pt8lOAMD34+s7IA5I9LT4?=
 =?us-ascii?Q?QZKFTltTCuQ13P+fmWagTPFgsc5pPVTBodx9arHimRXYw/yKzYXe6NnjO1vK?=
 =?us-ascii?Q?5PVvZ5MoBNf37qQwoR66bhL3FlDG+jGFEYTsqMjzVhX8uDCJfUGwopm60qNO?=
 =?us-ascii?Q?QQP6mUh4ien8tYIM1THLaCr2K/dmnGyEMxvbgpqV4yvtI33Ly7ScZBT6IttB?=
 =?us-ascii?Q?8hpLvBpxSZVueuQFfibZvlWbn2AH4mH7xjOtz7V8ePn8wMXmEg+waGQTvuVp?=
 =?us-ascii?Q?9r13lnjHmjgH+QMZl7KIMBOqiayp0eyqIQM9wErjSWJWIJmPPgC91ua0X3IS?=
 =?us-ascii?Q?fl//BRtC8n/Lon/axl2sVixJ9YBt1FHJVwQTaZb1G/TRL2BAVM3ZeV8Ba0SM?=
 =?us-ascii?Q?oj5cOxPbLKOB0CqpoawaSb7o5WMxpxA01LyWuXW0O4K3pA9xT3vKrBHeDmQI?=
 =?us-ascii?Q?0s1rKEv333dlhVAVawFGCqMRnf5/VYRYZet2frVMxfw5q9RVsP/JUaE4psna?=
 =?us-ascii?Q?CybZamigaSrqaKZJoRbBvAVkGsMx3b2Cjkx0/Fz4Wd3QvrbiRFsIFUWCsW9g?=
 =?us-ascii?Q?2KFJ7FIBm5qYUcX0JCQWsHmH7f9WBUDjjvzVY9MAHxv27/3brM71hOtGu6gx?=
 =?us-ascii?Q?n0F1FKQyrVLGy/Ql6gm0/Pj+v8xhNW4GGXjIgOUyaj+H+2AY1oxVQzj93JMV?=
 =?us-ascii?Q?cDkXjAO5CuchR/0PzDHiSJQ8mWhQbtPJtNmLYBXRzTp8GKmHG0zkMpb8yDXb?=
 =?us-ascii?Q?D8JGj+OPalyzS0yvn2zbCve8Wxm0Rr9GzGspp3L0J4Hf1wjkzjq6bUSNI2F4?=
 =?us-ascii?Q?ii2heRzFdXecdFa8KPLMgdlO1alaqVq6p7wo0bWhce6pr5/M5TQov1lXJ4n9?=
 =?us-ascii?Q?EIuM1PMImuVr5zkLLtRYCfP9KJ/tyW2D81vS9pGifv8RapxWkvFF7XBz3lPM?=
 =?us-ascii?Q?rekr5yMeqaiFnB2ijVSVQgtXfq4ZaeCEXSuNcfSDKHnYdR2gQAVOEBoVJKc0?=
 =?us-ascii?Q?1ptI71vt6+5xQ9j7UkFKTotYovV4Cjp/vIuCty+PRi5fyZCMOxEUm8N4IomO?=
 =?us-ascii?Q?4lYVMe1x/sI4KqclLnk6WNoNUha6lDRzeEC7yi3uoaUHiVi6VZMmJaMsPDB7?=
 =?us-ascii?Q?S7v61WTJHJI2jqdoZ76wmWRj5NPg5kXMx/awW1xjaelf5io0FVtESsY2CUHW?=
 =?us-ascii?Q?Nm3TM2Y6TiKfDrTl4bJfzKgSNhtrSGCv374ZArevfSEYlChH9N6EwsF1/uM+?=
 =?us-ascii?Q?kblo0lxFI+Q2DSeSqahgWlsU?=
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: CY4PR11MB1750.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 52978feb-43cf-44f1-bfa1-08d8cca3662e
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Feb 2021 02:35:43.7990 (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: zDhpaytOcDjCMzRGdubkyl21eX4/uEQJ9inDRBujl1TTOCjlDU3O6T3UD0vZjrHnAif0wrfohuuGRGE2RYdUPA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR11MB1336
X-OriginatorOrg: intel.com
Subject: Re: [dpdk-stable] [PATCH v2] app/testpmd: fix meter commands help
 strings
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.29
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>

Hi

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Monday, February 8, 2021 23:15
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Adrien
> Mazarguil <adrien.mazarguil@6wind.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH v2] app/testpmd: fix meter commands help strings
>=20
> Helps strings syntax is "command : description", the 'command' part was m=
issing,
> updated command help strings.
>=20
> Fixes: 281eeb8afc55 ("app/testpmd: add commands for metering and policing=
")
> Fixes: 30ffb4e67ee3 ("app/testpmd: add commands traffic metering and
> policing")
> Fixes: e63b50162aa3 ("app/testpmd: clean metering and policing commands")
> Cc: stable@dpdk.org
>=20
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: jasvinder.singh@intel.com
> Cc: cristian.dumitrescu@intel.com
>=20
> - "set port meter dscp table" documented with 'port_id' & 'mtr_id', but
>   command itself is not requiring it, can be better to double check the
>   intention in the command.
> - In command "show port meter stats <port_id> <mtr_id> yes|no", it is
>   not clear what 'yes|no' is, can be better to have a 'clear' keyword
>   there: "show port meter stats <port_id> <mtr_id> clear yes|no"
> - 'meter' commands seems using many high level commands, that is harder
>   to remember when you take all commands into account:
>   "show port meter ..."
>   "add port meter ..."
>   "del port meter ..."
>   "create port meter ..."
>   "enable port meter ..."
>   "disable port meter ..."
>   "set port meter ..."
>   And some high level commands created just for 'meter'. Instead I think
>   it is better to group the commands, like:
>   "port meter [add,del,create,enable,disable] ..."
>   "show port meter ..."
>   It is already too late but it worth to keep in mind for the possible
>   future update.
>=20
> v2:
> * Fixed typo, actiono -> action0
> * Added more info to help string, like "<variable>(possible values)"
> ---
>  app/test-pmd/cmdline.c     |  2 +-
>  app/test-pmd/cmdline_mtr.c | 33 +++++++++++++++++++--------------
>  2 files changed, 20 insertions(+), 15 deletions(-)
>=20
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 59722d268bc3..1b9da38fb745 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -719,7 +719,7 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"set port meter profile (port_id) (mtr_id) (profile_id)\n"
>  			"    meter update meter profile\n\n"
>=20
> -			"set port meter dscp table (port_id) (mtr_id)
> [(dscp_tbl_entry0)\n"
> +			"set port meter dscp table [(dscp_tbl_entry0)\n"
>  			"(dscp_tbl_entry1)...(dscp_tbl_entry63)]\n"
>  			"    update meter dscp table entries\n\n"
>=20

I still have some concern on this one.
I understand the command itself doesn't include port_id & meter_id.
But this command use (void *)&cmd_set_port_meter_dscp_table_token_string, t=
o take a string from cmd.
And in parser cmd_set_port_meter_dscp_table_parsed(), it will use parse_mul=
ti_token_string() to parse this token_string.
static int
parse_multi_token_string(char *t_str, uint16_t *port_id,
	uint32_t *mtr_id, enum rte_color **dscp_table)
{
	char *token;
	uint64_t val;
	int ret;

	/* First token: port id */
	token =3D strtok_r(t_str, PARSE_DELIMITER, &t_str);
	if (token =3D=3D  NULL)
		return -1;

	ret =3D parse_uint(&val, token);
	if (ret !=3D 0 || val > UINT16_MAX)
		return -1;

	*port_id =3D val;

	/* Second token: meter id */
	token =3D strtok_r(t_str, PARSE_DELIMITER, &t_str);
	if (token =3D=3D NULL)
		return 0;

	ret =3D parse_uint(&val, token);
	if (ret !=3D 0 || val > UINT32_MAX)
		return -1;

	*mtr_id =3D val;

	ret =3D parse_dscp_table_entries(t_str, dscp_table);
	if (ret !=3D 0)
		return -1;

	return 0;
}

In this function, the first thing in that string will be parsed as port id,=
 the second thing will be parsed as meter id and then table entries.
So I think if the helper string is "set port meter dscp table [(dscp_tbl_en=
try0)...]". This will actually confuse users.
If a user follows the helper and sets command "set port meter dscp table ds=
cp_tbl_entry0", user will get error because the " dscp_tbl_entry0" is treat=
ed as portid and there is no meter id provided.

I understand they shouldn't put port id and meter id in token string parse =
and should be put in command itself. But that's another story.

> --
> 2.29.2