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

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Dewar, Alan [mailto:ad759e at intl.att.com]
> Sent: Thursday, January 4, 2018 1:35 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu at intel.com>;
> 'alangordondewar at gmail.com' <alangordondewar at gmail.com>
> Cc: 'dev at dpdk.org' <dev at dpdk.org>; 'Alan Dewar' <alan.dewar at att.com>
> Subject: RE: [PATCH v4] sched: make RED scaling configurable
> 
> > > > > +int
> > > > > +rte_red_set_scaling(uint16_t max_red_queue_length);
> > > > > +
> > > > > +/**
> > > > > + * @brief Reset the RED scaling factor - only for use by RED
> > > > > +unit-tests
> > > > > + *
> > > > > + * @return Operation status
> > > > > + */
> > > > > +void
> > > > > +rte_red_reset_scaling(void);
> > > >
> > > > As stated above, this function is probably not useful and my vote is
> > > > to
> > > remove it.
> > > >
> > >
> > > It is needed by the revised unit-test program.  This function can't be
> > > moved into the unit-test program because it needs to reset variables
> > > that are statically declared within rte_red.c
> > >
> > >
> >
> > Hi Alan,
> >
> > We only put API that makes sense for a real app, not for unit test.
> 
> I didn't add rte_red_reset_scaling function to
> lib/librte_sched/rte_sched_version.map, so doesn't that mean that this
> function wouldn't be part of the rte_sched library's external API?

Nope, it does not work this way.

> 
> > About unit tests: Each test in the unit test suite should start from scratch,
> i.e. create a RED object from scratch, configure it, use it, free it rather than
> use RED objects created by previous tests. We need to avoid the latter
> approach, as it is creating fake dependencies between tests that alter the
> overall test results. Each test should be independent, and not rely on
> previous tests. Makes sense?
> 
> I can appreciate what you are suggesting, but rte_red.c does some one-time
> initialization of a module static array called rte_red_log2_1_minus_Wq.  The
> values assigned to elements in this array depend upon what the RED scaling
> factor is set to.   This means that the first RED sub-test that calls
> rte_red_config_init imposes its scaling factor for all subsequent RED sub-
> tests.    The new RED unit-tests need to use different scaling factors.
> 
> The only way to make these RED sub-tests completely independent would be
> to create and execute separate test images for each sub-test, rather than
> building all the sub-tests into a single image and using the test-harness's
> command-line to select which sub-test to execute.
> 
> As an alternative I'm guessing that we could change how rte_red.c does its
> initialization.
> 

OK, makes sense to me. Let's create an API function with the updated name of __rte_red_reset() which resets all globals and forces retrain of the module. It should be part of the API map.

> Regards
> Alan
> 
> 
> >
> > Regards,
> > Cristian