Hi Jasvinder, > -----Original Message----- > From: Singh, Jasvinder > Sent: Monday, September 9, 2019 11:05 AM > To: dev@dpdk.org > Cc: Dumitrescu, Cristian <cristian.dumitre...@intel.com> > Subject: [PATCH v2 00/15] sched: subport level configuration of pipe nodes > > This patchset refactors the dpdk qos sched library to allow subport > level configuration flexibility of the pipe nodes. > > 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. > > 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. > > In order to keep the implementation complexity under control, all > pipes within the same subport share the same configuration for queue > sizes. > > v2: > - fix qsize parsing in sample application > - fix checkpatch warnings > > 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 > > 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(-) > > -- > 2.21.0
Acked-by: Cristian Dumitrescu <cristian.dumitre...@intel.com> 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_subport. The former reserves a fixed number of bits in struct rte_mbuf::sched.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 subport, 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 say all subports have the same number of pipes (n_max_pipes_per_subport), but some of these pipes might not be implemented by all the subports. Maybe we should name the port parameter as n_pipes_per_subport and the subport parameter as n_pipes_per_subport_enabled? PS: I did check the critical functions rte_sched_port_qindex() and rte_sched_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 leftover or is this used to simplify the memory allocation? Assuming the latter, my vote is to remove it. Regards, Cristian
