[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(-)
  

Hi Jasvinder,

> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Monday, September 9, 2019 11:05 AM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitrescu@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.dumitrescu@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
  

> -----Original Message-----
> From: Dumitrescu, Cristian
> Sent: Monday, September 23, 2019 2:06 PM
> To: Singh, Jasvinder <jasvinder.singh@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH v2 00/15] sched: subport level configuration of pipe nodes
> 
> Hi Jasvinder,
> 
> > -----Original Message-----
> > From: Singh, Jasvinder
> > Sent: Monday, September 9, 2019 11:05 AM
> > To: dev@dpdk.org
> > Cc: Dumitrescu, Cristian <cristian.dumitrescu@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.dumitrescu@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. 

I will improve doxygen comments as suggested above, thanks.

Maybe we should name the port parameter
> as n_pipes_per_subport and the subport parameter as
> n_pipes_per_subport_enabled?

Will rename these parameters as suggested.

> 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?

Thanks for spotting this :)

> 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.

I will remove this field from the port structure. 

Thank you once again for review and comments, will fix above in v3 version.
Jasvinder