RE: [PATCH v8] sched: enable CMAN at runtime

["Danilewicz, MarcinX" <marcinx.danilewicz@intel.com>]

<snip>
> > Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@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