RE: [PATCH v6] sched: enable traffic class oversubscription conditionally

[Dumitrescu, Cristian]

Hi Marcin,

Comments inline below.

<snip>

> @@ -2403,8 +2481,16 @@ grinder_schedule(struct rte_sched_port *port,
>       uint32_t pkt_len = pkt->pkt_len + port->frame_overhead;
>       uint32_t be_tc_active;
> 
> -     if (!grinder_credits_check(port, subport, pos))
> -             return 0;
> +     switch (subport->tc_ov_enabled) {
> +     case 1:
> +             if (!grinder_credits_check_with_tc_ov(port, subport, pos))
> +                     return 0;
> +             break;
> +     case 0:
> +             if (!grinder_credits_check(port, subport, pos))
> +                     return 0;
> +             break;
> +     }
> 

Using a switch statement for a binary condition instead of if-else does not 
make sense to me. I know you mention you saw better performance with the 
switch, but I am pretty sure it is not the switch providing the performance 
increase. You are using if-else for testing the new subport->tc_ov_enabled 
throughout the code (an example is just below in your patch), so I suggest you 
do the same here:

if (subport->tc_ov_enabled) {
        if (!grinder_credits_check_with_tc_ov(port, subport, pos))
                return 0;
} else {
        if (!grinder_credits_check(port, subport, pos))
                return 0;
}

>       /* Advance port time */
>       port->time += pkt_len;
> @@ -2770,7 +2856,11 @@ grinder_handle(struct rte_sched_port *port,
>                                               subport->profile;
> 
>               grinder_prefetch_tc_queue_arrays(subport, pos);
> -             grinder_credits_update(port, subport, pos);
> +
> +             if (subport->tc_ov_enabled)
> +                     grinder_credits_update_with_tc_ov(port, subport,
> pos);
> +             else
> +                     grinder_credits_update(port, subport, pos);
> 
>               grinder->state = e_GRINDER_PREFETCH_MBUF;
>               return 0;
> --
> 2.25.1

Regards,
Cristian