From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2A8A4A059F; Fri, 10 Apr 2020 13:45:13 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 757241D44E; Fri, 10 Apr 2020 13:45:12 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 386391D443 for ; Fri, 10 Apr 2020 13:45:10 +0200 (CEST) IronPort-SDR: o9v1pDmUEtrIYBrdGYFfPgPiAIkL/C4CjAGsjeaTFs+b/aT90/15qoHmF9TR5UpAb9huXCNPNP CZj863AXHPLQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Apr 2020 04:45:09 -0700 IronPort-SDR: EklZHAZrMjsh+tPQstej50hAAalvISQOjLvTDH7tgGqpRTOuRJC1u82nNVilhjRFeP6kXDvwPs mV7Os9GGXGNQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,366,1580803200"; d="scan'208";a="297789091" Received: from orsmsx107.amr.corp.intel.com ([10.22.240.5]) by FMSMGA003.fm.intel.com with ESMTP; 10 Apr 2020 04:45:09 -0700 Received: from orsmsx115.amr.corp.intel.com (10.22.240.11) by ORSMSX107.amr.corp.intel.com (10.22.240.5) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 10 Apr 2020 04:45:09 -0700 Received: from ORSEDG001.ED.cps.intel.com (10.7.248.4) by ORSMSX115.amr.corp.intel.com (10.22.240.11) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 10 Apr 2020 04:45:08 -0700 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.176) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 10 Apr 2020 04:45:08 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=f/9LED1074xGsUV2LEMkVjjPaHXUI04LhdGWPKGGns3qqO3r4QZQXJZ0OJ8yvbh2P8jkjao2EjQyFhL/IDGUDyRskwCjrjoJhG+rgtliHMw7X2ywfg4ugxY2E4/a79WrQeg6J/Cgnp7aM8aSd43YZZP7g2xXeChiCkwtDN2Ckiqu1b3gJnF7UIE3ipqH5MQ7k+6VwROIx4mtFCVt4q9/Z7jAcKDzaYln+yOYxIRkkl3zAfN/0X5+G36iD8tqCtu7nMVENUO4HTmnPcFpCxJhk6SfIhnv6WWKQTXikxp93fHlTePUMxJlHBIwTa8XEtmaAMt7RLNPmqcCCaxNEpWXRA== 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=48POFl2fvG1bJLgYmJmpppPnog7Ji1GBIhn3TkSw/Ek=; b=BXoFyEWO/pw0MmEOsaPHHKuLbVTryi67DLOaJoosdXcueICGSHnjUhqzHxc9fv1qPC8QxgiE2aVYF2iDvp+xyfNSVuqSoS0Zcoez4KxUO8GfTZ+keN4zPNEE8DKsKZkJhw+oiyhDQaKRirlCd4Qse3t7fz3bkrNZJr27Rmi0IRhASmquT3wWe9U8oMsqn9S7eOM+EHc9173pCceMvmljFhCfyFzZa8ImVhwodAAWYqNklySanlUSfuV7XpHyGvA2jaVChGQIJLcpxHf7RY5ga4ssrYFuQD36DLTa7Gi8AMOePeyJ/xkbrBECGq7a2Pj7L+Lj2H14EuB/TZBYLlQouQ== 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=48POFl2fvG1bJLgYmJmpppPnog7Ji1GBIhn3TkSw/Ek=; b=Mqa6+r9V4g6m58VaOy2HlU3lHo5zOSYj0gPCeJ/tyA/9c4MxEqX7wKInOjL3CGEr8rM8NLkh8NdQrHb/Q/CduKQBiP5ddym6HQRPQ1xHAdSRbK+AQ3IDDiCX2B4HPnMSyU6BBMwEGxvICvJTYzKcoXgBwSjYjSnRdFi+Y72z/8M= Received: from BYAPR11MB2935.namprd11.prod.outlook.com (2603:10b6:a03:82::24) by BYAPR11MB2903.namprd11.prod.outlook.com (2603:10b6:a03:89::31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.15; Fri, 10 Apr 2020 11:45:06 +0000 Received: from BYAPR11MB2935.namprd11.prod.outlook.com ([fe80::786e:a42b:df03:a829]) by BYAPR11MB2935.namprd11.prod.outlook.com ([fe80::786e:a42b:df03:a829%5]) with mapi id 15.20.2878.021; Fri, 10 Apr 2020 11:45:06 +0000 From: "Dumitrescu, Cristian" To: Nithin Dabilpuram CC: Thomas Monjalon , "Yigit, Ferruh" , Andrew Rybchenko , "dev@dpdk.org" , "jerinj@marvell.com" , "kkanas@marvell.com" Thread-Topic: [EXT] RE: [PATCH 1/2] ethdev: add tm cap for private shaper packet mode Thread-Index: AQHWBqyvHJ/9k4iRG0Go4EWnkPq7Q6ht2WbAgAAbGACABFW1EA== Date: Fri, 10 Apr 2020 11:45:06 +0000 Message-ID: References: <20200330160019.29674-1-ndabilpuram@marvell.com> <20200407172106.GA4853@outlook.office365.com> In-Reply-To: <20200407172106.GA4853@outlook.office365.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.2.0.6 dlp-reaction: no-action dlp-product: dlpe-windows authentication-results: spf=none (sender IP is ) smtp.mailfrom=cristian.dumitrescu@intel.com; x-originating-ip: [192.198.151.175] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 32b88245-29bc-45b8-7937-08d7dd449d57 x-ms-traffictypediagnostic: BYAPR11MB2903: 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-forefront-prvs: 0369E8196C x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB2935.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10019020)(136003)(366004)(346002)(39860400002)(396003)(376002)(6916009)(53546011)(54906003)(66446008)(186003)(71200400001)(478600001)(64756008)(66556008)(26005)(76116006)(7696005)(6506007)(30864003)(9686003)(52536014)(55016002)(316002)(81156014)(66946007)(66476007)(2906002)(8676002)(33656002)(4326008)(5660300002)(86362001)(8936002)(290074003); DIR:OUT; SFP:1102; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: P6y1UZgtWpydnEHFRtLJ5HsTrONlCG5erEruSOB8j88DR/EaV9F8G8/sA6uxHujTk7pFX+DCXwrXDc9hXK0W8KOX/Mu+mNLzU+dZfsknmppEjfXK7LD1YrNS1/eH8I9dYe8X8Wy0AZPuCwe369zZQuK0GeaEGNe8ejrjreLzCtfoVXd9XJFlZTbEzc4CEYKCqY7vxftb204J1GRmRYlKhEA2ZsV7+zTGh4F/3sSBDRQWPgPC2GgYSQOO/5JnB4XnIuzRvudYpQLn1dB/edocUiFQQZr12wgoP4/UUGP/oTQRhhhlk0DbjQL74OFtxAte9DbjrvOal4Q0zdfkcHlQqA/Fy5wvR2d23QxUrg/7k1Zh9SsC0rWHlWtP0xFuxWpuGYMlgzKYM36dB6ydKXKrpUKYl5w+Wbw5BvwUXGQqe710WsIOoA20I+SuMbXY3sZ/AT+K/7sIultEhPeXIHzWeTa4w2a58K6mlY/8W+D9ht4DaHmQOzc0xYpIWY4jvpLX x-ms-exchange-antispam-messagedata: fP5G/2HsM91umEDxdXWVJDnUsH1iglnhc445u6ZXawaFFL+zA/xMi727ViwWHkvQuynJ/uJEv203ySBrQJg3km+UmmnH4wDh4CsNJ9GfnNRf4qSkCgy+kyCzUASAerDTMjAKcDR61M7/CEsxke7Y3g== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 32b88245-29bc-45b8-7937-08d7dd449d57 X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Apr 2020 11:45:06.1355 (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: vM2Fb92iO8/KqzO1TH87djCjN/C0n/dtWuesr84guL+gKvLKWWINvg9MlWU7G7onOlAL2GV3dpvL55ov7akAGEvQILDKJKsU8jGoHma9M1c= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB2903 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [EXT] RE: [PATCH 1/2] ethdev: add tm cap for private shaper packet mode X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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: Nithin Dabilpuram > Sent: Tuesday, April 7, 2020 6:21 PM > To: Dumitrescu, Cristian > Cc: Thomas Monjalon ; Yigit, Ferruh > ; Andrew Rybchenko > ; dev@dpdk.org; jerinj@marvell.com; > kkanas@marvell.com > Subject: Re: [EXT] RE: [PATCH 1/2] ethdev: add tm cap for private shaper > packet mode >=20 > Hi Cristian, >=20 > Thanks for your comments. I have some queries below. > On Tue, Apr 07, 2020 at 04:31:47PM +0000, Dumitrescu, Cristian wrote: > > External Email > > > > ---------------------------------------------------------------------- > > Hi Nithin, > > > > > -----Original Message----- > > > From: Nithin Dabilpuram > > > Sent: Monday, March 30, 2020 5:00 PM > > > To: Dumitrescu, Cristian ; Thomas > Monjalon > > > ; Yigit, Ferruh ; > Andrew > > > Rybchenko > > > Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin > > > Dabilpuram > > > Subject: [PATCH 1/2] ethdev: add tm cap for private shaper packet mod= e > > > > > > Some NIC hardware have private shaper attached to > > > every node and has a limitation where packet mode is applied > > > both to the scheduling of a node's children using WFQ and > > > shaping of traffic out of the private shaper. > > > This cannot be expressed using existing capabilities or configuration= s. > > > > > > So this patch adds a tm capability that if set by a PMD implies that > > > packet mode when configured is even applied to private shaper > > > connected to that node. This also implies the limitation > > > that all the SP children of that node should have same mode > > > at any point of time i.e either packet mode or byte mode and > > > same applies to private shaper in that NIC PMD. > > > > > > This patch also adds missing capability that tells whether PMD > > > supports wfq weight mode or not. > > > > > > Signed-off-by: Nithin Dabilpuram > > > --- > > > lib/librte_ethdev/rte_tm.h | 62 > > > +++++++++++++++++++++++++++++++++++++++++++--- > > > 1 file changed, 59 insertions(+), 3 deletions(-) > > > > > > diff --git a/lib/librte_ethdev/rte_tm.h b/lib/librte_ethdev/rte_tm.h > > > index f9c0cf3..50bcea6 100644 > > > --- a/lib/librte_ethdev/rte_tm.h > > > +++ b/lib/librte_ethdev/rte_tm.h > > > @@ -339,6 +339,20 @@ struct rte_tm_capabilities { > > > */ > > > uint32_t sched_wfq_weight_max; > > > > > > + /** WFQ weight mode supported. Non-zero value indicates wfq > > > weight mode > > > + * is supported and a SP child (even a wfq group) can be configured > > > to > > > + * use packet-mode or byte-mode for weight calculations. > > > + */ > > > + int sched_wfq_weight_mode_supported; > > > + > > > > This is incorrect, as the WFQ support, including the weight aspect of W= FQ, is > already part of the existing set of capabilities: see sched_wfq_weight_ma= x > and the other sched_wfq_* capability fields >=20 > This is a missing capability for an existing functionality. > "struct rte_tm_node_params:nonleaf.wfq_weight_mode" field could be > used to toggle between > packet-mode or byte-mode for WFQ weights. >=20 > The field if NULL also says that mode defaults to byte-mode. >=20 This capability field should be split into sched_wfq_weight_byte_mode_suppo= rted and sched_wfq_weight_packet_mode_supported. I agree that both of these modes are already supported by the API, but not = explicitly mentioned in capability structure, so I think it makes sense to = add them to capabilities too. We should have the same look & feel for all t= he features that accept byte mode and packet mode, i.e. WFQ weight, shaper = rate, WRED thresholds. Makes sense? > > > > > + /** Private shaper and scheduler weight mode. > > > + * When non-zero value indicates that all SP children should have > > > + * same weight mode and the same mode applies to private > > > + * shaper as well. This is only valid if > > > + * *sched_wfq_weight_mode_supported* is set. > > > + */ > > > + int sched_shaper_private_weight_mode; > > > + > > > > If I understand your intention correctly, you are trying to introduce p= acket > mode (in addition to the existing byte mode) for (1) scheduler WFQ weight= s > and for (2) shaper rates. Basically, the ability to express WFQ weights i= n bytes > as well as packets, and the ability to express shaper rates in bytes and = well as > packets. Is this correct? >=20 > Isn't packet mode for (1) already supported via > "struct rte_tm_node_params:nonleaf.wfq_weight_mode" and > rte_tm_node_wfq_weight_mode_update() ? >=20 > I'm trying to add support for packet-mode for (2). >=20 See previous note. > > > > Assuming yes, we probably need to do it in a slightly different way: > > 1/ Similar to the WRED packet mode that was introduced by Nikhil Rao's > patches a while ago (in addition to WRED's byte mode), see WRED capabilit= y > and configuration. > > 2/ Decouple between scheduler and shaper. So we should add sched_* > fields and shaper_* fields, but never sched_shaper_*, as it creates a > functional dependency that does not exist. > > > > In line with methodology already used for WRED, I suggest: > > a) Scheduler WFQ capabilities (TM/level/node): > sched_wfq_packet_mode_supported, sched_wfq_byte_mode_supported >=20 > I'll add "sched_wfq_packet_mode_supported" field. >=20 > Since currently when "struct > rte_tm_node_params:nonleaf.wfq_weight_mode" is NULL, > mode defaults to byte mode, is it needed to have > "sched_wfq_byte_mode_supported" ? > Or I should also update the text in "struct rte_tm_node_params" ? >=20 See previous comment. Yes, it makes sense to add both sched_wfq_weight_byte= _mode_supported and sched_wfq_weight_packet_mode_supported to capabilities = structure, even though they refer to features already supported by the API.= We should have the same look 7 feel for all features that support byte mod= e and packet mode. >=20 > > b) Shaper capabilities (TM/level/node): > shaper_rate_packet_mode_supported, > shaper_rate_byte_mode_supported. > Ack. OK, great. > > c) Shaper profile (struct rte_tm_shaper_params): add an integer > packet_mode flag with 0 =3D byte-mode (default) and 1 =3D packet mode for= the > values in struct rte_tm_token_bucket. >=20 > Ok. I'll add a field "packet-mode" in rte_tm_shaper_params and enforce > restrictions in PMD. OK, great. > > > > It is important to note that the API must allow a combination of packet > mode and byte mode (for different nodes, not for the same), but an > implementation can support either a single mode or both (should be > enforced by the driver). > > > > > /** WRED packet mode support. When non-zero, this parameter > > > indicates > > > * that there is at least one leaf node that supports the WRED pack= et > > > * mode, which might not be true for all the leaf nodes. In packet > > > @@ -554,6 +568,21 @@ struct rte_tm_level_capabilities { > > > */ > > > uint32_t sched_wfq_weight_max; > > > > > > + /** WFQ weight mode supported. Non-zero value > > > indicates > > > + * wfq weight mode is supported and a SP child > > > + * (even a wfq group) can be configured to use > > > + * packet-mode or byte-mode for weight > > > calculations. > > > + */ > > > + int sched_wfq_weight_mode_supported; > > > + > > > + /** Private shaper and scheduler weight mode. > > > + * When non-zero value indicates that all SP children > > > + * should have same weight mode and the same > > > mode > > > + * applies to private shaper as well. This is only > > > + * valid if *sched_wfq_weight_mode_supported* is > > > set. > > > + */ > > > + int sched_shaper_private_weight_mode; > > > + > > > /** Mask of statistics counter types supported by the > > > * non-leaf nodes on this level. Every supported > > > * statistics counter type is supported by at least one > > > > See above the comments on TM capabilities. > > > > > @@ -735,6 +764,21 @@ struct rte_tm_node_capabilities { > > > * WFQ weight, so WFQ is reduced to FQ. > > > */ > > > uint32_t sched_wfq_weight_max; > > > + > > > + /** WFQ weight mode supported. Non-zero value > > > indicates > > > + * wfq weight mode is supported and a SP child > > > + * (even a wfq group) can be configured to use > > > + * packet-mode or byte-mode for weight > > > calculations. > > > + */ > > > + int sched_wfq_weight_mode_supported; > > > + > > > + /** Private shaper and scheduler weight mode. > > > + * When non-zero value indicates that all SP children > > > + * should have same weight mode and the same > > > mode > > > + * applies to private shaper as well. This is only > > > + * valid if *sched_wfq_weight_mode_supported* is > > > set. > > > + */ > > > + int sched_shaper_private_weight_mode; > > > } nonleaf; > > > > > > /** Items valid only for leaf nodes. */ > > > > See above the comments on the TM capabilities. > > > > > @@ -836,10 +880,19 @@ struct rte_tm_wred_params { > > > * Token bucket > > > */ > > > struct rte_tm_token_bucket { > > > - /** Token bucket rate (bytes per second) */ > > > + /** Token bucket rate. This is in "bytes per second" by default. > > > + * For private shaper attached to node that is set in packet mode > > > + * and tm capability *sched_shaper_private_weight_mode* is set, > > > + * this is interpreted as "packets per second". > > > + */ > > > uint64_t rate; > > > > > > - /** Token bucket size (bytes), a.k.a. max burst size */ > > > + /** Token bucket size, a.k.a. max burst size. > > > + * This is in "bytes" by default. > > > + * For private shaper attached to node that is set in packet mode > > > + * and tm capability *sched_shaper_private_weight_mode* is set, > > > + * this is interpreted as "packets". > > > + */ > > > uint64_t size; > > > }; > > > > > > > Comments are not correct, as API should allow a combination of both the > packet mode and the byte mode (for different nodes, not for the same > node), so both capabilities shaper_rate_packet_mode and > shaper_rate_byte_mode can be set. Hence, the comments should not > specify a capability, but the fact that these values can specify either b= yte or > packets, depending on a flag elsewhere. >=20 > Ok. As per your above comment I'll add field "packet-mode" in "struct > rte_tm_shaper_params" and update this comment accordingly. OK, great. > > > > > @@ -924,7 +977,10 @@ struct rte_tm_node_params { > > > * indicates that WFQ is to be used for all priorities. > > > * When non-NULL, it points to a pre-allocated array > > > of > > > * *n_sp_priorities* values, with non-zero value for > > > - * byte-mode and zero for packet-mode. > > > + * byte-mode and zero for packet-mode. The same > > > mode is > > > + * used for private shaper connected to this node if > > > + * tm capability > > > *sched_shaper_private_weight_mode* is > > > + * true. > > > */ > > > > This comment is incorrect, as sched should not be combined with shaper. > The user should select between packet mode and byte mode for the WFQ > weight independently of the mode for the shaper rate, although an > implementation (driver) should enforce the correct values. > > > > > int *wfq_weight_mode; > > > > > > -- > > > 2.8.4 > > > > > > Regards, > > Cristian