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 3ED4AA0C43; Tue, 19 Oct 2021 11:34:44 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 015C140142; Tue, 19 Oct 2021 11:34:44 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id CC35D4003E for ; Tue, 19 Oct 2021 11:34:41 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10141"; a="291935096" X-IronPort-AV: E=Sophos;i="5.85,384,1624345200"; d="scan'208";a="291935096" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Oct 2021 02:34:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.85,384,1624345200"; d="scan'208";a="490223935" Received: from fmsmsx605.amr.corp.intel.com ([10.18.126.85]) by fmsmga007.fm.intel.com with ESMTP; 19 Oct 2021 02:34:40 -0700 Received: from fmsmsx608.amr.corp.intel.com (10.18.126.88) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Tue, 19 Oct 2021 02:34:40 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx608.amr.corp.intel.com (10.18.126.88) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Tue, 19 Oct 2021 02:34:39 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12 via Frontend Transport; Tue, 19 Oct 2021 02:34:39 -0700 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.174) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.12; Tue, 19 Oct 2021 02:34:39 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UndLT6ILJj31UlOqRAgqqDTmZjYPFbdqbhz5T2BlGAA+mblxf90pA/aulDAw2L3r5OtOGvHAedbJHmaC0e0I6SltInYxbvv+39SyVxtK2LIfIhR0jG5FtOwRgg/59V9eDWN0IxfsV4CHX1UsNcdpZfnEpA06Q88HCa3LKywW+qXJs8r47aSDJk4071rHaJ9vncRnoMR2+dc7NECdfbrQxIuoMcqcWm0pQIzjGsu12OKxwvvaAb4CjzoUgGO+giWTP0BozOIwrfiaCw5GYVz+J5qLLQHrV/0nWnrWe+f4RnQ6GXH//JZ+Uf0VSjBSVipVkNh6751QuyHjdkEJ8Xr9Qw== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=/EWQtgkO2u8Y8jTx9BHZMUwv1FrhhU8FtBNMN+w0ZzM=; b=bDdZ72GS9XOtH+5pNuma+IhqHnM2TK4QgWuML6vfTjoa1efWTGzeSBew+uu2B2a6kPdlZ1bfNHTo9VxpcnHqj/Ju2d3bsLctCvccNJOtWuOAeEs7paY7zDFE0sgahWXcZEbCzweVsX42Xv6HY4H/y1MDQ1p/AFVNqlPkkhZKfL1uR2PU01vALli1TNAAobkhsClB3Qq2lQws/Uik63eUca5IKft/qQz2UpzXCVz+LZ67nURLEicGZr2R3gIJ9BKQ54N1ZM2z9r5pXl5hlPZkrRClXIM52XxlZizbczBDRkC2763SHWwbthpYNpzHMCRRK18QGFNnJ+6QfsN7GpXbQQ== 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=/EWQtgkO2u8Y8jTx9BHZMUwv1FrhhU8FtBNMN+w0ZzM=; b=MI/Lgw5wYwEF0HGOiogEDdOwVncHWSXK+IFGsgtpm+cFb6M1zih9Aho85HbGCnRfm7KNaDnhIAGAmSe3Ei4TXAzaFDaX7+wbZwbToaD/qkv9Rla1Ix8MKlTvzCDBO6EzX8ySiBpFS0EMYAXjy86RIQCR3C5efPNz3nNmvixMmGE= Received: from CH0PR11MB5363.namprd11.prod.outlook.com (2603:10b6:610:ba::12) by CH0PR11MB5740.namprd11.prod.outlook.com (2603:10b6:610:101::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4608.16; Tue, 19 Oct 2021 09:34:38 +0000 Received: from CH0PR11MB5363.namprd11.prod.outlook.com ([fe80::90cf:5b59:eef2:1128]) by CH0PR11MB5363.namprd11.prod.outlook.com ([fe80::90cf:5b59:eef2:1128%8]) with mapi id 15.20.4608.018; Tue, 19 Oct 2021 09:34:38 +0000 From: "Liguzinski, WojciechX" To: "Dumitrescu, Cristian" , "dev@dpdk.org" , "Singh, Jasvinder" CC: "Ajmera, Megha" Thread-Topic: [PATCH v14 1/5] sched: add PIE based congestion management Thread-Index: AQHXwcuxSY/xl4YBW0WgpIdSxPiHkavaEKug Date: Tue, 19 Oct 2021 09:34:38 +0000 Message-ID: References: <20211014153346.2816600-1-wojciechx.liguzinski@intel.com> <20211015081620.2873739-1-wojciechx.liguzinski@intel.com> <20211015081620.2873739-2-wojciechx.liguzinski@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.6.200.16 dlp-reaction: no-action authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 4098da0e-17d1-4764-3b04-08d992e3ab92 x-ms-traffictypediagnostic: CH0PR11MB5740: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: ICsPsrbQVQzTiPl1sDmek+DgrrhpAS1L8VsT0zFczRrMT3yfpo20AeI9iyiE0PTdd1iP1hNyB2bbODo3ECXn0zluH32ajLi57/zya4JymV5avpK5HqrOurVtL8c1mxQzLQuukDHV2OH6ubbCGl4mMj+bCvsqhA+jrp8F1+Ft81fjsRCxjXsJcAMyQGXtQspkqDBhGBJpDaFUSlbNCS7DYDdH2jKU2yV86yEMJrC9zIa6DFtgqM7zbX8oM3gekSRNyoY1PKzelqnXfVlDvOo7G63Sk/2Bwr9O7mUmF0XHiqpkZwGdzqNceOiFWT9mu9vdn1lIZD4YBIY9GOTLZ4x+Hf+crpV2y+r0i/NWP6pSato/LrK4MqoQA058xeS64sq0tV4vnTTHZ2AjTsPDLCq089PZlIxxmmhOOf7BUHcR+z2tOiVixYijov3EBTMT32ch6+LHyzjjTNUQ/Y3w4vE5cmJ8uQu/5nMdYIIcqPx2k3Z7nHbpUK4nKciTqyRHaDdbXTPOaOJo6vRm3h82ISnE1u0B/VC5ohJIDSSAxOyJvQAiK12ds8BHXxifRYmnTTlf1y9cEd+XwJ8Q1TaVK61f3INr+DIYtNhPxbEd074V3O2zdq59zMvwqffNa8Z9Xe2YcIj1Nrrkqm+G5wED8mnqUhVwkdGKIjS4RQ2iPfNy9EQds4tAGdbZgBx867t1oqaLJdI0EySFwOEHGQTeZbM4fw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH0PR11MB5363.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(366004)(316002)(52536014)(86362001)(4326008)(508600001)(83380400001)(76116006)(122000001)(6506007)(5660300002)(38070700005)(38100700002)(66556008)(26005)(71200400001)(82960400001)(53546011)(9686003)(8936002)(186003)(55016002)(6636002)(110136005)(107886003)(66446008)(8676002)(66476007)(33656002)(7696005)(64756008)(66946007)(2906002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?CtsB4//qnt70YL+QNZCGxZnnvBCtzGHXGLajsmjsEddM8e+PUGo2x7l3hU/y?= =?us-ascii?Q?uN8S32yhFuXecCBQYt1i3/ZFnsUR4shs569zk4bQZc6uqv/mycM0Nxe/RJAg?= =?us-ascii?Q?RaeY2qVjE/CNHr4zDm7hZCDZnS7GDKhpcEjRecGUW1LMX128UjUqSkzKJ6jB?= =?us-ascii?Q?BGQSYzYE6/ZhqBmNfyTbEltCysHKkH4Sgxc2d3QcT4akzrBHE3aumyZqQpp5?= =?us-ascii?Q?LY2iq2bY0yiQJOkT+cKKj4pjf+n3bth7Dee00Ozbm+TPZ0Pb5JtrXb3xD6FJ?= =?us-ascii?Q?pd1qyfvvPK2l2eoyzCy3W7uZh8W0HBEYOWJPPBXuN/k5v07p/o6sPunWrFoj?= =?us-ascii?Q?ZQsIrmyDerYXsvtetHhexNPDWETQsawVzt7RThDVhqv1Iiii85H0DPWxbVb/?= =?us-ascii?Q?CzjV2r5d2WnqjBr306uYo9sY2nuxRkyuFviceCRQCQNe7YkHZ9ggc1QQvcX7?= =?us-ascii?Q?jrVvOH6r0V7IrilPJ5d7W/3+3kfGXcmidkvAq8PSYrnkQ5Q9lKzRp8Zq9Wv2?= =?us-ascii?Q?sq6VJdrTrwrTLw5K/ZNCrqEdP9kjArGIYN1KPmoXbXBOF94EIXCv9Nbr8EKM?= =?us-ascii?Q?0/Jj5fLQdJYANEVNMgvQYVzCUmllZDxmoXc0aHanBJW2sCZrudFLMrcyHuVy?= =?us-ascii?Q?08+/RKg6/zr2qknSrrEFiWooMRg9Uv+tdv2EW6vqtFPWAFag/hwqyQ4h8maj?= =?us-ascii?Q?tiyZlBg3sCitDWoNEYtwKxU+KlDK6xJmodhPSyvJTGbmJxt6ic1RJz26jedS?= =?us-ascii?Q?PHaagHs4jQEyU8klMAyUWon6z3iT0L4xGnCGLP+qpPo40FDWAauHzneaNii0?= =?us-ascii?Q?uayUeCY6sq7GBx6FTVA3aKX2TLBB8ED8DRl2/X60+5OMZgIDmfRmHIGsntFZ?= =?us-ascii?Q?RtYMXaLTXqzGshbs36wEcB2zxZDoLzVbUxALmif7Y8BKL5wi57hCLx95Q/en?= =?us-ascii?Q?WFXCKrKnBX6+w5qm5fJJOW+kHGzhyDZVemYPC3IrTPIRzGf+F1WEU32eiJVK?= =?us-ascii?Q?SdX0xtRiyBNGNUR3gTgOv2ydqIwkesdD3Q47j0V+tPXVsLl9MEh91dt9cHRf?= =?us-ascii?Q?PYZ2ztJJD9Csq8ztIzqHSj9mlZ0uRvFPo9Z1FUdqe3EG23aL4ff90L/9G1VK?= =?us-ascii?Q?U/1SWtJbI6WKk8Q9piU9Y+3A1PscCzs470AZ+FkYP61WEqr05hDWKc7kYHcp?= =?us-ascii?Q?ZZYFz5tR004Oxt3tKyh+9JHI4cHb5AvkO3dmRTclKT5ZTT5hzWtysFqRz4dl?= =?us-ascii?Q?Xy7Z1Zsb3oaBCG+wUWQPX2cE0G/XHeIJFFJjPEWVtdJGvWvssPll4qdpR3YM?= =?us-ascii?Q?FdprENLOOtGo2/WKrp4NBzt5?= 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: CH0PR11MB5363.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 4098da0e-17d1-4764-3b04-08d992e3ab92 X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Oct 2021 09:34:38.2518 (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: Osot0LMCPKgIWHB1v8jd/YbMMqVPk4c/Udiyp6MpaCFfhE8Biy2kHkmw5KNHQHrgz/NJVytnPAU4VkUehRNZ/p0nYUt3kGeZaMu+wlgup3o= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR11MB5740 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v14 1/5] sched: add PIE based congestion management 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, - As you asked I have added some points/one-liners in the patches what is c= hanged - Regarding usage of rte_sched_cman_params structure I will write you a sep= arate message - Reverted to use the RTE_SCHED_CMAN_RED flag - Also changed WRED occurrences back to RED - No, I wasn't getting a compilation error when struct rte_sched_cma_params= was defined below its instantiation... But, to be certain I moved it above= that place in code. - version.map updated with a comment, like you suggested Thanks for the review comments! BR, Wojtek -----Original Message----- From: Dumitrescu, Cristian =20 Sent: Friday, October 15, 2021 3:51 PM To: Liguzinski, WojciechX ; dev@dpdk.org; S= ingh, Jasvinder Cc: Ajmera, Megha Subject: RE: [PATCH v14 1/5] sched: add PIE based congestion management Hi Wojciech, Thank you for your patchset! Can you please, at least starting from this version, add a short change log= at the top of your file, just after the signoff line? It helps a lot durin= g the review process, and you can find abundant examples in other patchsets= from this email list. One line of description for every change would be ni= ce, thank you. > -----Original Message----- > From: Liguzinski, WojciechX > Sent: Friday, October 15, 2021 9:16 AM > To: dev@dpdk.org; Singh, Jasvinder ;=20 > Dumitrescu, Cristian > Cc: Ajmera, Megha > Subject: [PATCH v14 1/5] sched: add PIE based congestion management >=20 > Implement PIE based congestion management based on rfc8033 >=20 > Signed-off-by: Liguzinski, WojciechX > --- > drivers/net/softnic/rte_eth_softnic_tm.c | 6 +- > lib/sched/meson.build | 10 +- > lib/sched/rte_pie.c | 82 +++++ > lib/sched/rte_pie.h | 393 +++++++++++++++++++++++ > lib/sched/rte_sched.c | 240 +++++++++----- > lib/sched/rte_sched.h | 63 +++- > lib/sched/version.map | 3 + > 7 files changed, 701 insertions(+), 96 deletions(-) create mode=20 > 100644 lib/sched/rte_pie.c create mode 100644 lib/sched/rte_pie.h >=20 > diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c > index a858f61f95..a066eed186 100644 > --- a/lib/sched/rte_sched.c > +++ b/lib/sched/rte_sched.c > @@ -183,8 +187,14 @@ struct rte_sched_subport { > /* Pipe queues size */ > uint16_t qsize[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE]; >=20 > -#ifdef RTE_SCHED_RED > - struct rte_red_config > red_config[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][RTE_COLORS]; > +#ifdef RTE_SCHED_CMAN > + enum rte_sched_cman_mode cman; > + > + RTE_STD_C11 > + union { > + struct rte_red_config > red_config[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][RTE_COLORS]; > + struct rte_pie_config > pie_config[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE]; > + }; > #endif Can you please use here the rte_sched_cman_params structure that you just c= reated in rte_sched.h as opposed to inlining this structure. Yes, I agree i= t might have some ripple effect throughout this file, but I think it should= be very limited, and also quick to do. > diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h > index c1a772b70c..07fcf439d8 100644 > --- a/lib/sched/rte_sched.h > +++ b/lib/sched/rte_sched.h > @@ -61,9 +61,10 @@ extern "C" { > #include > #include >=20 > -/** Random Early Detection (RED) */ > -#ifdef RTE_SCHED_RED > +/** Congestion Management */ > +#ifdef RTE_SCHED_CMAN > #include "rte_red.h" > +#include "rte_pie.h" > #endif >=20 > /** Maximum number of queues per pipe. > @@ -110,6 +111,28 @@ extern "C" { > #define RTE_SCHED_FRAME_OVERHEAD_DEFAULT 24 > #endif >=20 > +/** > + * Congestion Management (CMAN) mode > + * > + * This is used for controlling the admission of packets into a packet q= ueue > or > + * group of packet queues on congestion. > + * > + * The *Random Early Detection (RED)* algorithm works by proactively > dropping > + * more and more input packets as the queue occupancy builds up. When > the queue > + * is full or almost full, RED effectively works as *tail drop*. The *We= ighted > + * RED* algorithm uses a separate set of RED thresholds for each packet > color. > + * > + * Similar to RED, Proportional Integral Controller Enhanced (PIE) rando= mly > + * drops a packet at the onset of the congestion and tries to control th= e > + * latency around the target value. The congestion detection, however, i= s > based > + * on the queueing latency instead of the queue length like RED. For mor= e > + * information, refer RFC8033. > + */ > +enum rte_sched_cman_mode { > + RTE_SCHED_CMAN_WRED, /**< Weighted Random Early Detection > (WRED) */ The algorithm is RED, not WRED. Let's stick to RTE_SCHED_CMAN_RED, please. = The Weighted aspect comes into place when defining the struct rte_sched_cma= n_params::red_params as an array indexed by color below. > + RTE_SCHED_CMAN_PIE, /**< Proportional Integral Controller > Enhanced (PIE) */ > +}; > + > /* > * Pipe configuration parameters. The period and credits_per_period > * parameters are measured in bytes, with one byte meaning the time > @@ -174,12 +197,30 @@ struct rte_sched_subport_params { > /** Max allowed profiles in the pipe profile table */ > uint32_t n_max_pipe_profiles; >=20 > -#ifdef RTE_SCHED_RED > - /** RED parameters */ > - struct rte_red_params > red_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][RTE_COLORS]; > +#ifdef RTE_SCHED_CMAN > + /** Congestion Management parameters */ > + struct rte_sched_cman_params *cman_params; You are instantiating struct rte_sched_cma_params here, but you are definin= g it just below, i.e. after you attempted to instantiate it. Aren't you get= ting a build error when compiling with RTE_SCHED_CMAN defined? > #endif > }; >=20 > +#ifdef RTE_SCHED_CMAN > +/* > + * Congestion Management configuration parameters. > + */ > +struct rte_sched_cman_params { > + /** Congestion Management mode */ > + enum rte_sched_cman_mode cman_mode; > + > + union { > + /** WRED parameters */ In the comment: RED parameters instead of WRED, please. > + struct rte_red_params > red_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][RTE_COLORS]; > + > + /** PIE parameters */ > + struct rte_pie_params > pie_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE]; > + }; > +}; > +#endif > + > diff --git a/lib/sched/version.map b/lib/sched/version.map > index 53c337b143..54e5e96d4f 100644 > --- a/lib/sched/version.map > +++ b/lib/sched/version.map > @@ -30,4 +30,7 @@ EXPERIMENTAL { > rte_sched_subport_pipe_profile_add; > # added in 20.11 > rte_sched_port_subport_profile_add; > + > + rte_pie_rt_data_init; > + rte_pie_config_init; You need to put a comment about the release when these symbols got introduc= ed, similar to above. > }; > -- > 2.25.1 Thank you for your work! Regards, Cristian