Re: [PATCH net-next 5/8] net: sched: pie: add more conditions to auto-tune alpha and beta

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 31 Oct 2018 21:49:29 +0530
Leslie Monis <lesliemonis@gmail.com> wrote:

> From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>
> 
> The update in drop probability depends on the parameters
> alpha and beta, which in turn reflect the current congestion
> level. However, the previous if-else cases were recommended
> when the supported bandwidth was up to 12 Mbps but, current
> data links support a much higher bandwidth, and the
> requirement for more bandwidth is in never-ending demand.
> Hence, RFC 8033 suggests using more if-else cases for better
> fine-tuning of parameters alpha and beta in order to control
> the congestion as much as possible.
> 
> Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
> Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
> Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
> Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
> Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
> Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
> ---
>  net/sched/sch_pie.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
> index f4e189a..c84e91e 100644
> --- a/net/sched/sch_pie.c
> +++ b/net/sched/sch_pie.c
> @@ -343,10 +343,30 @@ static void calculate_probability(struct Qdisc *sch)
>  	 * appropriately 2) scaling down by 16 to come to 0-2 range.
>  	 * Please see paper for details.
>  	 *
> -	 * We scale alpha and beta differently depending on whether we are in
> -	 * light, medium or high dropping mode.
> +	 * We scale alpha and beta differently depending on how heavy the
> +	 * congestion is.
>  	 */
> -	if (q->vars.prob < MAX_PROB / 100) {
> +	if (q->vars.prob < MAX_PROB / 1000000) {
> +		alpha =
> +		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 15;
> +		beta =
> +		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 15;
> +	} else if (q->vars.prob < MAX_PROB / 100000) {
> +		alpha =
> +		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 13;
> +		beta =
> +		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 13;
> +	} else if (q->vars.prob < MAX_PROB / 10000) {
> +		alpha =
> +		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 11;
> +		beta =
> +		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 11;
> +	} else if (q->vars.prob < MAX_PROB / 1000) {
> +		alpha =
> +		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 9;
> +		beta =
> +		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 9;
> +	} else if (q->vars.prob < MAX_PROB / 100) {
>  		alpha =
>  		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 7;
>  		beta =


Seems like the if/else chain is getting long in the tail. Maybe a loop
or table driven approach would be clearer.