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 F41C9A054F; Fri, 5 Mar 2021 19:44:58 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AE7D222A33F; Fri, 5 Mar 2021 19:44:58 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 2789022A2EB for ; Fri, 5 Mar 2021 19:44:55 +0100 (CET) IronPort-SDR: nHes/lgGnK8cYo/UfiNY3UaEe5/lrzUo9LHI0h6dwFXFWeJSBJO5Wy6dTfkPhGpBnVFSrvMQ5K lPYKpGwXD9Xw== X-IronPort-AV: E=McAfee;i="6000,8403,9914"; a="184333501" X-IronPort-AV: E=Sophos;i="5.81,226,1610438400"; d="scan'208";a="184333501" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Mar 2021 10:44:54 -0800 IronPort-SDR: LFbRuqdfUK+1M70ScIduNzDw1Ej1l8PcyhlhyyN2AFa+MuoHz7J/xBWDrn7j5vOEgVAMzLoH70 BqHoGfiw6RuA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,226,1610438400"; d="scan'208";a="519154412" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by orsmga004.jf.intel.com with ESMTP; 05 Mar 2021 10:44:54 -0800 Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) 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.2106.2; Fri, 5 Mar 2021 10:44:54 -0800 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) 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.2106.2; Fri, 5 Mar 2021 10:44:53 -0800 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2 via Frontend Transport; Fri, 5 Mar 2021 10:44:53 -0800 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.42) 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.2106.2; Fri, 5 Mar 2021 10:44:52 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Tw5v5UXi0QBp905ZnkhYU3r3k+K5TIqQZhUwnU/WNSQGkTTSd8/jk9P5e/LxWN6n9a1xNU5BPzj3c9yEG9eNgbmyCx/9L1okUpBY3AvAk6OiizbXTz/kmdZAdVRlhTMD6KUIbJiZTXDOrxw7QdWiCMx/9ZpMS3euWkYTUAgbj5JNqK1i+y4tT5Fk8ZpO9zSqHD6a7ePEKmuluPPtwOnYZvrCLWZtHpkoyqy+B7bQvJzOeMlLDXQnujwM+zMet74ysLQsiQxeOVSvVZWE63DsT5TacI4hY1+/3w9swzamN4K0wk2g/oelqjPMEHNmyRXLfc6csqMvazMWTjHvIMRUgg== 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=uWVYGVTMmMKv3VlwLjEWddWhvc6BkWpOwB5OXBVWOFo=; b=OTDFV4l8HsA4fWyl66xCa+zc+cfyjdKhLZ71e4iqnt7LbwR2HydBJ4YdUBGOY6yRC86bBzu8AW5JRj6YR+bsVddRxots8FBE6DOeo9JM3sFmtf2KD8jY90hx+8BGom4HuY7muf4EfGI63TnHnty6cuecP0ufc1S4tVTYxiRaLoc6B4REz65xx0Uy95rLkFRnQtF69yWdsFKuZm4QbbIEeDc9pv2HLpyK1Odt7+1rK4QgEdR2vS2OzcACGKRDXIR/VmRT1h2Z1MDl8kRfeU59Aeu4yxAdXvBK3WILwLFjUX9WFN5GcouZmL4XyI1xaziQM+DWld+HMvQqzvWk5IxPZQ== 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=uWVYGVTMmMKv3VlwLjEWddWhvc6BkWpOwB5OXBVWOFo=; b=lUUqq5wHo2f8tZ5/Vp2JVij8OLJcWaObxZ8pN8w3Bvm8iblzlXuFWf3x4aGo8u5cSPM+SVcuRC27pBWIWcmSvzzcTUO31pZhotzJuQabzf7MlFGYdlWxzqGkAGrRCkL0szCpTGngcTSwg60j5x6DQb5IyvNXlX+4WuMs1Zv/ekg= Received: from MWHPR11MB2032.namprd11.prod.outlook.com (10.169.237.141) by MW3PR11MB4650.namprd11.prod.outlook.com (20.181.53.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3912.19; Fri, 5 Mar 2021 18:44:47 +0000 Received: from MWHPR11MB2032.namprd11.prod.outlook.com ([fe80::30c2:967:f635:def2]) by MWHPR11MB2032.namprd11.prod.outlook.com ([fe80::30c2:967:f635:def2%5]) with mapi id 15.20.3912.025; Fri, 5 Mar 2021 18:44:47 +0000 From: "Dumitrescu, Cristian" To: Matan Azrad , Li Zhang , Dekel Peled , Ori Kam , Slava Ovsiienko CC: "dev@dpdk.org" , NBU-Contact-Thomas Monjalon , Raslan Darawsheh , "mb@smartsharesystems.com" , "ajit.khaparde@broadcom.com" , "Yigit, Ferruh" , "Singh, Jasvinder" Thread-Topic: [dpdk-dev] [RFC v4 1/4] ethdev: add meter PPS profile Thread-Index: AQHXDoa2zb8ApkK+UkWjVZeNoHieI6pvHMqQgAEqKQCAAFbtcIAABpIAgAAQKdCAAEzhAIABsKEAgACxowCAAlt88A== Date: Fri, 5 Mar 2021 18:44:47 +0000 Message-ID: References: <20210125010235.1768333-1-lizh@nvidia.com> <20210301103532.184983-1-lizh@nvidia.com> <20210301103532.184983-2-lizh@nvidia.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.5.1.3 dlp-reaction: no-action dlp-product: dlpe-windows authentication-results: nvidia.com; dkim=none (message not signed) header.d=none;nvidia.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [109.78.54.76] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 412515ba-7a21-479a-2aff-08d8e006c099 x-ms-traffictypediagnostic: MW3PR11MB4650: 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: zc0B2HobXuw+ox7R4sBiwXkdmyJHudB3kiQITuWiOsobI9tLoyKkBp0RPKPc7ERvXYGQfa16PkLEWnjsZyZjnh45nnkL1YmHs+OfCZNbw/Vjkg/fvDbRuLGP8AqfgCRr+NFNtdvLIADic4VHSATpyhY036fuEUWOfhMtov0xUF0YaJ2Tp9czvq63Twe6xl1/7Kocr4XQphzITu8fYHxR7MrbYEjK6UVWRKOJJXYtMOZ8R+bfeonQ5cxx5BAWFFycgINbmOdyEAo2KltWEaCfPo/RUN8Bf45cbN3zLEv56k42Fa4/diEpp5n4WzVinpKM+Aq3cLF89JGSwEwu1yMgEw7/L1gacK56FoC+BKDZkeeksQDAT5hVnZ2Zcdlqh1mWOpCnMbbzna8w3MVKUlKqoA== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR11MB2032.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(376002)(346002)(366004)(136003)(396003)(39860400002)(186003)(54906003)(33656002)(71200400001)(8676002)(478600001)(52536014)(53546011)(86362001)(66476007)(110136005)(316002)(4326008)(55016002)(107886003)(7416002)(26005)(5660300002)(30864003)(2906002)(9686003)(8936002)(64756008)(76116006)(6506007)(66556008)(66946007)(7696005)(66446008)(83380400001); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?AHnZuDtUKfNtdEyMyTRZvHxPP5O2vF0fepTvHQkHhRNbHbHuLhnVMy3hdDFS?= =?us-ascii?Q?e8DLuq593sipszxRBTAZ5Xn20kQmxMzQWhu8KkWViiJBFN8UupADT3fjP7P5?= =?us-ascii?Q?BL3uita9nsJ2uzYh+r8yUYLa9+a++vUr3t4d/jAC2w+1I+p2cSB+jhnAYd3b?= =?us-ascii?Q?lajzvLc4hi4VeZayyUQqIJ5Oq/FlzrI8DDXxiVsb1ob+3rtMRzXzhbUfSbtX?= =?us-ascii?Q?ikSkdnLst/GCCVEspPJfoK0seNbVKZ+vUDV8uN8l6R0pYz3Y+Oxu5U+Ok7Ac?= =?us-ascii?Q?dkKMXOJDB7Ryp0xfWasGbugUM4M9Bps0/ew4odn9SEQNaIaqhKH49ktg1NZG?= =?us-ascii?Q?3DIQFrJc5BYKxLWDTmgY87roZ0UxzCHAaf33JRCNRkRyJVWtjIOx+LpEN+CA?= =?us-ascii?Q?BlXlA2WcyXlSh3sYVFwXXwr5tLaQDu0YhTRAIovpRQNxUYv1S7kCnoMCBIPT?= =?us-ascii?Q?3jPMOBVwZSMhMMBXhPw3IvPEw7cVoDekV7ZhM9wkZNQIHPjBSCZX5/yI7KGw?= =?us-ascii?Q?zr8taS7EYc3R/zQAbSCYt6Kc72Pchzp+HgE0r8LgBKaUGvZS9usvuv4+NFgs?= =?us-ascii?Q?MsxYeeG1H+1FFlqOHrvVqs5MU/fDnghJCsQCQ0AU7SvFQx5NXNC2yCG7331T?= =?us-ascii?Q?xdd7toTH0b9a7kZkY5RFIugLSPOZ1Xwsfxi89+q92YHtkCOOHOcIjCjijwEm?= =?us-ascii?Q?p3L1Au61m+cR2Oo3Wn4ZRf4e9u/TN8DU+b740e/J73GcDJ9Tigx1N4ht2QQA?= =?us-ascii?Q?2RCqdu1cevEzFOc2nbNRc3muPyqpgWdLNnal8bhJ2h6IzP9ZXD8rIoKYB2rZ?= =?us-ascii?Q?6dkxnUNFLAO0/20aIUUErcoZ4S9MlG9wdaoQa6+lbN+Wpf3sA9bulS2o6rJa?= =?us-ascii?Q?9jNT20qzWI+Q99SKdO0Ax1PA2pYQ0KRBmA1w4gPwtdy4Gqjk9qB1lRKz2jmr?= =?us-ascii?Q?dP7akv4ORs7pQdtjpd/mNrGtjWc7CQLV7O1O+PPwWUJ0LfyJQ4b3h3xykj9A?= =?us-ascii?Q?MqxWqXjzYj3pu8bV93X6jU7pRa4gK8CtlasLQfyCXQlDKGZZCM+foqb3f8EZ?= =?us-ascii?Q?tvx42Xz1XUMfdB8Gzdgzmr7W4XK2N82wywCmy1uM/U+d3w6i/ZobIji84NqL?= =?us-ascii?Q?qypMGICaPucAP/vnC3PPcgzgflV/six6JbH2PoUhCBeKYCw6XRsZ++YoFIvs?= =?us-ascii?Q?4ZsGVZVK+sokzIeR+9kZMmDlIx5G4N7JcQBc344eANl08RAcN8oYRJaNaAam?= =?us-ascii?Q?UTLyiEfwCMqWWbSi4ckBUh1qkcIIlewdALZE8x28MtsiCYDGR70+17F0eUIY?= =?us-ascii?Q?SCk=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: MWHPR11MB2032.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 412515ba-7a21-479a-2aff-08d8e006c099 X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Mar 2021 18:44:47.7184 (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: hPI6wwvzGR2JzSUaRxCr9uvVGOPRUoGlU9BBRy62YjTNa/tt8i2bpvTd6BO6whwduuEJ+6BU8X5HJyDzsiwGjGEzVMyYL8eq7F0Hgmt7JKE= X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4650 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [RFC v4 1/4] ethdev: add meter PPS profile X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" > -----Original Message----- > From: Matan Azrad > Sent: Thursday, March 4, 2021 6:34 AM > To: Dumitrescu, Cristian ; Li Zhang > ; Dekel Peled ; Ori Kam > ; Slava Ovsiienko > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon > ; Raslan Darawsheh ; > mb@smartsharesystems.com; ajit.khaparde@broadcom.com; Yigit, Ferruh > ; Singh, Jasvinder > Subject: RE: [dpdk-dev] [RFC v4 1/4] ethdev: add meter PPS profile >=20 > Hi Cristian >=20 > From: Dumitrescu, Cristian > > Hi Matan, > > > > > -----Original Message----- > > > From: Matan Azrad > > > Sent: Tuesday, March 2, 2021 6:10 PM > > > To: Dumitrescu, Cristian ; Li Zhang > > > ; Dekel Peled ; Ori Kam > > > ; Slava Ovsiienko > > > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon > ; > > > Raslan Darawsheh ; mb@smartsharesystems.com; > > > ajit.khaparde@broadcom.com; Yigit, Ferruh ; > > > Singh, Jasvinder > > > Subject: RE: [dpdk-dev] [RFC v4 1/4] ethdev: add meter PPS profile > > > > > > Hi Cristian > > > > > > Good discussion, thank you for that! > > > > > > From: Dumitrescu, Cristian > > > > Hi Matan, > > > > > > > > > -----Original Message----- > > > > > From: Matan Azrad > > > > > Sent: Tuesday, March 2, 2021 12:37 PM > > > > > To: Dumitrescu, Cristian ; Li Zhan= g > > > > > ; Dekel Peled ; Ori Kam > > > > > ; Slava Ovsiienko > > > > > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon > > > ; > > > > > Raslan Darawsheh ; > mb@smartsharesystems.com; > > > > > ajit.khaparde@broadcom.com; Yigit, Ferruh > > > > > ; Singh, Jasvinder > > > > > > > > > > Subject: RE: [dpdk-dev] [RFC v4 1/4] ethdev: add meter PPS profil= e > > > > > > > > > > HI Cristian > > > > > > > > > > From: Dumitrescu, Cristian > > > > > > Hi Matan, > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Matan Azrad > > > > > > > Sent: Tuesday, March 2, 2021 7:02 AM > > > > > > > To: Dumitrescu, Cristian ; Li > > > > > > > Zhang ; Dekel Peled ; Ori > > > > > > > Kam ; Slava Ovsiienko > > > > > > > > > > > > > > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon > > > > > ; > > > > > > > Raslan Darawsheh ; > > > mb@smartsharesystems.com; > > > > > > > ajit.khaparde@broadcom.com; Yigit, Ferruh > > > > > > > ; Singh, Jasvinder > > > > > > > > > > > > > > Subject: RE: [dpdk-dev] [RFC v4 1/4] ethdev: add meter PPS > > > > > > > profile > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Cristian > > > > > > > > > > > > > > Thank you for review, please see inline. > > > > > > > > > > > > > > From: Dumitrescu, Cristian > > > > > > > > > From: dev On Behalf Of Li Zhang > > > > > > > > > > > > > > > We had this same problem earlier for the rte_tm.h API, wher= e > > > > > > > > people > > > > > > > asked to > > > > > > > > add support for WRED and shaper rates specified in packets > > > > > > > > to the existing > > > > > > > byte > > > > > > > > rate support. I am more than happy to support adding the > > > > > > > > same here, but please let's adopt the same solution here > > > > > > > > rather than invent a different approach. > > > > > > > > > > > > > > > > Please refer to struct rte_tm_wred_params and struct > > > > > > > rte_tm_shaper_params > > > > > > > > from rte_tm.h: the packets vs. bytes mode is explicitly > > > > > > > > specified through > > > > > > > the use > > > > > > > > of a flag called packet_mode that is added to the WRED and > > > > > > > > shaper > > > > > profile. > > > > > > > > When packet_mode is 0, the profile rates and bucket sizes > > > > > > > > are specified in bytes per second and bytes, respectively; > > > > > > > > when packet_mode is not 0, the profile rates and bucket > > > > > > > > sizes are specified in packets and packets per > > > > > > > second, > > > > > > > > respectively. The same profile parameters are used, no need > > > > > > > > to invent additional algorithms (such as srTCM - packet > > > > > > > > mode) or profile data > > > > > > > structures. > > > > > > > > Can we do the same here, please? > > > > > > > > > > > > > > This flag approach is very intuitive suggestion and it has > advantages. > > > > > > > > > > > > > > The main problem with the flag approach is that it breaks ABI= and > API. > > > > > > > The profile structure size is changed due to a new field - AB= I > > > breakage. > > > > > > > The user must initialize the flag with zero to get old > > > > > > > behavior - API > > > > > breakage. > > > > > > > > > > > > > > > > > > > The rte_mtr API is experimental, all the API functions are > > > > > > correctly marked with __rte_experimental in rte_mtr.h file, so > > > > > > we can safely change the API > > > > > and > > > > > > the ABI breakage is not applicable here. Therefore, this proble= m > > > > > > does not > > > > > exist, > > > > > > correct? > > > > > > > > > > Yes, but still meter is not new API and I know that a lot of user > > > > > uses it for a long time. > > > > > Forcing them to change while we have good solution that don't > > > > > force it, looks me problematic. > > > > > > > > > > > > > Not really, only 3 drivers are currently implementing this API. > > > > > > The user is not the PMD, the PMDs are the providers. > > > I'm talking about all our customers, all the current DPDK based > > > applications like OVS and others (I familiar with at least 4 ConnectX > > > customer applications) which use the meter API and I'm sure there are > more > > around the world. > > > > > > > Even to these drivers, the required changes are none or extremely > small: > > > as Ajit > > > > was also noting, as the default value of 0 continues to represent > > > > the > > > existing > > > > byte mode, all you have to do is make sure the new flag is set to > > > > zero in the profile params structure, which is already done > > > > implicitly in most places as > > > this > > > > structure is initialized to all-zeros. > > > > > > Are you sure all the world initialize the struct to 0? and also in > > > this case, without new compilation, not all the struct will be > > > zeroes(the old size is smaller). > > > > > > > A simple search exercise for struct rte_mtr_meter_profile is all > > > > that is > > > needed. > > > > You also agreed the flag approach is very intuitive, hence better > > > > and nicer, > > > with > > > > no additional work needed for you, so why not do it? > > > > > > Do you understand that any current application that use the meter API > > > must recompile the code of the application? Part of them also probabl= y > > > need to set the flag to 0.... > > > Do you understand also the potential issues for the applications whic= h > > > are not aware to the change? Debug time, etc.... > > > > > > > > > > I don't see issues with Li suggestion, Do you think Li > > > > > > > suggestion has critical issues? > > > > > > > > > > > > It is probably better to keep the rte_mtr and the rte_tm APIs > > > > > > aligned, it simplifies the code maintenance and improves the > > > > > > user experience, which always pays off in the long run. Both > > > > > > APIs configure token buckets in either packet mode or byte mode= , > > > > > > and it is desirable to have them work in the > > > > > same > > > > > > way. Also, I think we should avoid duplicating configuration > > > > > > data structures > > > > > for > > > > > > to support essentially the same algorithms (such as srTCM or > > > > > > trTCM) if we > > > > > can. > > > > > > > > > > > > > > > > Yes, but I don't think this motivation is critical. > > > > > > > > I really disagree. As API maintainer, making every effort to keep > > > > the APIs > > > clear > > > > and consistent is a critical task for me. > > > > > > New pps profile is also clear and simple. > > > > > > > We don't want to proliferate the API data structures and parameters > > > > if there is a good way to avoid it. Especially > > > in > > > > cases like this, when the drivers are just beginning to pick up thi= s > > > > (still > > > > experimental) API, we have the rare chance to make things right an= d > > > therefore > > > > we should do it. Please also keep in mind that, as more feature are > > > > added > > > to > > > > the API, small corner cuts like this one that might not look like a > > > > big deal > > > now, > > > > eventually come back as unnecessary complexity in the drivers > themselves. > > > > > > I don't see a complexity in the current suggestion. > > > > > > > So, please, let's try to keep the quality of the APIs high. > > > > > > Also by this way is high. > > > > > > > > > Look, the flag approach is also good and makes the job. > > > The two approaches are clear, simple and in high quality. > > > I don't care which one from the 2 to take but I want to be sure we ar= e > > > all understand the pros and cons. > > > > > > If you understand my concern on flag approach and still insist to tak= e > > > the flag approach we will align. > > > > Yes, thanks for summarizing the pros and cons, I confirm that I do > understand > > your concerns. > > > > Yes, sorry to disappoint you, I still think the packet_mode based appro= ach > is > > better for the long run, as it keeps the APIs clean and consistent. We = are > not > > adding new algorithms here, we're just adding a new mode to an existing > > algorithm, so IMO we should not duplicate configuration data structures > and > > proliferate the number of algorithms artificially. >=20 > Actually, PPS meter is a new algorithm - you can see that current algorit= hms > RFCs don't talk about PPS. >=20 Yes, I know, I implemented it in librte_meter, but still, it is the same al= gorithm with just a different measurement unit (packet instead of byte), th= at's why many people (and you included :) ) still refer to it as srTCM - R= FC 2697. > > Yes, I do realize that in some limited cases the users will have to exp= licitly > set > > the new packet_mode flag to zero or one, in case they need to enable th= e > > packet mode, but I think this is an acceptable cost because: (A) This A= PI is > > clearly marked as experimental; (B) It is better to take a small increm= ental > hit > > now to keep the APIs in good order rather than taking a bit hit in a fe= w > years as > > more features are added in the wrong way and the APIs become > > unmanageable. >=20 > I don't think that the current suggestion is in wrong way. > In any case, you insist, we will align. >=20 Thank you. > > > And if we so, and we are going to break the API\ABI, we are going to > > > introduce new meter policy API soon and there, breaking API can help, > > > lets see in other discussion later. > > > > > > > Yes, as you point out API changes are unavoidable as new features are > added, > > we have to manage the API evolution correctly. > > > > > One more point: > > > Currently, the meter_id is managed by the user, I think it is better > > > to let the PMDs to manage the meter_id. > > > > > > Searching the PMD meter handler inside the PMD is very expensive for > > > the API call rate when the meter_id is managed by the user. > > > > > > Same for profile_id. > > > > > > Also all the rte_flow API including the shared action API taking the > > > PMD management approach. > > > > > > What do you think? > > > > > > > Yes, we have carefully considered and discussed both approaches a few > years > > back when the API was introduced, this is not done by accident :), ther= e are > > pros and cons for each of them. > > > > If the object IDs are generated by the driver (outputs of the API), the= n it is > the > > user application that needs to keep track of them, which can be very > painful. > > Basically, for each API object the user application needs to create its= own > > wrapper to store this ID. We basically transfer this problem to the use= r app. >=20 > No exactly\not for all, the app gets the meter_id in the same time it dec= ides > it now. >=20 > > If the object IDs are generated by the user application (inputs into th= e API), > > then we simplify the application by removing and indirection layer. Yes= , it is > > true that this indirection layer now moves into the driver, but we shou= ld try > to > > make the life easier for the appl developers as opposed to us, the driv= er > > developers. This indirection layer in the driver can be made a bit smar= ter > than > > just a slow "for" loop; the search operation can be made faster with a = small > bit > > of effort, such as keeping this list sorted based on the object ID, spl= itting > this list > > into buckets (similar to a hash table), etc, right? >=20 > Yes, there are even better solution than hash table from "rate" perspecti= ve. >=20 I'd be very interested to hear your proposals here. > But any solution costs a lot of memory just for this mapping... > When we talked about 4M meters supported(in mlx5 next release) it > becomes an issue. >=20 I thought your concern was about the speed/rate of API calls, you are sayin= g it is not speed but memory footprint?? I would imagine that a system that enables all the 4M meters is a big beast= with the most powerful CPU on the planet and many dozens of gigabytes of R= AM, so a few extra megabytes for some API layers is not a concern? > > Having the user app provide the object ID is especially important in th= e case > of > > rte_tm API, where we have to deal with a tree of nodes, with thousands = of > > nodes for each level. Having the app to store and manages this tree of = IDs > is a > > really bad idea, as the user app needs to mirror the tree of nodes on i= ts > side for > > no real benefit. As an added benefit, the user can generate these IDs u= sing > a > > rule, such as: given the specific path through the tree, the value of t= he ID > can > > be computed. >=20 > rte_tm is not rte_mtr - I think meter is different and used differently. > For example, as I know, no one from our dpdk meter customers(at least 5) > use TREEs for meter management. OVS, for example, just randomize some > meter_id and don't care about it. >=20 What kinds of trees? I'd be very interested to hear some proposals to make = this handle mapping faster. > Also, all the rte_flow API basics works with PMD ID\handle management > approach. >=20 Yes, I am not saying it is wrong, none of the approaches is wrong IMO. > > But again, as you also mention above, there is a list of pros and cons = for > every > > approach, no approach is perfect. We took this approach for the good > reasons > > listed above. >=20 > If you familiar with TREE usage with meter, maybe we can combined easily > the two approaches in this topic, >=20 > meter_id argument can be by reference, if it 0 - PMD set it, if not PMD u= se it. >=20 It would be good if you could elaborate here a bit, just to make sure we ar= e on the same page. > > > > > > The flag proposal is actually reducing the amount of work that > > > > > > you guys > > > > > need to > > > > > > do to implement your proposal. There is no negative impact to > > > > > > your > > > > > proposal > > > > > > and no big change, right? > > > > > > > > > > Yes you right, but the implementation effect is not our concern. > > > > > > > > > > > > > > > > > > This is a quick summary of the required API changes to add > > > > > > > > support for the packet mode, they are minimal: > > > > > > > > a) Introduce the packet_mode flag in the profile parameters > > > > > > > > data > > > > > > > structure. > > > > > > > > b) Change the description (comment) of the rate and bucket > > > > > > > > size > > > > > > > parameters in > > > > > > > > the meter profile parameters data structures to reflect tha= t > > > > > > > > their values represents either bytes or packets, depending > > > > > > > > on the value of the new flag packet_mode from the same > structure. > > > > > > > > c) Add the relevant capabilities: just search for "packet" > > > > > > > > in the rte_tm.h capabilities data structures and apply the > > > > > > > > same to the rte_mtr.h > > > > > > > capabilities, > > > > > > > > when applicable. > > > > > > > > > > > > > > > Regards, > > > > > > > > Cristian > > > > > > > > > > > > Regards, > > > > > > Cristian > > > > > > > > Regards, > > > > Cristian > > > > Regards, > > Cristian Regards, Cristian