From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6389FA0547 for ; 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" To: "Yigit, Ferruh" , "Singh, Jasvinder" , "Wu, Jingjing" , "Adrien Mazarguil" , "Dumitrescu, Cristian" CC: "dev@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: 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: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Archived-At: List-Archive: Hi > -----Original Message----- > From: Yigit, Ferruh > Sent: Monday, February 8, 2021 23:15 > To: Li, Xiaoyun ; Singh, Jasvinder > ; Wu, Jingjing ; Adrien > Mazarguil ; Dumitrescu, Cristian > > Cc: Yigit, Ferruh ; 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 > --- > 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 yes|no", it is > not clear what 'yes|no' is, can be better to have a 'clear' keyword > there: "show port meter stats 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 "(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