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 C8325A0561; Thu, 4 Mar 2021 07:34:33 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5F4F240684; Thu, 4 Mar 2021 07:34:33 +0100 (CET) Received: from hqnvemgate25.nvidia.com (hqnvemgate25.nvidia.com [216.228.121.64]) by mails.dpdk.org (Postfix) with ESMTP id CA57F40147 for ; Thu, 4 Mar 2021 07:34:31 +0100 (CET) Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate25.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Wed, 03 Mar 2021 22:34:30 -0800 Received: from HQMAIL105.nvidia.com ([172.20.161.6]) by hqpgpgate101.nvidia.com (PGP Universal service); Wed, 03 Mar 2021 22:34:30 -0800 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Wed, 03 Mar 2021 22:34:30 -0800 Received: from HQMAIL101.nvidia.com (172.20.187.10) by HQMAIL105.nvidia.com (172.20.187.12) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Thu, 4 Mar 2021 06:34:30 +0000 Received: from HQMAIL111.nvidia.com (172.20.187.18) by HQMAIL101.nvidia.com (172.20.187.10) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Thu, 4 Mar 2021 06:34:20 +0000 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.170) by HQMAIL111.nvidia.com (172.20.187.18) with Microsoft SMTP Server (TLS) id 15.0.1497.2 via Frontend Transport; Thu, 4 Mar 2021 06:34:20 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CLYHahB+FczSXjDh70DwKFBfBZx2QTRN1VW76ER7NSVxCMAOyDJbWCTXJC5PzzUgm+jpQgISphPSP6pL/w4PzUFJvenupmnaIIoOkgkL77PHGGbVxZZhLcwTTbIiP1ZkECKZLxffLe8+7f9ZD4lZbAdfcEh8qbR8r1h3GOJ8GDICxU5SxVpMEXmSdDsu/cGSGCQWZofXpM6+eo0myrc3K2HANzxhERH0g4tj58Gy/0jvNPhsdfBaRjq/IkHpO1cLUFcR1OboDOHL107gsZTVMDbstnqyfiV8iPYbHLZv8KZX/TIGfnNasIcgdXPYpqdijot2P5sOG4dEbF05e9er4Q== 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=5P1rFLwupaJfocQ2xJSraPtMgfBME1zYjFK1YcGgFak=; b=T8flkGAB5fT4RZHyVh7J95S1nBTCzag7fHBQE0OoCXZcDk5CRYOa0IT+mUMiZHAPu86xN0lpqs95tfwtPlyXrKA5kNIddJAWqhsNnN8fr0zbEpcBsLHLFm2U35xMt/U40vw1ijMWOnetIi3u4D5LvxMBcp5iJujDNZ2HU82QgKw4auqhRqlgzEoEwzvmbtDwzpddRjlHkoCpQl2Fb5x1Dzi3XqYXCGeF6tvagMUB2TyiOiWZZvtxj9rUBRQVeT/JArwjcNiWUJTUUTBGSq+ZfKjg/+cXPQA2/VXJxpP4/4m02qYF65GhBSIdl5fO0cpi8dA89uH6PR1yMIIczzovog== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none Received: from MW2PR12MB2492.namprd12.prod.outlook.com (2603:10b6:907:8::19) by MWHPR12MB1837.namprd12.prod.outlook.com (2603:10b6:300:113::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3912.17; Thu, 4 Mar 2021 06:34:18 +0000 Received: from MW2PR12MB2492.namprd12.prod.outlook.com ([fe80::99f2:8567:2f9e:c351]) by MW2PR12MB2492.namprd12.prod.outlook.com ([fe80::99f2:8567:2f9e:c351%4]) with mapi id 15.20.3890.028; Thu, 4 Mar 2021 06:34:18 +0000 From: Matan Azrad 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" Thread-Topic: [dpdk-dev] [RFC v4 1/4] ethdev: add meter PPS profile Thread-Index: AQHXDoaku6PLW53KtUOtQaDT1ovmy6pvHh4AgAEiL2CAAGH8AIAAAHAwgAAiHQCAADFYsIABxj+AgACd3SA= Date: Thu, 4 Mar 2021 06:34:18 +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: authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=nvidia.com; x-originating-ip: [216.228.117.191] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: d6974803-8ec1-4115-ace8-08d8ded789e9 x-ms-traffictypediagnostic: MWHPR12MB1837: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-header: ProcessedBy-CMR-outbound x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Fn+xBN7QIfHFHiFrROkaOdGv6KqJVp5ewC8OMLT71nNXBsKAlr/yRNd2qfenRLBKlE2L4zrh+BTVUZNKqrxwIMBwdRoPGOI2HRgnRMfUprNw3hFjr4kHJNYnPBEefC9S6PrZ7dvsUfikyke4AM6u4H5rGUDmmtJht84mNQfa3lsXZUXA735460kdOMA0A4Kk5xHQF6cmYBFPGuOtdxFQDOgY8LaSOh0O3tgjCzbDnEJWTZP7BSAUuU8j8mINf6g9PwIO6OOu5jHPUni9v5Ci31b4ufqAqDSPgqUXhE6lFzA29bnHzpmPQmfF4jwXxaTtN7xqTLfPkZrmyvM8r/EodsKiyvZ27jkvtAvnUOcCz0NHHvOp991JMWbjNnSC3QNR/wUJ4sQ+15hufAjl58P+/00s/2a3/ICSyKoo0pu2zY/8+cT4YcZ/lKoyH/UlW7x/AkcRrYJA57BMTq2ZZaGvKE+u7vj3XGW7ygHpafDmhJkHS2F3ELW6Ipiivbp1Zvw/LJz1dAisJALACDln+CR8ag== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MW2PR12MB2492.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(376002)(366004)(136003)(346002)(396003)(39860400002)(5660300002)(9686003)(30864003)(316002)(478600001)(52536014)(2906002)(53546011)(6506007)(33656002)(55016002)(186003)(26005)(64756008)(66556008)(66476007)(66446008)(76116006)(66946007)(54906003)(8676002)(86362001)(7696005)(8936002)(6636002)(110136005)(71200400001)(83380400001)(4326008); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?0prIDFCdf10LtueUNCI46ipoj26qRUQgI4Vwizs5W9WMPtmq6xuZy0yrJgXo?= =?us-ascii?Q?6hzJlcDv9St3eV+FMo5Bj1ndD2MgOshzvAAFy02rgh6SpuqFp5YecbS9a8JX?= =?us-ascii?Q?T5i5zkAXSzmkeToiFcAK7JZAo+v5HVcdQDVYWKLdUu8bh6nRzu/iPE3kmKAt?= =?us-ascii?Q?HoPsoUX02kW8dXoy/R61KsHxWh5K3gHEEe0kV9Bd+QqS80jAXjrnj/RK7IUV?= =?us-ascii?Q?7mVvQjvm84kuF5UTf3IQxqnYXUwDfoSpm22eeRFP9EUnE/9/Z/ua5HWfBZmn?= =?us-ascii?Q?ho/kZ9lyNWcJ1/FiDlbBKOHnZyKtWoboXJfXYkCe8JJdhvuLFHyQfKnuAZVi?= =?us-ascii?Q?zkWZnynQ2BacI96UhPwsCazoOKLSPAmft76OCVb1oyt+ddoWXHmOtHnc+/eR?= =?us-ascii?Q?zCiewwLLXA5BM0qWnHII+F/rl+XkILFGSxqhb/T83DCflEkhRDWqddYB78jd?= =?us-ascii?Q?joCiO5TfrZBH849SY1c/scPkIhqaf1u2WVN+osep7F8OddDLgjo2B2OZ4iat?= =?us-ascii?Q?LufmlKs2MEz12OOMewY+JZQ6kRC72dok2ITNcW2VEOI6qRJqCHRWJukbgpHd?= =?us-ascii?Q?iLhJHS5SAyPT1R2T/nlQYTYPgqizxNyG3QCCV7j9o5/+9DqMjSMVXdjsL3Wl?= =?us-ascii?Q?CRPjJCPtjlU3L4Dk5e9nRbNM/qi4KIVfM1e+GmE+GvoTh4+XCKSld3208GFs?= =?us-ascii?Q?XZU/sgEXUcIbZfNZED3NBoWtZY4zxicdms8uxzVpI1KvZ5B+tpptjKOk6K8m?= =?us-ascii?Q?m1WWcsPSyuiRV9Nno67839pr5VaShFIi1O1bYV0KZ/JN63IL0NWyNpnZZCAC?= =?us-ascii?Q?3Bj/watVKn8D8BvXlD6kDo2zFegcqPJx0Rt3gr7/nHFJ3vcliUDNOnnbaoqK?= =?us-ascii?Q?VlW4uq/W1KvKDZXqTUCRL92+tweMrqMOzc01HUlvf/XqSq6wwiDtNbZ1bWwG?= =?us-ascii?Q?0c+VybRxNDb4d4aXLzID4DJHBBY2kITsHNSLPalqBxGtIZvPrPtrzRs0uZw9?= =?us-ascii?Q?4uNosgEGsO6+QeO4ciPXswiIiMQtgl/5DabKl+yRAmjYTihyREszA39WxE1g?= =?us-ascii?Q?7lAJ93C9Zux55ZUAHEYNn6DJHI3y/Pq3uV+zBa60JFo8tX607x9/YmdkQjmB?= =?us-ascii?Q?lf3qBfmHH4s55hvMf7MQy8bZPjRX0MGJQIDl12dlGHZShLV83mHD6aX43qPP?= =?us-ascii?Q?CntiKX6GT1QFl4Kc7oIJlRkdKiENAgzI6RJUR/TjvporMN/XSXMJGRCEsZJU?= =?us-ascii?Q?Ugjzwa3zgqH1mkZaX+oE7mk3kaPNWoipfi85UzsfaQHj4e2Ys2jfsqoncnjb?= =?us-ascii?Q?WVbQao3v81g2uq33FSvCA/sd?= 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: MW2PR12MB2492.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: d6974803-8ec1-4115-ace8-08d8ded789e9 X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Mar 2021 06:34:18.4343 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: tAh9pvoP33qRJqhv/vpBqNj+qOm5CFDQIs12Uf3oY95ggTx7l2hKM1pCSBco+0l5/9ERp3nsV8tVT/eez1D2lQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR12MB1837 X-OriginatorOrg: Nvidia.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1614839671; bh=5P1rFLwupaJfocQ2xJSraPtMgfBME1zYjFK1YcGgFak=; h=X-PGP-Universal:ARC-Seal:ARC-Message-Signature: ARC-Authentication-Results:From:To:CC:Subject:Thread-Topic: Thread-Index:Date:Message-ID:References:In-Reply-To: Accept-Language:Content-Language:X-MS-Has-Attach: X-MS-TNEF-Correlator:authentication-results:x-originating-ip: x-ms-publictraffictype:x-ms-office365-filtering-correlation-id: x-ms-traffictypediagnostic:x-ld-processed: x-ms-exchange-transport-forked:x-microsoft-antispam-prvs:x-header: x-ms-oob-tlc-oobclassifiers:x-ms-exchange-senderadcheck: x-microsoft-antispam:x-microsoft-antispam-message-info: x-forefront-antispam-report:x-ms-exchange-antispam-messagedata: Content-Type:Content-Transfer-Encoding:MIME-Version: X-MS-Exchange-CrossTenant-AuthAs: X-MS-Exchange-CrossTenant-AuthSource: X-MS-Exchange-CrossTenant-Network-Message-Id: X-MS-Exchange-CrossTenant-originalarrivaltime: X-MS-Exchange-CrossTenant-fromentityheader: X-MS-Exchange-CrossTenant-id:X-MS-Exchange-CrossTenant-mailboxtype: X-MS-Exchange-CrossTenant-userprincipalname: X-MS-Exchange-Transport-CrossTenantHeadersStamped:X-OriginatorOrg; b=fGhdaXW8e+x4VWAYo5KDz/dZiyNq5hkQIoHC0yj6A2myduJzs7mi8pIXU5Pr78Jdo 0JCO0Big8nGBrnfi6CQOJ1fklNO/a4M01L2Ah3Ii9WpbhZV51XX0hNc3SDcke/2LDb kKLRjfxAlswr9Yvp5M4Hgn3eeWgtSwJpxZJFqdHkIvZHVpr0tB4UCYIKj2M1E+7Qw3 o9p6skqgfdJlcJfP8lMdlv5b/31jS4iMD48l+cGAqKPYlPSuyDZMWlTF9fv1hvmc7w rQDMlGamhln3oR8DC0cB/wqlnEKtWg5Jn1yA3yMfMj6KnaZ0AT4m23FFs8TCyme9ax r/uY5xrqzsCvg== 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" Hi Cristian From: Dumitrescu, Cristian > Hi Matan, >=20 > > -----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 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 > > > > > > > > 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, where > > > > > > > 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 adva= ntages. > > > > > > > > > > > > The main problem with the flag approach is that it breaks ABI a= nd API. > > > > > > The profile structure size is changed due to a new field - ABI > > 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 problem > > > > > 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 m= ore > around the world. > > > > > Even to these drivers, the required changes are none or extremely sma= ll: > > 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 probably > > need to set the flag to 0.... > > Do you understand also the potential issues for the applications which > > 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 this > > > (still > > > experimental) API, we have the rare chance to make things right and > > 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 themsel= ves. > > > > 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 are > > all understand the pros and cons. > > > > If you understand my concern on flag approach and still insist to take > > the flag approach we will align. >=20 > Yes, thanks for summarizing the pros and cons, I confirm that I do unders= tand > your concerns. >=20 > Yes, sorry to disappoint you, I still think the packet_mode based approac= h is > better for the long run, as it keeps the APIs clean and consistent. We ar= e 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 a= nd > proliferate the number of algorithms artificially. Actually, PPS meter is a new algorithm - you can see that current algorithm= s RFCs don't talk about PPS. > Yes, I do realize that in some limited cases the users will have to expli= citly set > the new packet_mode flag to zero or one, in case they need to enable the > packet mode, but I think this is an acceptable cost because: (A) This API= is > clearly marked as experimental; (B) It is better to take a small incremen= tal hit > now to keep the APIs in good order rather than taking a bit hit in a few = years as > more features are added in the wrong way and the APIs become > unmanageable. I don't think that the current suggestion is in wrong way. In any case, you insist, we will align. > > 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. > > >=20 > Yes, as you point out API changes are unavoidable as new features are add= ed, > we have to manage the API evolution correctly. >=20 > > 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? > > >=20 > Yes, we have carefully considered and discussed both approaches a few yea= rs > back when the API was introduced, this is not done by accident :), there = are > pros and cons for each of them. >=20 > If the object IDs are generated by the driver (outputs of the API), then = it is the > user application that needs to keep track of them, which can be very pain= ful. > Basically, for each API object the user application needs to create its o= wn > wrapper to store this ID. We basically transfer this problem to the user = app. No exactly\not for all, the app gets the meter_id in the same time it decid= es it now. =20 > If the object IDs are generated by the user application (inputs into the = 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 should= try to > make the life easier for the appl developers as opposed to us, the driver > developers. This indirection layer in the driver can be made a bit smarte= r than > just a slow "for" loop; the search operation can be made faster with a sm= all bit > of effort, such as keeping this list sorted based on the object ID, split= ting this list > into buckets (similar to a hash table), etc, right? Yes, there are even better solution than hash table from "rate" perspective= . 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 a= n issue. =20 =20 > Having the user app provide the object ID is especially important in the = 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 ID= s is a > really bad idea, as the user app needs to mirror the tree of nodes on its= side for > no real benefit. As an added benefit, the user can generate these IDs usi= ng a > rule, such as: given the specific path through the tree, the value of the= ID can > be computed. 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) us= e TREEs for meter management. OVS, for example, just randomize some meter_i= d and don't care about it. Also, all the rte_flow API basics works with PMD ID\handle management appro= ach. > But again, as you also mention above, there is a list of pros and cons fo= r every > approach, no approach is perfect. We took this approach for the good reas= ons > listed above. If you familiar with TREE usage with meter, maybe we can combined easily th= e two approaches in this topic, meter_id argument can be by reference, if it 0 - PMD set it, if not PMD use= it. > > > > > 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 that > > > > > > > their values represents either bytes or packets, depending > > > > > > > on the value of the new flag packet_mode from the same struct= ure. > > > > > > > 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 >=20 > Regards, > Cristian