[ovs-dev] [PATCH] utilities: Add new pipeline generator script.

[Daniele Di Proietto]

I've been using the script successfully for the past few days, so

Acked-by: Daniele Di Proietto <diproiettod at vmware.com>

I have a few comments (I feel ridiculous being so picky about a python
script, so feel free to apply/ignore them).

Would we care about making it run also with python3? Only 3 changes
are required (see inline).

I think it would be useful to add an option to initialize the python
RNG to make the process repeatable (at least on the same host).
This would be useful to gain more confidence when testing performance
related changes.  Something like:

    parser.add_argument("--seed", dest="seed", type=int,
                        help="Seed to initialize the random generator")

and then, before calling pipeline()

    if args.seed is not None:
        random.seed(args.seed)

A couple of more comments inline

Thanks!

Daniele

> On 25 Apr 2015, at 01:29, Ethan Jackson <ethan at nicira.com> wrote:
> 
> When doing OVS performance testing, it's important to have both
> realistic traffic traces and OpenFlow pipelines on which to evaluate
> prospective changes.  As a first step in this direction, this patch
> adds a python script which generates an OpenFlow pipeline intended to
> simulate typical network virtualization workloads.
> 
> Signed-off-by: Ethan Jackson <ethan at nicira.com>
> ---
> utilities/automake.mk    |   5 +-
> utilities/ovs-pipegen.py | 122 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 125 insertions(+), 2 deletions(-)
> create mode 100755 utilities/ovs-pipegen.py
> 
[...]
> 
> +    flows = []
> +    for stage in xrange(len(pipeline)):

python3: how about using range() instead of range()?

> +        action = resubmit(stage + 1)
> +        flows += [pipeline[stage](stage, action) for _ in xrange(size)]

python3: same here

> +        flows.append(flow_str(stage, "", action, priority=1))
> +
> +    flows.append(flow_str(len(pipeline), "", "in_port"))
> +
> +    for f in flows:
> +        print f

python3: print(f) instead of 'print(f)'

> +
> +
> +def main():
> +    description = textwrap.dedent(
> +        """
> +        Generate a test OpenFlow pipeline.
> +
> +        Open vSwitch relies heavily on flow caching to get good performance for
> +        packet processing.  While on average, this produces good results,
> +        performance is heavily depedent on the slow path OpenFlow tables, and
> +        how they're translated into datapath megaflows.  For this reason, when
> +        doing performance testing it's important to run with "realistic"
> +        OpenFlow tables to ensure results will stand up in the real world.
> +
> +        This script generates a simple OpenFlow pipeline intended to simulate
> +        realistic network virtualization workloads.  All traffic received is
> +        run through a series of OpenFlow tables designed to simulate a logical
> +        switch, router, and firewall, before forwarded back on it's in_port.

typo?

> +        """)
> +
> +    epilog = textwrap.dedent(
> +        """
> +        typical usage:
> +          ovs-ofctl del-flows bridge \\
> +          && %s | ovs-ofctl add-flows bridge - \\
> +          && ovs-ofctl dump-flows bridge
> +        """ % sys.argv[0])
> +
> +    parser = argparse.ArgumentParser(description=description, epilog=epilog,
> +                                     formatter_class=\
> +                                     argparse.RawDescriptionHelpFormatter)
> +    parser.add_argument("--size", dest="size", default=1000,
> +                        help="Size (rules) of each OpenFlow table.")
> +    args=parser.parse_args()

I would put whitespaces around "="