[dpdk-dev] [PATCH v6] sched: make RED scaling configurable

Alan Dewar alangordondewar at gmail.com
Tue Jan 16 16:50:27 CET 2018


On Mon, Jan 15, 2018 at 4:52 PM, Stephen Hemminger
<stephen at networkplumber.org> wrote:
>
> On Mon, 15 Jan 2018 16:16:09 +0000
> alangordondewar at gmail.com wrote:
>
> Looks like a good idea, minor editing feedback.
>
>
> > -     red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 + RTE_RED_SCALING);
> > -     red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 + RTE_RED_SCALING);
> > -     red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) << RTE_RED_SCALING;
> > +     red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 + rte_red_scaling);
> > +     red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 + rte_red_scaling);
>
> While you are at it remove unnecessary parenthesis here.
>

Okay will do.

> > +     red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) <<
> > +             rte_red_scaling;
>
> It reads easier if the the shift operator on the next line
>
>         red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv)
>                  << rte_red_scaling;
>

Okay will do.

> Why do functional tests have to be in same file and clutter the code?

Do you mean the same patch file or the same unit-test file?

I received feedback previously (might not have been this patch) where
I had split the functional changes and the new unit-tests into
separate patches and was asked to combine them into a single patch.  I
like the idea of if you have the fix you also have the new unit-tests.

As for having the new tests in the same file as existing tests, the
red tests are table driven so most of the changes to the unit-test
code are just adding new table entries to exercise the RED code with a
different scaling factor.


More information about the dev mailing list