[dpdk-dev] [RFC PATCH v4 0/3] Add PIE support for HQoS library

[Dumitrescu, Cristian]

Hi Wojciech,

Thank you for doing this work!

> -----Original Message-----
> From: Liguzinski, WojciechX <wojciechx.liguzinski at intel.com>
> Sent: Monday, July 5, 2021 9:04 AM
> To: dev at dpdk.org; Singh, Jasvinder <jasvinder.singh at intel.com>;
> Dumitrescu, Cristian <cristian.dumitrescu at intel.com>
> Cc: Dharmappa, Savinay <savinay.dharmappa at intel.com>; Ajmera, Megha
> <megha.ajmera at intel.com>
> Subject: [RFC PATCH v4 0/3] Add PIE support for HQoS library
> 
> DPDK sched library is equipped with mechanism that secures it from the
> bufferbloat problem
> which is a situation when excess buffers in the network cause high latency
> and latency
> variation. Currently, it supports RED for active queue management (which is
> designed
> to control the queue length but it does not control latency directly and is now
> being
> obsoleted). However, more advanced queue management is required to
> address this problem
> and provide desirable quality of service to users.

As already mentioned by other reviewers, I don't think RED/WRED is getting obsoleted. This entire paragraph is a bit fuzzy and not really adding much value IMO, I propose to remove it.

> 
> This solution (RFC) proposes usage of new algorithm called "PIE"
> (Proportional Integral
> controller Enhanced) that can effectively and directly control queuing latency
> to address
> the bufferbloat problem.

Please add a link to the public RFC for PIE in this cover letter.

> 
> The implementation of mentioned functionality includes modification of
> existing and
> adding a new set of data structures to the library, adding PIE related APIs.
> This affects structures in public API/ABI. That is why deprecation notice is
> going
> to be prepared and sent.

I think you are stating the obvious here, how about removing this paragraph as well?

> 
> Liguzinski, WojciechX (3):
>   sched: add PIE based congestion management
>   example/qos_sched: add PIE support
>   example/ip_pipeline: add PIE support
> 
>  config/rte_config.h                      |   1 -
>  drivers/net/softnic/rte_eth_softnic_tm.c |   6 +-
>  examples/ip_pipeline/tmgr.c              |   6 +-
>  examples/qos_sched/app_thread.c          |   1 -
>  examples/qos_sched/cfg_file.c            |  82 ++++-
>  examples/qos_sched/init.c                |   7 +-
>  examples/qos_sched/profile.cfg           | 196 +++++++----
>  lib/sched/meson.build                    |  10 +-
>  lib/sched/rte_pie.c                      |  82 +++++
>  lib/sched/rte_pie.h                      | 393 +++++++++++++++++++++++
>  lib/sched/rte_sched.c                    | 229 +++++++++----
>  lib/sched/rte_sched.h                    |  53 ++-
>  lib/sched/version.map                    |   3 +
>  13 files changed, 888 insertions(+), 181 deletions(-)
>  create mode 100644 lib/sched/rte_pie.c
>  create mode 100644 lib/sched/rte_pie.h
> 
> --
> 2.17.1

Regards,
Cristian