Hi Marcin,

<snip>
 
> >
> > Still only changes in rte_sched.c and no change in rte_sched.h for the API
> to
> > configure this feature?
> 
> Yes, because you said to remove whole
> rte_sched_subport_tc_ov_config(struct rte_sched_port *port,
>       uint32_t subport_id,
>       bool tc_ov_enable)
> here as comment to v4:
> > >
> > > This function should not exist, please remove it and keep the initial code
> that
> > > computes the tc_ov related variable regardless of whether tc_ov is
> enabled
> > or
> > > not.
> 
> And by the latest other changes the TC OV is enabled by default. All other 
> init
> for this feature is done with sched init as per yours other explanations. In
> turn any can change this new flag, but apparently in code without proper API
> for that?
> 
> Isnt that what you wanted?
> 

Nope, it looks like we have a misunderstanding here. Looking back at my 
comments from V3: What I meant is that the configuration values related to this 
feature (all the tc_ov configuration values) should be computed at 
initialization regardless of whether this feature is enabled or not in order to 
minimize code changes and the size of the patch. In V3, you moved a lot of the 
init code into a different function, but it was my mistake not to realize this 
was the API function you introduced, sorry about the misunderstanding.

I think we definitely need an API function to simply set the internal subport 
tc_ov_enabled flag (while also doing the proper argument checks that any well 
behaved API function must do), but we should not move here the init code that 
does not really need to be here. Makes sense?

Regards,
Cristian

Reply via email to