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 BDE5EA059F; Fri, 10 Apr 2020 13:57:07 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8AB0E1D513; Fri, 10 Apr 2020 13:57:06 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by dpdk.org (Postfix) with ESMTP id 67A171D168 for ; Fri, 10 Apr 2020 13:57:05 +0200 (CEST) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 03ABspO7022133; Fri, 10 Apr 2020 04:57:04 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=date : from : to : cc : subject : message-id : references : content-type : in-reply-to : mime-version; s=pfpt0818; bh=FppBgbcn1PV5EqzCNahjROF4p61bFCEfTtKUmvaPeUo=; b=QmbyUqTfD41TdrYH7JYbcTSoFkGOzt4SOrFP0L3JriXuo81KLIV5/gqZeX6SYTeRFtvn GfdavTFhD+sVUY0SA8vlLo9euyhCDVuojpVyqALXnCQepKPdxhvONcsvheoggvU4HTxs jhCu1ekaycojMsrkMgT3GAnkdBLRXCQFvWn4M6ZsOziD09DxhobKFkX/I/3lO8MJzWqD FqnQToR/g24bwLjXiZgL12KeNpsfkklWjGBvQE5gExUjGJ16LtOo3GrixeAdEpQ8z22+ m5Y+yPyWoTYCRfqk9EyrTTohU2eEZNqsA6BoSTSPshZIFp+OD3Da+yv3PPud6YiwxGdO og== Received: from sc-exch04.marvell.com ([199.233.58.184]) by mx0b-0016f401.pphosted.com with ESMTP id 3091mecxx0-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Fri, 10 Apr 2020 04:57:04 -0700 Received: from SC-EXCH02.marvell.com (10.93.176.82) by SC-EXCH04.marvell.com (10.93.176.84) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Fri, 10 Apr 2020 04:57:02 -0700 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.109) by SC-EXCH02.marvell.com (10.93.176.82) with Microsoft SMTP Server (TLS) id 15.0.1497.2 via Frontend Transport; Fri, 10 Apr 2020 04:57:01 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Gfr5fP0RRFzU3nws/btk0kcGO9hpdBsgmOT9dCD2HMPgl5K+UVeWkk9MC2bwU1K4hKasmbK7RoTFyeet8Vug9LwLf7/MNC2daeGQYp53kzw59t7w5TlZURJavPJqR8MPgd1TSPRAyDJalIP/ZzI0Wn2N5X7OZVB/ddu4ixY6xpuH2+ke7HRbCv+O+wBnEf5LcKKI+wkFX4L20u0tZ41MeWetuK29aZqRn6aIwsZLtiau0bMAvILOTuXg2VGLtDakk+5bCZU4YzT8kI2qY5tDchK7UR8IA+x+XLvcdwMrnihzeg0EXTEHbfVcoyvq+Lh5xIWmPXTy+WgOkpbRshdJqw== 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=FppBgbcn1PV5EqzCNahjROF4p61bFCEfTtKUmvaPeUo=; b=chTw9Do6TjduzB/8zeB47/WDcQVU2HotZEzrc0whG2eGF932fy9QshMThr1awTuZ4RjuYwIn2aROE73Gdt//RGaHdoSfO3q7I4+QgeN+rf3DF58/6kiy/CaBbWJEaKsom7bqjt+lmMZzh40TlrpTeuhcDGQ4sjkwF2ZkYQBeLGMw6pZFhXrcl0efc1YarDokOjxjT108T3PPvKkCMruc7G5ljHlwPfxI+ymx3Kx3Ik1jkMgejO+MMlq/4S2QhQXfCoVST+qi9Zps8TE+lCBa8+4lMrdtyzhMm5VGmWIK7wN/VYmvWnu5I3T/6v/U8yEyu/yKW4c2rs9y7It7BpiMcQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com; dkim=pass header.d=marvell.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector1-marvell-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=FppBgbcn1PV5EqzCNahjROF4p61bFCEfTtKUmvaPeUo=; b=iThj0f+1qqY4OPS0pBAFSPjVlLfBdroHujDVIotDCeTFKUNeRF5sGzckDcPKUpNYabetG26vJc9jlgvlPJ5gNtH6UaS0yArJW2sD6UBcvdqAbtujfqltPSeNlyneTLxgwSs77l0n/xYuDTEWVMBoZEmztBl6GyTbAXOFaAR9xIw= Received: from MWHPR1801MB2063.namprd18.prod.outlook.com (2603:10b6:301:6a::11) by MWHPR1801MB2061.namprd18.prod.outlook.com (2603:10b6:301:69::38) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2900.15; Fri, 10 Apr 2020 11:56:59 +0000 Received: from MWHPR1801MB2063.namprd18.prod.outlook.com ([fe80::380f:5ca1:ce60:6586]) by MWHPR1801MB2063.namprd18.prod.outlook.com ([fe80::380f:5ca1:ce60:6586%7]) with mapi id 15.20.2856.027; Fri, 10 Apr 2020 11:56:59 +0000 Date: Fri, 10 Apr 2020 17:26:43 +0530 From: Nithin Dabilpuram To: "Dumitrescu, Cristian" CC: Thomas Monjalon , "Yigit, Ferruh" , Andrew Rybchenko , "dev@dpdk.org" , "jerinj@marvell.com" , "kkanas@marvell.com" Message-ID: <20200410115643.GA24080@outlook.office365.com> References: <20200330160019.29674-1-ndabilpuram@marvell.com> <20200407172106.GA4853@outlook.office365.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.2 (34cd43c) (2019-09-21) X-ClientProxiedBy: PN1PR0101CA0019.INDPRD01.PROD.OUTLOOK.COM (2603:1096:c00:e::29) To MWHPR1801MB2063.namprd18.prod.outlook.com (2603:10b6:301:6a::11) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from outlook.office365.com (115.113.156.2) by PN1PR0101CA0019.INDPRD01.PROD.OUTLOOK.COM (2603:1096:c00:e::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2900.17 via Frontend Transport; Fri, 10 Apr 2020 11:56:56 +0000 X-Originating-IP: [115.113.156.2] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: ca6b7d4e-4669-46e1-805f-08d7dd46460d X-MS-TrafficTypeDiagnostic: MWHPR1801MB2061: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-Forefront-PRVS: 0369E8196C X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR1801MB2063.namprd18.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10009020)(4636009)(376002)(346002)(396003)(366004)(136003)(39860400002)(66556008)(86362001)(66946007)(2906002)(316002)(6666004)(1076003)(54906003)(30864003)(107886003)(5660300002)(4326008)(26005)(6916009)(55016002)(52116002)(7696005)(9686003)(53546011)(6506007)(956004)(478600001)(8936002)(16526019)(81156014)(186003)(8676002)(66476007)(55236004)(33656002)(290074003); DIR:OUT; SFP:1101; Received-SPF: None (protection.outlook.com: marvell.com does not designate permitted sender hosts) X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: BCTsHHh7yhQQTWzRBlN42ccCMI6HQ8Ep0kKRKnCKKnnSSMfWoLnDFxcwOskmoSB61HQihIX+grOY9cAcF1kavyf/cmphjnfMqxUPqriZzJt19C6QPnWW0kbnOIk/MVcbJ14yrvZtMadXFS/1KemwccsxlqlAfxYmECA9k1drWV4DYzAoBsJ6bCzHaogULcOwjn861ZSB0vfcTSLGPGGjuXYLbBXRvKohnkaPWauZo4C2C3GvuKeTpzVrCPXGuWUQjp0oB9l7Bwczkl6u9uV/t5fFuUuUjsCTw1HuPJFJdI75b83duCGCW1+r8NinScUiKZFTDHhjvm0rxzDa5ur9qsu1QNPpiMqkwTO7HwUKvs7fZbKLGn94hZmLZdtzFOPbol3uv/TVVk2bdTnA3z901Ng0Ez596UzzpRIakBMkqhVtpGyR3SersPiASd4Ltfi1edCbOzBbV4H+SNIbg0m2z9QpLXGW7GAAmycbftbNWQD9/74DTo/3zdZNuGAjhu89 X-MS-Exchange-AntiSpam-MessageData: Wf87NzPvCanwcL37nOvCUPgmXBlgAIl1776ggXmf+7YuXbZshVfSr/uuRscPAEU32t0Rdy3b3m5+GwhPqvRGhrCUT0EHB7mHevP7FN2KKK6lOtMyPOtlF7u+hZlFdiFZmknBQ8VFjre+V52qWQ1+8w== X-MS-Exchange-CrossTenant-Network-Message-Id: ca6b7d4e-4669-46e1-805f-08d7dd46460d X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Apr 2020 11:56:59.4240 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: wTI/ASvftvuFn64/dg9myt58YFWZLrhPiBrhultbsg4AmCNQtpChIrEEDvCIAiuoKH4Mf095DGEe6KKCQJnC3A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR1801MB2061 X-OriginatorOrg: marvell.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138, 18.0.676 definitions=2020-04-10_03:2020-04-09, 2020-04-10 signatures=0 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" Thanks Cristian. Agree with your comments, Will send a v2 addressing them. On Fri, Apr 10, 2020 at 11:45:06AM +0000, Dumitrescu, Cristian wrote: > > > > -----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 > > > > Hi Cristian, > > > > 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 mode > > > > > > > > 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 configurations. > > > > > > > > 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 WFQ, is > > already part of the existing set of capabilities: see sched_wfq_weight_max > > and the other sched_wfq_* capability fields > > > > 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. > > > > The field if NULL also says that mode defaults to byte-mode. > > > > This capability field should be split into sched_wfq_weight_byte_mode_supported 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 the 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 packet > > mode (in addition to the existing byte mode) for (1) scheduler WFQ weights > > and for (2) shaper rates. Basically, the ability to express WFQ weights in bytes > > as well as packets, and the ability to express shaper rates in bytes and well as > > packets. Is this correct? > > > > 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() ? > > > > I'm trying to add support for packet-mode for (2). > > > > 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 capability > > 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 > > > > I'll add "sched_wfq_packet_mode_supported" field. > > > > 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" ? > > > > 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 mode and packet mode. > > > > > > 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 = byte-mode (default) and 1 = packet mode for the > > values in struct rte_tm_token_bucket. > > > > 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 packet > > > > * 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 byte or > > packets, depending on a flag elsewhere. > > > > 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