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 00763A054F; Tue, 2 Mar 2021 15:33:08 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B352922A26C; Tue, 2 Mar 2021 15:33:07 +0100 (CET) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mails.dpdk.org (Postfix) with ESMTP id 77A3B40142 for ; Tue, 2 Mar 2021 15:33:06 +0100 (CET) IronPort-SDR: P9VNzvXZwRnknLU2+SU/Sn/GLRL44lb4NC1vShLKtrbreS7vApro7dpbdZRiiOp/4qqmvNZ603 Sa6qyql/NbrA== X-IronPort-AV: E=McAfee;i="6000,8403,9910"; a="173969192" X-IronPort-AV: E=Sophos;i="5.81,216,1610438400"; d="scan'208";a="173969192" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Mar 2021 06:33:04 -0800 IronPort-SDR: UR6vcbPWSlhFZUT/6fJWAwdvIbOMHiK0WR94Hwxs9j1Z/BICFSmMho7rcJjsgWClGQavXbYQWO JyWCEYHh5pzw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,216,1610438400"; d="scan'208";a="373606345" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by fmsmga007.fm.intel.com with ESMTP; 02 Mar 2021 06:33:04 -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; Tue, 2 Mar 2021 06:33:03 -0800 Received: from orsmsx603.amr.corp.intel.com (10.22.229.16) 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; Tue, 2 Mar 2021 06:33:03 -0800 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) 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; Tue, 2 Mar 2021 06:33:03 -0800 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.44) 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.2106.2; Tue, 2 Mar 2021 06:33:03 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SXpmw9n5gchF5mRsjFe5MrvNEJ/LIFJ1wZNZChSEoM/rtq2exwQKpf6zjOBJ+siXG7ZEWIBnAtZvWOX7ApLEyalYbL4h3nNkngaTHe0jLKFmxwcCtKQXHOCXKQqouxsDmMAgju/3rFIYnC///ccaaoI5ngC320MRuVUhl4rvhNlukPkgoBDnId6fAYjBS9Cx6E0WKsVCNEBL7OpOBdRxQz1vK58YEkUIap08Fbl/YulblslkePO/QCbsj9aoeb+exQtrbVIiagKMc/1onnoMyhfmKLCQBHUkVTrdGVCji4P1VSzxzr7RDgntmy1wn/Zsi+0CEeRi4RR/2OKqFonAXw== 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=7jqJlRgTtRTEuR2ZTTLhj/bA4Z112d0tI8e1PiacWjk=; b=OyGZYoG31qbsG2YZHgnFMIo6Qwl3VaKiVg0zQUr1Kjvq3+nScka9UdEODde94LrHikvvhBZIcG4Ds3P4XD/ZVfmwTspASRF+qpR3q09EJUGDjucPpQihlOIo2CQe/7JTpokTXZ1P9A/61mM/FNQL0miI8Ofi5x1FpZpnVgmAJ/+rqaV8hCBJYqHV+ldV70TZE7nHWOUrSSan3URrNrtdoN57vtLhGz3ungYaDs+i8MjBMgEFWlN51eHEyVRhU7W+3RPiqyfH96EEBDkXQuTZGS2PjsPQAd/t3veNr0Gcg2870ijIJWZzFSXVRjPqIkzbE06ImVkJJsbEXxPkK2owLA== 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=7jqJlRgTtRTEuR2ZTTLhj/bA4Z112d0tI8e1PiacWjk=; b=XBBFz+x7IDLqUFRKrKA5SqDAurDs6dytLLxW7AJIeOFAlp8WirT9ApaxLuIjXH+a3ng1Xjd9HbKKVsZ+cukwQNF83jdzVj7RPWozaxeUZdw/MNgWaSePriWKdyYh59UXe1o5NpzGXxwCSZn4+bMHH825hAaknN0W1uLLn/E0ayk= Received: from MWHPR11MB2032.namprd11.prod.outlook.com (2603:10b6:300:2b::13) by MWHPR1101MB2238.namprd11.prod.outlook.com (2603:10b6:301:55::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3890.23; Tue, 2 Mar 2021 14:33:01 +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.3890.029; Tue, 2 Mar 2021 14:33:01 +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+UkWjVZeNoHieI6pvHMqQgAEqKQCAAFbtcIAABpIAgAAQKdA= Date: Tue, 2 Mar 2021 14:33:00 +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: 76f18c54-bd79-4080-dba6-08d8dd881529 x-ms-traffictypediagnostic: MWHPR1101MB2238: 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:9508; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: OqFM5g2baa/igQIQnc6eDk0Nvl705RD3XWUeEMbFWOyrmT+mCxE0CwZIQU2Gh35K8F4hB798WfmpTaBzVerBFw0edYPpaCFHJimhRZlhDGUYqsf63jB3Pf7ZoXEHEvBpsU4FL6Gvb0tPs6xUuRq11YznjonKh/8KaAOzkoooABy0WEO1R/r0DKT0/G63Rfv9B0KreLEIZH3jLJKo0Iy8SbO0O/vF8bHbzgmiCgtq4XB+nxVznQKZ4OGFMp8tGItOEeq0rtKBUchzpQjuaG0YGDTuUPsD4X+H4arq9M/5zzaqbCNKsB6K3VaGp2ocLYx5eo+SonZ+m9/Zz3dE5G/Kkrrydq3G5f0+0A5W3EcOQPUyfPjV20htwL2SzTGDC75rDDlx+Ah3SVHC4lI3/FPfyuozgkJjRPBT+K5+Ml0H973Z1f0xcdcKe7CdfiD18WAHAX2iwxZssJuwrnrUOevme15Z+HoNeM+ZwUtRXjEkmOvVIk3rfBay8aIdzEyTfzv8V1zlY/f09qTHvZ4AwGbodw== 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:(366004)(136003)(39860400002)(376002)(346002)(396003)(86362001)(316002)(66476007)(66556008)(66446008)(52536014)(64756008)(110136005)(33656002)(83380400001)(6506007)(53546011)(7416002)(4326008)(54906003)(8936002)(2906002)(107886003)(9686003)(55016002)(478600001)(66946007)(7696005)(71200400001)(186003)(26005)(8676002)(76116006)(5660300002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?5k77NGTazavLbdQFsac3OvPd0qUwMO2negxTKQF4/DAf2q89RMSgMzeoKTKi?= =?us-ascii?Q?Gj1y8C6OPfGPYYw4McKvCOfB2owka6MPQhwL86YI+NxS01aE3gDAeJZjwGef?= =?us-ascii?Q?RfARcs4ZVYviRIFPk15rgmOPDmifwNPQgeLDlV6vca0QFpYMX2/z7n4Nu7RU?= =?us-ascii?Q?87Oz22nNtsyL/rTm0iIRzFJxZcP7+Cra/hMLX9UgzOh4BOCIFAziIAP5722h?= =?us-ascii?Q?kepeVDj+hD9aREdWnvVIbDK2mVmUzwQ8/I7IZwGuYNdo4DJLbx6Otlar91JE?= =?us-ascii?Q?a52JjN4rNqBZMWltguiiPdjJ6zrLob/UUugST8COYokNqawurRz+ozhG3PH1?= =?us-ascii?Q?+xCojp/hXb2HhoS95dyAVB2Ckj/6rXEskHVJnkF83I/53Z00TUf7aaQ+iEkO?= =?us-ascii?Q?SqqWw57J5RplR0UfAAhCvmK//3zXBC6wW5ispVcPDt8u3L4BEHLv6IEL5TcR?= =?us-ascii?Q?OysX2FW63Bq4xC+UTd6Y+FrQcnbdabjT/JLrg9bcVE7P2VMmzPqhQFtamPOH?= =?us-ascii?Q?f/24RSD3Hg9twxsn07DGYkH0xkRLr7btL7RwuAc1oo23f+e4tWredgsp2yCw?= =?us-ascii?Q?FYi+jyt5x0GQeYug4qo5tPVEbIt7w0q2oknOt+UF5FQH86I6/8kiDD0GxKLZ?= =?us-ascii?Q?V+Y8rMaA/4yZ0dhMZPUpkluRNr8Jm/pbk5qSmZwbnIWPE0p5DDMOnaT5dINr?= =?us-ascii?Q?ahRPZeJHExQcUD7YdHX9WJwcIBALe+JNRtSvu7hvwbJuELu0TUpQz3S0pm2w?= =?us-ascii?Q?Iq+amEgTl2zyqcFLiMNkXJAE/eIf5EMz/81yH8CX6/nH8VE+RCRJG74eFbHH?= =?us-ascii?Q?CS+8vLqIWBoyNgyJPKiOJkT1LJ4Fe4D//QaQoUJFw1oVafQJKTI4tv8mKBNc?= =?us-ascii?Q?/pnDC1hULhYyblPH3oaID6I1XNzlESb3FX0zkqMZI9vCIkLE/qgaDbMmPEg4?= =?us-ascii?Q?x0KnWCL96yKHz05zGgp1fDd0e+1Ben4e+rzUo1IgqTJat0FPmpuvCD+MlU5i?= =?us-ascii?Q?I/aTG91qWb7606o66mMn+GtY8cO16rXxBx+1q1p9vhL2ajBxegug9F/NAGDu?= =?us-ascii?Q?qAKDbANIjxxmgxAH9t2gXFUwE08udX+sF6xlkVBeYdtuJKAaxC8YB5Y/8Wrz?= =?us-ascii?Q?n5gUDMSp1gg4HHMQvWoXwhY/RPpGl7UOTk4RAiS3o9a0JOw7FnNevbJKShRp?= =?us-ascii?Q?4H+BV6HHe3R5u+MJKUln09xnzaWp8WqBE9dQ9hBSw4IHMrEqy5qo4E05mX+x?= =?us-ascii?Q?IInPekpDyRS7j7WqD9SEzCYjE6pe2TJr38/M3p01movJJLXEk0YDUV4sFLIz?= =?us-ascii?Q?WQE=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: 76f18c54-bd79-4080-dba6-08d8dd881529 X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Mar 2021 14:33:01.1361 (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: JNITk5XQ3M1ruLHNahc0F7c+Yt2ufonV/f/8jZk+0XYwcxleiBxomZvyef6HfkxJtU3inbSj+itnRAUqZ09PBJUaxfjB/g7voEvhRS3Q0Dg= X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR1101MB2238 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" 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 >=20 > HI Cristian >=20 > 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 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 - ABI breaka= ge. > > > 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 ma= rked > > 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 n= ot > exist, > > correct? >=20 > 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, l= ooks > me problematic. >=20 Not really, only 3 drivers are currently implementing this API. 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. A simple search exercise for struct rte_mtr_meter_profile is all that is ne= eded. 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? >=20 > > > 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 e= ither > > 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 struc= tures > for > > to support essentially the same algorithms (such as srTCM or trTCM) if = we > can. > > >=20 > 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. We don't want to proliferat= e 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 pic= k 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 mor= e 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 complexit= y in the drivers themselves. So, please, let's try to keep the quality of the APIs high. >=20 > > 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? >=20 > Yes you right, but the implementation effect is not our concern. >=20 >=20 > > > > 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 o= f > > > > 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