From: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
To: "Dewar, Alan" <ad759e@intl.att.com>,
"'alangordondewar@gmail.com'" <alangordondewar@gmail.com>
Cc: "'dev@dpdk.org'" <dev@dpdk.org>, 'Alan Dewar' <alan.dewar@att.com>
Subject: Re: [dpdk-dev] [PATCH v4] sched: make RED scaling configurable
Date: Thu, 4 Jan 2018 18:25:44 +0000 [thread overview]
Message-ID: <3EB4FA525960D640B5BDFFD6A3D891267BAF8B99@IRSMSX108.ger.corp.intel.com> (raw)
In-Reply-To: <3F9268EEC0E43747A5FFFC6B48EF0321FBE783@gbcdcmbx03.intl.att.com>
> -----Original Message-----
> From: Dewar, Alan [mailto:ad759e@intl.att.com]
> Sent: Thursday, January 4, 2018 1:35 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> 'alangordondewar@gmail.com' <alangordondewar@gmail.com>
> Cc: 'dev@dpdk.org' <dev@dpdk.org>; 'Alan Dewar' <alan.dewar@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
prev parent reply other threads:[~2018-01-04 18:25 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-01 13:41 [dpdk-dev] [PATCH] " alangordondewar
2017-09-11 15:51 ` Kantecki, Tomasz
2017-09-13 10:15 ` [dpdk-dev] [PATCH v2] " alangordondewar
2017-09-18 22:03 ` Kantecki, Tomasz
2017-09-20 13:12 ` [dpdk-dev] [PATCH v3] " alangordondewar
2017-09-25 10:36 ` Dumitrescu, Cristian
2017-09-26 8:02 ` Dewar, Alan
2017-10-03 9:21 ` [dpdk-dev] [PATCH v4] " alangordondewar
2017-10-03 17:15 ` Dumitrescu, Cristian
2018-01-02 16:21 ` Dumitrescu, Cristian
2018-01-02 16:43 ` Dumitrescu, Cristian
2018-01-03 14:29 ` Dewar, Alan
2018-01-03 16:20 ` Dumitrescu, Cristian
2018-01-04 13:34 ` Dewar, Alan
2018-01-04 18:25 ` Dumitrescu, Cristian [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3EB4FA525960D640B5BDFFD6A3D891267BAF8B99@IRSMSX108.ger.corp.intel.com \
--to=cristian.dumitrescu@intel.com \
--cc=ad759e@intl.att.com \
--cc=alan.dewar@att.com \
--cc=alangordondewar@gmail.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).