<snip>
> > Signed-off-by: Marcin Danilewicz <marcinx.danilew...@intel.com>
> > ---
> > Log: v2 change in rte_sched.h to avoid ABI breakage.
> > v3 changes from comments
> > v4 rebase to 22.07-rc1
> > v5 rebase to main latest
> > v6 commit message fixed
> > v7 changes from comments
> > v8 with changes from comments
>
>
> You need to explicitly mention the changes done in each version to help
> reviewer easily locate the changes.
Yes, yes. I am going to provide more detailed changes and explaining history on
what changes was made. This split development over time is the best example to
show that need.
<snip>
>>@@ -96,9 +94,7 @@ static const struct rte_sched_subport_params
> > subport_params_default = {
> > .pipe_profiles = pipe_profile,
> > .n_pipe_profiles = 0, /* filled at run time */
> > .n_max_pipe_profiles = RTE_DIM(pipe_profile), -#ifdef
> RTE_SCHED_CMAN
> > .cman_params = &cman_params,
> > -#endif /* RTE_SCHED_CMAN */
> > };
>
>
> Similar to what is discussed for qos_sched sample app, set cman_params to
> NULL and remove default parameters for "static struct
> rte_sched_cman_params cman_params" above.
Sure, that will be done.
> > -#ifdef RTE_SCHED_CMAN
> > - struct rte_sched_cman_params cman_params = {
> > - .cman_mode = RTE_SCHED_CMAN_RED,
> > - .red_params = { },
> > - };
> > + subport_params->cman_params = NULL;
>
> No need to set subport_params->cman_params again to null as it is already
> set to NULL in init.c.
Sure, app_load_cfg_profile -> cfg_load_subport is also not called several
times, just once
<snip>
> > }
> > -#ifdef RTE_SCHED_CMAN
> > - set_subport_cman_params(subport_params+i,
> > cman_params);
> > -#endif
> > + subport_params[i].cman_params = &cman_params;
>
> Since cman_params_is global variable, memory is allocated regardless of
> whether cman mechanism is enabled or disabled. So subport_params-
> >cman_params will never point to NULL even when red/pie is disabled.
> Define local flag "cman_enabled" and set his flag if red or pie is enabled and
> check this flag to set subport_params[i].cman_params to cman_params.
Ah .. nice spot. But, cant we do that with additional check, using what we have?
For an instance:
If (cman_params.cman_mode == RTE_SCHED_CMAN_PIE || cman_params.cman_mode ==
RTE_SCHED_CMAN_RED) {
Then set subport_params[i].cman_params to cman_params.
}
BR,
/Marcin
