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 F2CAEA00BE; Tue, 10 May 2022 11:09:50 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E53584281D; Tue, 10 May 2022 11:09:50 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id 9CB1A4069D for ; Tue, 10 May 2022 11:09:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1652173788; x=1683709788; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=V+yYXp1KiUy43Liz76OJdRig3QHK+eL+Ig6+G8e5Dko=; b=nOe9da9O7HtrQ0Jit9uJwFJFFjmcNoXbyCFYwreB6Dnod+8hjoDzo2+T jQWR0gJJRkjZCy72tgfuoyjnGjbSrPuQDwiUtH1erIDEMdWSS9uo6D2ZN QqqjSMCszqXdGWNcPwbtT0XVP/1fBtegMFmwNQEDsADlzJUWLUwtTjQKd JqAQooYtxKagatlQ9i46s2/h2T35d+Lju2cU9883c1ZSYlNEAhPNC7qMb RwSwc5hGQIM9sTKwFg1INPAcBVaofHN4l1oBHZ/urPOY04PJtWXc3QYI2 mLn08bfC7uzSqzMJWHUJoXP0jYEeL30we+QS65rNd71/cmhiKTL2Xpk60 Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10342"; a="268154761" X-IronPort-AV: E=Sophos;i="5.91,214,1647327600"; d="scan'208";a="268154761" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 May 2022 02:09:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.91,214,1647327600"; d="scan'208";a="738586983" Received: from orsmsx604.amr.corp.intel.com ([10.22.229.17]) by orsmga005.jf.intel.com with ESMTP; 10 May 2022 02:09:47 -0700 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) 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.2308.27; Tue, 10 May 2022 02:09:47 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) 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.2308.27 via Frontend Transport; Tue, 10 May 2022 02:09:47 -0700 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (104.47.55.168) 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.2308.27; Tue, 10 May 2022 02:09:46 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MnyraiQV284fS/ZJnYX8QYsfbC2DnKOfk2LfsR4fLX6juV4dcZ0acJgb0ruSJufRdKj+HWppsMepUt0PFlr7PLmf1gZWqc3AiX+GYXSlwPxVKn8O30yCNBon4GKl7dR/3t/GJI858TlMomfQ6naBERyv9hYFb/nkiaw3EWzdoG76iZyBgiTOEZFNpc4QrjH4ZfRWMIW09SIWkk2ylYEXDUfvIfATLoQDgJbbV0rfVrs7HV0XUTPUrBTHmW+MzJP6yL24k77t+MN/Ty1ja+WVRhUbRIY4n7pz7gGZZJ9h50P6ZN06o6p+LU4G3AuA/OoeF9QnCpGYle0oBrndd7vn7A== 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=2kYCaoIp8Z/6Z67WhBMByNbO1NOivjWoTmU9BVRvtlg=; b=V+mJswBoe1cVEtDf1BnlYmvXpDSIGSsn8rr8tcqujBGIO9XBSRpg/PwL+UQtmIUDpA0tqxW+iHmFOAN5Wyy0tidWGb6EdoL0Lnd/mSmS0pZidFV/AAAOR719vFYv3SpG2ERMstZcLMomBaoDUFNE12LCQpP0Ix6PuH17DbbXKvEznasdwGDfkeDZKc0lRAjASOlw5dVSTrJzUn1lD1ZgZ81vfjZjS7zNqv2CXIlsbNYszRv4oFFnS1jnpits917dw/yeYhkgfVqzNHb9kSdbNFI75c4EkVUC+T2Ut7YZMfCZHlI5ORdPJ97PW4l4K7QXLiFggnRPGPEF1dpyTJrJPw== 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 Received: from DM8PR11MB5670.namprd11.prod.outlook.com (2603:10b6:8:37::12) by DM6PR11MB4012.namprd11.prod.outlook.com (2603:10b6:5:6::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5227.20; Tue, 10 May 2022 09:09:44 +0000 Received: from DM8PR11MB5670.namprd11.prod.outlook.com ([fe80::5dc9:53d7:3ece:fa2d]) by DM8PR11MB5670.namprd11.prod.outlook.com ([fe80::5dc9:53d7:3ece:fa2d%2]) with mapi id 15.20.5227.023; Tue, 10 May 2022 09:09:44 +0000 From: "Dumitrescu, Cristian" To: "Ajmera, Megha" , "Danilewicz, MarcinX" , "dev@dpdk.org" , "Singh, Jasvinder" CC: "Thakur, Sham Singh" Subject: RE: [PATCH v3] sched: enable/disable TC OV at runtime Thread-Topic: [PATCH v3] sched: enable/disable TC OV at runtime Thread-Index: AQHYWhiOjMuBu5wHJkOvKczZqJ0XNK0XAoTQgAC7FICAACdPEA== Date: Tue, 10 May 2022 09:09:44 +0000 Message-ID: References: <20220407145153.238969-2-marcinx.danilewicz@intel.com> <20220427092357.491720-1-marcinx.danilewicz@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.6.401.20 dlp-reaction: no-action dlp-product: dlpe-windows authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 545632f1-2b26-46ad-9e9d-08da3264d33f x-ms-traffictypediagnostic: DM6PR11MB4012:EE_ x-microsoft-antispam-prvs: x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: sqJ/jqZ/TsNYLe2AEvUxnPJhCDGhc7IyFF0rXhaUshwYWoAisDem7LBK671tLErOild8NNIszQRPxzM7v9ZVp70v0gWga4vrpg+lvOyyGZRk8qXloqYjCedO+yVzbxHZJYWgJTIHnro/S1TsC62yT9RK1Tmzf48aY2NB7bmaOLLnVK0iZ00cAIWIJtPTQf/do4qV/ZKbdUur6rBEkZWV21DSDFaQNJFvgOnY3Fd3RWkNZpK0uKJ+l3/nX34DQY15PtUzyII0NIwjn6wDkQIa6xkxo96osmI/ZoV+/lACqDNpOSEXQOZEX5nuLvJ7uJfJCWImHv3qZANrCpJx6I482j6Mt3c10fAGYGYtdw23y6kunKM/erm9bb99pAcYlqZtzJ7R/7zslYb6Ozq8fLJEl/Lf5+EieCgQHE7jDMC3n2CgxuXhx6NzUmwgwfT3GflG9NGX330+H2Z9TdUUxtLihwUU8PsWEzndzGiy8dMQBPDF5sYQN2/+ZAgazMFL+1jx7kDXZaQdkrh8YXItKAXrvWWpTEzalgpjHcHMIjVbLZu5zqkaHuS5OBJG9DQoaHE1F3cf2hFtD4gsL5o6jXUAMX3QQ1sL+tTkMhBpw1MlHEWhO3JOyzPgolrqoeNayzsyIB8U3dbykhdcsNra4FdiIq5vZbUssKJjsQBRCFszqh/3OtGtCZzsUCCir0Ix9EWSPgJB76zUyk6lFas9GoMTYg== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM8PR11MB5670.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(366004)(55016003)(52536014)(8936002)(71200400001)(110136005)(508600001)(8676002)(66556008)(66476007)(66946007)(66446008)(64756008)(33656002)(76116006)(2906002)(82960400001)(7696005)(83380400001)(6636002)(9686003)(4326008)(122000001)(26005)(6506007)(107886003)(186003)(53546011)(5660300002)(38070700005)(30864003)(316002)(38100700002)(86362001); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?eOEqC6ZrwLbggfvM0rAq3LTTQW5EXk0/8Dyf3BGgDRjhoxjhAVbJ/VmFOxYY?= =?us-ascii?Q?DhCy8sN5lBFPpxrOMlX5xec2Ti48kCtHMS64fc6r9k5WyU2e6/QT4Gwzc4sd?= =?us-ascii?Q?XBw7nxg+x310l19a9QglBJYXtRau+l4LbV8D0B+o5m/dJrQLM+0+OS/2DSSo?= =?us-ascii?Q?ysiiBXE7l/GeH6GzLCuelxlmzHcgapNL/6VoAqQYHoF9U4gJW3+CI6uZWIYh?= =?us-ascii?Q?IFoQqb6qCfM1KwmRcu6Uqtx6xjWyfG6g3nsQmFyZsBPd9Y8eKs3y/1T6hmSv?= =?us-ascii?Q?u7a1A+ffwXdwHYGcJqLyBy33Vy7eMyStikja0C+lMB38KcjhG+BApQx0lO8U?= =?us-ascii?Q?RRukybF1uRcO/adMMqY8mEcX5oIZMPXK+aPuzzcp3Vtg0uuDWRbuP4VU6YI2?= =?us-ascii?Q?LHCQmE43T/Lq1rDbZH8cOy4RMedSsqJN2gzfK0E6tLrnaAxuw0sO+bLh8kCL?= =?us-ascii?Q?FQu3SsyHnhSvKFX+Tj/QvqZWuia4uZ/FE4L5TSrNrCpnKoq4NwD/+bcfE6vI?= =?us-ascii?Q?6zNXAbEhIng3vQZVNVzbDrGhDngn9E0IoCC0T/3iMO+fDihmFoa1qwrTpAMA?= =?us-ascii?Q?fHSeQlmvwuTFxNDnmICpmJtaW777eVHr1h57nU1pWZFZoJ2jAplaFpqsbbu9?= =?us-ascii?Q?B1NrRrEI3bEttMVfMT2y3Io0mvbQMcVVXIPGq8PN0QjvVZCeQ3YtqbwAsi0d?= =?us-ascii?Q?kbMA/AGWQY0EUISNEKQHWfsiCa2MXgtvpXblGIA0K4q0F+SWxaqWin/aYH4F?= =?us-ascii?Q?Z/SZuESb/sVk2o8bMTsBBftJRpTyv9CSeONtBsCHum50YqzSJwt0dhHZLtPO?= =?us-ascii?Q?2GI7bAshJu4CEyfnIatjak/SyA1tpNjli161Y4XQBK7CYVGNa5ZD++Rflt85?= =?us-ascii?Q?4AvMpfxbtd4nIUvqgipy5EbvWOgzjk9/9KPF+Qi+J7I/G5bdxR9SU0dkgSNe?= =?us-ascii?Q?Hg51gQJdlffHRPlyIgYiqqFNACEyukZZ/+LPkY879IlJcKC0ewVEyLS439fJ?= =?us-ascii?Q?u5nOH/p1Dzk5SqRi2afbldBp52rWt1z9K9+NtCNMFHiBWU6i+RF5J2D8QGcG?= =?us-ascii?Q?0WKXA7mAXzBViCqXF95ydqeQ+N8hzPNKW71WKWRbn8DbeqMomOW+2aLiRkO5?= =?us-ascii?Q?0zRb8MRifn2q2Cu3jGpQQDZM6vXO/CuO3vB7quIWtenmTKJ8L60U84ubc/Qf?= =?us-ascii?Q?W4v89+5FrIGHRjQDlDYGgGrkud8S5rcDybWDz2IrgkbUFe4KhAbBKcqo5eM8?= =?us-ascii?Q?Ft7kqGXkV06kJ+mGauTE5zOjIU4LGM4bdgfgFcIsvuM0XzdDWkODNfljXHz7?= =?us-ascii?Q?nxa29u3+ZvEwWFoUtHY3SSeO7CQ3/zCJ32HkLwLo3C6lZxWpldkJkALKh81a?= =?us-ascii?Q?ea8J9aSC0nVUrXGs4mUzV22gjKEEz+T6xh70DPUFmtY1ktVV1nsXIsjZrYrV?= =?us-ascii?Q?Ot2MjfYhj2Y6NlnyaVDtpnOn9uxXtrb8LMFm5AfKiDlcSBYO2dEO67Q9Obc1?= =?us-ascii?Q?ryCjTpavxeXH2s4QIayX6DRWiIGX9UiTK+XIAz1fdzJnh7eO6cdmT8p6Fnta?= =?us-ascii?Q?HN++xyLyUk541HIa0vKG0B19SgeL1gIo58yT1aNcRDR+e0SffXnlg9Absyqy?= =?us-ascii?Q?/nf/339KQ2T+NNWQfZoKwTfUOqvKdgwZ4q+y16BsmgYCvDHkQmVKA2LjEV8x?= =?us-ascii?Q?h+TBzucQvFupbFYLJcmtd0vsc1Kt6Sn8A1BnD0v9C1ywJBOBy/WsO2Rt9nkg?= =?us-ascii?Q?YmF+KFQr0IAlrGPN+jT0vQfumPbDg7A=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: DM8PR11MB5670.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 545632f1-2b26-46ad-9e9d-08da3264d33f X-MS-Exchange-CrossTenant-originalarrivaltime: 10 May 2022 09:09:44.8470 (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: 3fQLx5b9CiIZUFcEQfv3w0PPbsfPoqdZhc8QIAHLJLfbLDssGfSXo8sCnADiAQVVoXc073QMeNj6dtSX9geFox8oEgPJx03tJCn+HwUBimM= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4012 X-OriginatorOrg: intel.com 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 Hi Megha, > -----Original Message----- > From: Ajmera, Megha > Sent: Tuesday, May 10, 2022 7:41 AM > To: Dumitrescu, Cristian ; Danilewicz, Mar= cinX > ; dev@dpdk.org; Singh, Jasvinder > > Cc: Thakur, Sham Singh > Subject: RE: [PATCH v3] sched: enable/disable TC OV at runtime >=20 > Hi Cristian, Marcin, >=20 > > > -----Original Message----- > > > From: Danilewicz, MarcinX > > > Sent: Wednesday, April 27, 2022 10:24 AM > > > To: dev@dpdk.org; Singh, Jasvinder ; > > > Dumitrescu, Cristian > > > Cc: Ajmera, Megha > > > Subject: [PATCH v3] sched: enable/disable TC OV at runtime > > > > We are not trying to enable/disable the traffic class oversubscription = feature > at > > run-time, but at initialization. If cat, we should prohibit changing th= is post- > > initialization. > > >=20 > If we only need this to be configured at initialization time, then we can= as well > take this flag in subport config API itself. Then there will be no need f= or a new > API. The purpose of new API was to enable/disable this feature at runtime= . >=20 Yes, I agree this would be the ideal way to drive this change, but the prob= lem is that modifying the existing subport parameter structure would repres= ent an API change. This would require a deprecation notice, and the patch w= ould be blocked until 22.11 release. Are you willing to wait until 22.11? I= f not, then adding the configuration function for this flag is the next bes= t thing. > > Also the name of the feature should not be abbreviated in the patch tit= le. > > > > I suggest you rework the title to: > > [PATCH] sched: enable traffic class oversubscription conditionally > > > > > > > > Added new API to enable or disable TC over subscription for best > > > effort traffic class at subport level. > > > Added changes after review and increased throughput. > > > > > > By default TC OV is disabled. > > > > It should be the other way around, the TC_OV should be enabled by defau= lt. > The > > TC oversubscription is a more natural way to use this library, we usual= ly want > to > > disable this feature just for better performance in case this functiona= lity is > not > > needed. Please initialize the tc_ov flag accordingly. > > >=20 > In original code, this feature has always been disabled as it impacts > performance. > So, in my opinion we should keep it disabled by default and let user enab= le it > when required. >=20 In the original code, yes, it had to be explicitly enabled through a build-= time flag. This was not the best option, and this is precisely what we are = trying to fix with this patch. But on the other hand all the users of these library that I know use it wit= h the TC oversubscription turned on. Functionality is more important for th= em than performance. Hence my vote now is to enable it by default; those us= ers that prefer performance over functionality can easily turn this feature= off with no issues. > > > > > > Signed-off-by: Marcin Danilewicz > > > --- > > > lib/sched/rte_sched.c | 189 > > > +++++++++++++++++++++++++++++++++++------- > > > lib/sched/rte_sched.h | 18 ++++ > > > lib/sched/version.map | 3 + > > > 3 files changed, 178 insertions(+), 32 deletions(-) > > > > > > diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c index > > > ec74bee939..6e7d81df46 100644 > > > --- a/lib/sched/rte_sched.c > > > +++ b/lib/sched/rte_sched.c > > > @@ -213,6 +213,9 @@ struct rte_sched_subport { > > > uint8_t *bmp_array; > > > struct rte_mbuf **queue_array; > > > uint8_t memory[0] __rte_cache_aligned; > > > + > > > + /* TC oversubscription activation */ > > > + int is_tc_ov_enabled; > > > > How about we simplify the name of this variable to: tc_ov_enabled ? > > > > > } __rte_cache_aligned; > > > > > > struct rte_sched_port { > > > @@ -1165,6 +1168,45 @@ rte_sched_cman_config(struct rte_sched_port > > > *port, } #endif > > > > > > +int > > > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, > > > + uint32_t subport_id, > > > + bool tc_ov_enable) > > > +{ > > > + struct rte_sched_subport *s; > > > + struct rte_sched_subport_profile *profile; > > > + > > > + if (port =3D=3D NULL) { > > > + RTE_LOG(ERR, SCHED, > > > + "%s: Incorrect value for parameter port\n", __func__); > > > + return -EINVAL; > > > + } > > > + > > > + if (subport_id >=3D port->n_subports_per_port) { > > > + RTE_LOG(ERR, SCHED, > > > + "%s: Incorrect value for parameter subport id\n", > > > __func__); > > > + return -EINVAL; > > > + } > > > + > > > + s =3D port->subports[subport_id]; > > > + s->is_tc_ov_enabled =3D tc_ov_enable ? 1 : 0; > > > + > > > + if (s->is_tc_ov_enabled) { > > > + /* TC oversubscription */ > > > + s->tc_ov_wm_min =3D port->mtu; > > > + s->tc_ov_period_id =3D 0; > > > + s->tc_ov =3D 0; > > > + s->tc_ov_n =3D 0; > > > + s->tc_ov_rate =3D 0; > > > + > > > + profile =3D port->subport_profiles + s->profile; > > > + s->tc_ov_wm_max =3D rte_sched_time_ms_to_bytes(profile- > > > >tc_period, > > > + s->pipe_tc_be_rate_max); > > > + s->tc_ov_wm =3D s->tc_ov_wm_max; > > > + } > > > + return 0; > > > +} > > > > This function should not exist, please remove it and keep the initial c= ode that > > computes the tc_ov related variable regardless of whether tc_ov is enab= led > or > > not. > > > > All the tc_ov related variables have the tc_ov particle in their name, = so there > is > > no clash. This is initialization code, so no performance overhead. Let'= s keep > the > > code unmodified and compute both the tc_ov and the non-tc_ov varables a= t > > initialization, regardless of whether the feature is enabled or not. > > > > This comment is applicable to all the initialization code, please adjus= t all the > init > > code accordingly. There should be no diff showing in the patch for any = of the > init > > code! > > > > For this file "rte_sched.c", your patch should contain just two additio= nal run- > > time functions, i.e. the non-tc-ov version of functions > grinder_credits_update() > > and grindler_credits_check(), and the small code required to test when = to use > > the tc-ov vs. the non-tc_ov version, makes sense? > > > > > + > > > int > > > is_tc_ov_enabled (struct rte_sched_port *port, > > > uint32_t subport_id, > > > @@ -1254,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port > > > *port, > > > s->n_pipe_profiles =3D params->n_pipe_profiles; > > > s->n_max_pipe_profiles =3D params->n_max_pipe_profiles; > > > > > > + /* TC over-subscription is disabled by default */ > > > + s->is_tc_ov_enabled =3D 0; > > > + > > > > By default, this feature should be enabled: > > s->is_tc_ov_enabled =3D 1; > > > > > #ifdef RTE_SCHED_CMAN > > > if (params->cman_params !=3D NULL) { > > > s->cman_enabled =3D true; > > > @@ -1316,13 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port > > > *port, > > > > > > for (i =3D 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) > > > s->grinder_base_bmp_pos[i] =3D > > > RTE_SCHED_PIPE_INVALID; > > > - > > > - /* TC oversubscription */ > > > - s->tc_ov_wm_min =3D port->mtu; > > > - s->tc_ov_period_id =3D 0; > > > - s->tc_ov =3D 0; > > > - s->tc_ov_n =3D 0; > > > - s->tc_ov_rate =3D 0; > > > } > > > > > > { > > > @@ -1342,9 +1380,6 @@ rte_sched_subport_config(struct rte_sched_port > > > *port, > > > else > > > profile->tc_credits_per_period[i] =3D 0; > > > > > > - s->tc_ov_wm_max =3D rte_sched_time_ms_to_bytes(profile- > > > >tc_period, > > > - s- > > > >pipe_tc_be_rate_max); > > > - s->tc_ov_wm =3D s->tc_ov_wm_max; > > > s->profile =3D subport_profile_id; > > > > > > } > > > @@ -1417,17 +1452,20 @@ rte_sched_pipe_config(struct rte_sched_port > > > *port, > > > double pipe_tc_be_rate =3D > > > (double) params- > > > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > > > / (double) params->tc_period; > > > - uint32_t tc_be_ov =3D s->tc_ov; > > > > > > - /* Unplug pipe from its subport */ > > > - s->tc_ov_n -=3D params->tc_ov_weight; > > > - s->tc_ov_rate -=3D pipe_tc_be_rate; > > > - s->tc_ov =3D s->tc_ov_rate > subport_tc_be_rate; > > > + if (s->is_tc_ov_enabled) { > > > + uint32_t tc_be_ov =3D s->tc_ov; > > > > > > - if (s->tc_ov !=3D tc_be_ov) { > > > - RTE_LOG(DEBUG, SCHED, > > > - "Subport %u Best-effort TC oversubscription is > > > OFF (%.4lf >=3D %.4lf)\n", > > > - subport_id, subport_tc_be_rate, s- > > > >tc_ov_rate); > > > + /* Unplug pipe from its subport */ > > > + s->tc_ov_n -=3D params->tc_ov_weight; > > > + s->tc_ov_rate -=3D pipe_tc_be_rate; > > > + s->tc_ov =3D s->tc_ov_rate > subport_tc_be_rate; > > > + > > > + if (s->tc_ov !=3D tc_be_ov) { > > > + RTE_LOG(DEBUG, SCHED, > > > + "Subport %u Best-effort TC > > > oversubscription is OFF (%.4lf >=3D %.4lf)\n", > > > + subport_id, subport_tc_be_rate, s- > > > >tc_ov_rate); > > > + } > > > } > > > > > > /* Reset the pipe */ > > > @@ -1460,19 +1498,22 @@ rte_sched_pipe_config(struct rte_sched_port > > > *port, > > > double pipe_tc_be_rate =3D > > > (double) params- > > > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > > > / (double) params->tc_period; > > > - uint32_t tc_be_ov =3D s->tc_ov; > > > > > > - s->tc_ov_n +=3D params->tc_ov_weight; > > > - s->tc_ov_rate +=3D pipe_tc_be_rate; > > > - s->tc_ov =3D s->tc_ov_rate > subport_tc_be_rate; > > > + if (s->is_tc_ov_enabled) { > > > + uint32_t tc_be_ov =3D s->tc_ov; > > > + > > > + s->tc_ov_n +=3D params->tc_ov_weight; > > > + s->tc_ov_rate +=3D pipe_tc_be_rate; > > > + s->tc_ov =3D s->tc_ov_rate > subport_tc_be_rate; > > > > > > - if (s->tc_ov !=3D tc_be_ov) { > > > - RTE_LOG(DEBUG, SCHED, > > > - "Subport %u Best effort TC oversubscription is > > > ON (%.4lf < %.4lf)\n", > > > - subport_id, subport_tc_be_rate, s- > > > >tc_ov_rate); > > > + if (s->tc_ov !=3D tc_be_ov) { > > > + RTE_LOG(DEBUG, SCHED, > > > + "Subport %u Best effort TC > > > oversubscription is ON (%.4lf < %.4lf)\n", > > > + subport_id, subport_tc_be_rate, s- > > > >tc_ov_rate); > > > + } > > > + p->tc_ov_period_id =3D s->tc_ov_period_id; > > > + p->tc_ov_credits =3D s->tc_ov_wm; > > > } > > > - p->tc_ov_period_id =3D s->tc_ov_period_id; > > > - p->tc_ov_credits =3D s->tc_ov_wm; > > > } > > > > > > return 0; > > > @@ -2318,6 +2359,45 @@ grinder_credits_update(struct rte_sched_port > > > *port, > > > pipe->tb_credits =3D RTE_MIN(pipe->tb_credits, params->tb_size); > > > pipe->tb_time +=3D n_periods * params->tb_period; > > > > > > + /* Subport TCs */ > > > + if (unlikely(port->time >=3D subport->tc_time)) { > > > + for (i =3D 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > > > + subport->tc_credits[i] =3D sp->tc_credits_per_period[i]; > > > + > > > + subport->tc_time =3D port->time + sp->tc_period; > > > + } > > > + > > > + /* Pipe TCs */ > > > + if (unlikely(port->time >=3D pipe->tc_time)) { > > > + for (i =3D 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > > > + pipe->tc_credits[i] =3D params->tc_credits_per_period[i]; > > > + pipe->tc_time =3D port->time + params->tc_period; > > > + } > > > +} > > > + > > > +static inline void > > > +grinder_credits_update_with_tc_ov(struct rte_sched_port *port, > > > + struct rte_sched_subport *subport, uint32_t pos) { > > > + struct rte_sched_grinder *grinder =3D subport->grinder + pos; > > > + struct rte_sched_pipe *pipe =3D grinder->pipe; > > > + struct rte_sched_pipe_profile *params =3D grinder->pipe_params; > > > + struct rte_sched_subport_profile *sp =3D grinder->subport_params; > > > + uint64_t n_periods; > > > + uint32_t i; > > > + > > > + /* Subport TB */ > > > + n_periods =3D (port->time - subport->tb_time) / sp->tb_period; > > > + subport->tb_credits +=3D n_periods * sp->tb_credits_per_period; > > > + subport->tb_credits =3D RTE_MIN(subport->tb_credits, sp->tb_size); > > > + subport->tb_time +=3D n_periods * sp->tb_period; > > > + > > > + /* Pipe TB */ > > > + n_periods =3D (port->time - pipe->tb_time) / params->tb_period; > > > + pipe->tb_credits +=3D n_periods * params->tb_credits_per_period; > > > + pipe->tb_credits =3D RTE_MIN(pipe->tb_credits, params->tb_size); > > > + pipe->tb_time +=3D n_periods * params->tb_period; > > > + > > > /* Subport TCs */ > > > if (unlikely(port->time >=3D subport->tc_time)) { > > > subport->tc_ov_wm =3D > > > @@ -2348,6 +2428,39 @@ grinder_credits_update(struct rte_sched_port > > > *port, static inline int grinder_credits_check(struct rte_sched_por= t > > > *port, > > > struct rte_sched_subport *subport, uint32_t pos) > > > +{ > > > + struct rte_sched_grinder *grinder =3D subport->grinder + pos; > > > + struct rte_sched_pipe *pipe =3D grinder->pipe; > > > + struct rte_mbuf *pkt =3D grinder->pkt; > > > + uint32_t tc_index =3D grinder->tc_index; > > > + uint64_t pkt_len =3D pkt->pkt_len + port->frame_overhead; > > > + uint64_t subport_tb_credits =3D subport->tb_credits; > > > + uint64_t subport_tc_credits =3D subport->tc_credits[tc_index]; > > > + uint64_t pipe_tb_credits =3D pipe->tb_credits; > > > + uint64_t pipe_tc_credits =3D pipe->tc_credits[tc_index]; > > > + int enough_credits; > > > + > > > + /* Check pipe and subport credits */ > > > + enough_credits =3D (pkt_len <=3D subport_tb_credits) && > > > + (pkt_len <=3D subport_tc_credits) && > > > + (pkt_len <=3D pipe_tb_credits) && > > > + (pkt_len <=3D pipe_tc_credits); > > > + > > > + if (!enough_credits) > > > + return 0; > > > + > > > + /* Update pipe and subport credits */ > > > + subport->tb_credits -=3D pkt_len; > > > + subport->tc_credits[tc_index] -=3D pkt_len; > > > + pipe->tb_credits -=3D pkt_len; > > > + pipe->tc_credits[tc_index] -=3D pkt_len; > > > + > > > + return 1; > > > +} > > > + > > > +static inline int > > > +grinder_credits_check_with_tc_ov(struct rte_sched_port *port, > > > + struct rte_sched_subport *subport, uint32_t pos) > > > { > > > struct rte_sched_grinder *grinder =3D subport->grinder + pos; > > > struct rte_sched_pipe *pipe =3D grinder->pipe; @@ -2403,8 +2516,16 > @@ > > > grinder_schedule(struct rte_sched_port *port, > > > uint32_t pkt_len =3D pkt->pkt_len + port->frame_overhead; > > > uint32_t be_tc_active; > > > > > > - if (!grinder_credits_check(port, subport, pos)) > > > - return 0; > > > + switch (subport->is_tc_ov_enabled) { > > > + case 1: > > > + if (!grinder_credits_check_with_tc_ov(port, subport, pos)) > > > + return 0; > > > + break; > > > + case 0: > > > + if (!grinder_credits_check(port, subport, pos)) > > > + return 0; > > > + break; > > > + } > > > > There should be no switch statement here, please replace with an if > statement. I > > suggest the following: > > > > int status; > > > > status =3D subport->tc_ov_enabled ? grinder_credits_check_with_tc_ov(po= rt, > > subport, pos) : grinder_credits_check(port, subport, pos); if (!status) > > return 0; > > > > > > > > /* Advance port time */ > > > port->time +=3D pkt_len; > > > @@ -2770,7 +2891,11 @@ grinder_handle(struct rte_sched_port *port, > > > subport->profile; > > > > > > grinder_prefetch_tc_queue_arrays(subport, pos); > > > - grinder_credits_update(port, subport, pos); > > > + > > > + if (unlikely(subport->is_tc_ov_enabled)) > > > > Please remove the "unlikely" from here, don't put any likely/unlikely h= ere at > all. > > > > > + grinder_credits_update_with_tc_ov(port, subport, pos); > > > + else > > > + grinder_credits_update(port, subport, pos); > > > > > > grinder->state =3D e_GRINDER_PREFETCH_MBUF; > > > return 0; > > > diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h index > > > 5ece64e527..94febe1d94 100644 > > > --- a/lib/sched/rte_sched.h > > > +++ b/lib/sched/rte_sched.h > > > @@ -579,6 +579,24 @@ rte_sched_port_enqueue(struct rte_sched_port > > > *port, struct rte_mbuf **pkts, uint int > > > rte_sched_port_dequeue(struct rte_sched_port *port, struct rte_mbuf > > > **pkts, uint32_t n_pkts); > > > > > > +/** > > > + * Hierarchical scheduler subport TC OV enable/disable config. > > > > The name of the feature should be fully stated here: traffic class > > oversubscription, not the abbreviation, please change. > > > > > > > + * Note that this function is safe to use at runtime > > > + * to enable/disable TC OV for subport. > > > > We should actually forbit this rather than encourage it. Calling this f= unction > > several times does not make sense, and it can create limitations that c= an > come > > back and byte us in the future, whenever we might need to extend this c= ode, > for > > no reason. > > > > Please actually replace with: "This function should be called at the ti= me of > > subport initialization." > > > > > + * > > > + * @param port > > > + * Handle to port scheduler instance > > > + * @param subport_id > > > + * Subport ID > > > + * @param tc_ov_enable > > > + * Boolean flag to enable/disable TC OV > > > + * @return > > > + * 0 upon success, error code otherwise > > > + */ > > > +__rte_experimental > > > +int > > > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, uint32_t > > > subport_id, bool tc_ov_enable); > > > + > > > #ifdef __cplusplus > > > } > > > #endif > > > diff --git a/lib/sched/version.map b/lib/sched/version.map index > > > d22c07fc9f..c6e994d8df 100644 > > > --- a/lib/sched/version.map > > > +++ b/lib/sched/version.map > > > @@ -34,4 +34,7 @@ EXPERIMENTAL { > > > # added in 21.11 > > > rte_pie_rt_data_init; > > > rte_pie_config_init; > > > + > > > + # added in 22.03 > > > > This is not in 22.03, it will hopefully be in 22.07. > > > > > + rte_sched_subport_tc_ov_config; > > > }; > > > -- > > > 2.25.1 > > > > Regards, > > Cristian Regards, Cristian