Thanks for your feedback Ondrej.
Comments below.
On 6/18/20 12:21 PM, Ondrej Lichtner wrote:
On Mon, Jun 15, 2020 at 06:22:53PM +0200, Adrian Moreno wrote:
> Currently the TRex Test has a hardcoded set of streams.
>
> In order to make it more generic and extendable, this RFC first refactors
> the client and server code into a library that does not depend on other
> LNST code.
I'm OK with refactoring the LNST TRex test module code into a more
generic wrapper layer used by both the test module as well as the tperf
tool.
I think we need to be a little careful about how to approach that so
that this doesn't become "too big" at that point it probably makes more
sense to completely split that into a separate project that wraps TRex
(because TRex is a pain) and also provides a tperf cli for ease of use
and then implementing an LNST Test module that further builds on top of
this wrapper library. I think this has a lot of issues - such as
supporting another project, or the fact that this really just means that
TRex itself should be made better.
For now, I would not like to pay the maintenance price of another project to be
honest :)
Also, at this point the lnst.Tests.TRex module doesn't have a
very big
dependency on LNST - it uses a couple of parameters for type checking
and defines a module interface API, which is just that there's a "run"
method, but you still kept that in the standalone library. I guess this
is your main point for possible future extensions?
Right, the more that can be added to the standalone library, the better because
the development is much easier. However, I must admit it's my lack of expertise
in LNST talking here.
Could we implement tperf using the existing "Tests/Trex.py" without having to
run a full blown lnst-slave? If it's possible without doing too much
LNST-specific things, that would save us the refactoring, the weird "placement"
and reduce the temptations of creating another project. What do you think?
> That way, a small cli appliation can be implemented to inject
> traffic using the same code as LNST would. This is useful for early prototyping.
>
> An example of such as tool is introduced: test_tools/tperf. It basically runs the
> client and server as LNST would and report the aggregated throughput.
I also like the idea of this small "tperf" tool to help with prototyping,
that definitelly sounds interesting since TRex can be a lot of pain...
probably not worth investigating a generic solution at this point as
everything else is much simpler IMO.
>
> Finally, the stream can be modularized into TRex compatible modules so that:
> - New stream generators can be easily implemented
> - TRex tools (e.g: stl-sym) can be used for stream generator development
This took me a bit to understand... I was confused by the module
implementation with a "register" function... I now see that it's another
one of TRex weirdnes, I would personally prefer a class based approach
but I guess this is fine since it follows some "standard"
Right. It's a trex thing. We can have another class-based API for LNST but
keep
the trex stl one for prototyping. But we might break the latter without knowing.
>
> The current stream generation is modularized as an example: udp_simple
So does this patchset require any other changes to the recipes that
currently use TRex? or is it drop in? Haven't tested this since I don't
have a pre-setup dpdk environment...
Right now, no change is needed. The default module implements the current
functionality so we're good.
When we add more modules, we can have an optional parameter that can be set by
new tests pointing to the new modules.
>
> I'm only a sporadic contributor to LNST so sending this RFC to get some
feedback.
I think the only thing I would complain about is the placement of the
"lnst.TRex" module... I'm not sure I want it at this top level, I think
it also contributed to my slower understanding of the RFC. Not exactly
sure where to put it though. I'll keep thinking about this
I agree. I dislike the placement as well.
If you agree with the approach described above (tperf depend on Tests/Trex.py
directly) we could have:
lnst/Tests/Trex.py
lnst/Tests/TRexModules/udp_simple.py
lnst/Tests/TRexModules/...(otherModules).py
test-tools/tperf/tperf
What do you think?
-Ondrej
>
> Adrian Moreno (3):
> lnst.Tests.TRex Refactor TRex client and server
> test_tools: Add tperf
> lnst.TRex Use stl compatible modules to generate streams
>
> lnst/TRex/TRex.py | 205 ++++++++++++++++++++++++++++++++++++++++
> lnst/TRex/__init__.py | 0
> lnst/TRex/udp_simple.py | 38 ++++++++
> lnst/Tests/TRex.py | 137 ++++-----------------------
> test_tools/tperf/tperf | 192 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 452 insertions(+), 120 deletions(-)
> create mode 100644 lnst/TRex/TRex.py
> create mode 100644 lnst/TRex/__init__.py
> create mode 100644 lnst/TRex/udp_simple.py
> create mode 100755 test_tools/tperf/tperf
>
> -- ,
> 2.26.2
>
> _______________________________________________
> LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
> To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
> Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedora...
--
Adrián Moreno