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 F007BA0613 for ; Mon, 23 Sep 2019 15:06:11 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7C8A01BEE3; Mon, 23 Sep 2019 15:06:10 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 357EB1BEE2 for ; Mon, 23 Sep 2019 15:06:09 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 Sep 2019 06:06:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,539,1559545200"; d="scan'208";a="179119910" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by orsmga007.jf.intel.com with ESMTP; 23 Sep 2019 06:06:06 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.26]) by irsmsx110.ger.corp.intel.com ([169.254.15.189]) with mapi id 14.03.0439.000; Mon, 23 Sep 2019 14:06:05 +0100 From: "Dumitrescu, Cristian" To: "Singh, Jasvinder" , "dev@dpdk.org" Thread-Topic: [PATCH v2 00/15] sched: subport level configuration of pipe nodes Thread-Index: AQHVZvYqkikf5s3BAkKICZUSVlptJKc5S9sQ Date: Mon, 23 Sep 2019 13:06:05 +0000 Message-ID: <3EB4FA525960D640B5BDFFD6A3D891268E92F889@IRSMSX108.ger.corp.intel.com> References: <20190823144602.58213-1-jasvinder.singh@intel.com> <20190909100530.86020-1-jasvinder.singh@intel.com> In-Reply-To: <20190909100530.86020-1-jasvinder.singh@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYzgxNzhlNTctYWNhZC00NDE2LWE5MDctZDViOTZhMTlmOGJlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUE5zN1wvTnY1STVVNGJkcG1ybEtYWXl2S21zTVVpam4wakhCXC9HZUxLejRLZjJZbHF0VFliYkc3OUM5K0xLWFlqIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 00/15] sched: subport level configuration of pipe nodes 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" Hi Jasvinder, > -----Original Message----- > From: Singh, Jasvinder > Sent: Monday, September 9, 2019 11:05 AM > To: dev@dpdk.org > Cc: Dumitrescu, Cristian > Subject: [PATCH v2 00/15] sched: subport level configuration of pipe node= s >=20 > This patchset refactors the dpdk qos sched library to allow subport > level configuration flexibility of the pipe nodes. >=20 > Currently, all parameters for the pipe nodes (subscribers) > configuration are part of the port level structure which forces > all groups of subscribers (pipes) in different subports to > have similar configurations in terms of their number, queue sizes, > traffic-classes, etc. >=20 > The new implementation moves pipe nodes configuration parameters > from port level to subport level structure. This allows different > subports of the same port to have different configuration for the > pipe nodes, for examples- number of pipes, queue sizes, pipe > profiles, etc. >=20 > In order to keep the implementation complexity under control, all > pipes within the same subport share the same configuration for queue > sizes. >=20 > v2: > - fix qsize parsing in sample application > - fix checkpatch warnings >=20 > Jasvinder Singh (15): > sched: add pipe config params to subport struct > sched: modify internal structs for config flexibility > sched: remove pipe params config from port level > shced: add pipe config to subport level > sched: modify pipe functions for config flexibility > sched: modify pkt enqueue for config flexibility > sched: update memory compute to support flexiblity > sched: update grinder functions for config flexibility > sched: update pkt dequeue for flexible config > sched: update queue stats read for config flexibility > test/sched: modify tests for subport config flexibility > net/softnic: add subport config flexibility to TM > ip_pipeline: add subport config flexibility to TM > examples/qos_sched: add subport configuration flexibility > sched: remove redundant code >=20 > app/test/test_sched.c | 35 +- > doc/guides/rel_notes/release_19_11.rst | 6 +- > drivers/net/softnic/rte_eth_softnic_tm.c | 51 +- > examples/ip_pipeline/cli.c | 71 +- > examples/ip_pipeline/tmgr.c | 23 +- > examples/ip_pipeline/tmgr.h | 7 +- > examples/qos_sched/app_thread.c | 4 +- > examples/qos_sched/cfg_file.c | 229 ++-- > examples/qos_sched/init.c | 54 +- > examples/qos_sched/main.h | 1 + > examples/qos_sched/profile.cfg | 5 +- > examples/qos_sched/profile_ov.cfg | 5 +- > examples/qos_sched/stats.c | 36 +- > lib/librte_sched/Makefile | 2 +- > lib/librte_sched/meson.build | 2 +- > lib/librte_sched/rte_sched.c | 1400 +++++++++++++--------- > lib/librte_sched/rte_sched.h | 114 +- > 17 files changed, 1183 insertions(+), 862 deletions(-) >=20 > -- > 2.21.0 Acked-by: Cristian Dumitrescu A few comments/suggestion, mainly on some API name changes: 1. We need a good comment explaining the difference between rte_sched_port_= params::n_max_pipes_per_subport and rte_sched_subport_params::n_pipes_per_s= ubport. The former reserves a fixed number of bits in struct rte_mbuf::sche= d.queue_id for the pipe_id for all the subports of the same port, while the= latter provides a mechanism to enable/allocate fewer pipes for each subpor= t, as needed, with the benefit of avoiding memory allocation for the queues= of the pipes that are not really needed. Another way to look at it is to s= ay all subports have the same number of pipes (n_max_pipes_per_subport), bu= t some of these pipes might not be implemented by all the subports. Maybe w= e should name the port parameter as n_pipes_per_subport and the subport par= ameter as n_pipes_per_subport_enabled? PS: I did check the critical functions rte_sched_port_qindex() and rte_sche= d_port_pkt_read_tree_path(), and they look good to me. 2. The rte_sched_PORT_pipe_profile_add() should probably be now renamed as = rte_sched_SUBPORT_pipe_profile_add(), right? 3. The port parameter n_max_pipe_profiles does not make sense to me, as we = are now introducing the n_pipe_profiles subport parameter. Is this a code l= eftover or is this used to simplify the memory allocation? Assuming the lat= ter, my vote is to remove it. Regards, Cristian