Reconf tests: Fix async measurements 09/22209/1
authorVratko Polak <vrpolak@cisco.com>
Mon, 23 Sep 2019 13:00:00 +0000 (15:00 +0200)
committerVratko Polak <vrpolak@cisco.com>
Tue, 24 Sep 2019 08:24:38 +0000 (10:24 +0200)
TRex does not zero the server counters.
It copies the values to use as reference,
and subtracts them when asked for results.

But the reference is stored in the client (not the server).
And CSIT uses different scripts to start and stop async traffic,
which means different clients.

This patch introduces a workaround.
Async start will return xstats objects to use as reference,
and async stop will use the objects to compute the correct results.
The xstats objects are stored in TrafficGenerator instance.
Sync measurement does not export the counters, to shorten logs.

Other improvements:
+ Make stop_traffic_on_tg return measurement results directly.
+ Rename --async to --async_start as "async" is reserved in Python 3.7
+ Minor pylint, docstring and typo fixes.

Change-Id: I5fc56a0763afb7d62cfa7c0651f96b6867de3e15
Signed-off-by: Vratko Polak <vrpolak@cisco.com>
resources/libraries/python/TrafficGenerator.py
resources/libraries/robot/performance/performance_utils.robot
resources/tools/trex/trex_stateless_profile.py
resources/tools/trex/trex_stateless_stop.py

index 57ff224..fafb2b7 100644 (file)
@@ -106,6 +106,7 @@ class TGDropRateSearchImpl(DropRateSearch):
             logger.trace("comparing: {los} < {acc} {typ}".format(
                 los=loss, acc=loss_acceptance, typ=loss_acceptance_type))
             return float(loss) <= float(loss_acceptance)
             logger.trace("comparing: {los} < {acc} {typ}".format(
                 los=loss, acc=loss_acceptance, typ=loss_acceptance_type))
             return float(loss) <= float(loss_acceptance)
+        return False
 
     def get_latency(self):
         """Returns min/avg/max latency.
 
     def get_latency(self):
         """Returns min/avg/max latency.
@@ -147,6 +148,8 @@ class TrafficGenerator(AbstractMeasurer):
         self.frame_size = None
         self.traffic_profile = None
         self.warmup_time = None
         self.frame_size = None
         self.traffic_profile = None
         self.warmup_time = None
+        # TODO: Rename "xstats" to something opaque, so TRex is not privileged?
+        self._xstats = (None, None)
 
     @property
     def node(self):
 
     @property
     def node(self):
@@ -402,7 +405,7 @@ class TrafficGenerator(AbstractMeasurer):
                 raise RuntimeError('pkill t-rex failed')
 
     def _parse_traffic_results(self, stdout):
                 raise RuntimeError('pkill t-rex failed')
 
     def _parse_traffic_results(self, stdout):
-        """Parse stdout of scripts into fieds of self.
+        """Parse stdout of scripts into fields of self.
 
         Block of code to reuse, by sync start, or stop after async.
         TODO: Is the output TG subtype dependent?
 
         Block of code to reuse, by sync start, or stop after async.
         TODO: Is the output TG subtype dependent?
@@ -424,20 +427,26 @@ class TrafficGenerator(AbstractMeasurer):
     def trex_stl_stop_remote_exec(self, node):
         """Execute script on remote node over ssh to stop running traffic.
 
     def trex_stl_stop_remote_exec(self, node):
         """Execute script on remote node over ssh to stop running traffic.
 
-        Internal state is updated with results.
+        Internal state is updated with measurement results.
 
         :param node: TRex generator node.
         :type node: dict
 
         :param node: TRex generator node.
         :type node: dict
-        :returns: Nothing
         :raises RuntimeError: If stop traffic script fails.
         """
         # No need to check subtype, we know it is TREX.
         ssh = SSH()
         ssh.connect(node)
 
         :raises RuntimeError: If stop traffic script fails.
         """
         # No need to check subtype, we know it is TREX.
         ssh = SSH()
         ssh.connect(node)
 
+        x_args = ""
+        for index, value in enumerate(self._xstats):
+            if value is not None:
+                # Nested quoting is fun.
+                value = value.replace("'", "\"")
+                x_args += " --xstat{i}='\"'\"'{v}'\"'\"'".format(
+                    i=index, v=value)
         (ret, stdout, _) = ssh.exec_command(
         (ret, stdout, _) = ssh.exec_command(
-            "sh -c '{}/resources/tools/trex/"
-            "trex_stateless_stop.py'".format(Constants.REMOTE_FW_DIR))
+            "sh -c '{d}/resources/tools/trex/trex_stateless_stop.py{a}'"\
+            .format(d=Constants.REMOTE_FW_DIR, a=x_args))
 
         if int(ret) != 0:
             raise RuntimeError('TRex stateless runtime error')
 
         if int(ret) != 0:
             raise RuntimeError('TRex stateless runtime error')
@@ -450,6 +459,9 @@ class TrafficGenerator(AbstractMeasurer):
             rx_port=1):
         """Execute script on remote node over ssh to start traffic.
 
             rx_port=1):
         """Execute script on remote node over ssh to start traffic.
 
+        In sync mode, measurement results are stored internally.
+        In async mode, initial data including xstats are stored internally.
+
         :param duration: Time expresed in seconds for how long to send traffic.
         :param rate: Traffic rate expressed with units (pps, %)
         :param frame_size: L2 frame size to send (without padding and IPG).
         :param duration: Time expresed in seconds for how long to send traffic.
         :param rate: Traffic rate expressed with units (pps, %)
         :param frame_size: L2 frame size to send (without padding and IPG).
@@ -498,7 +510,7 @@ class TrafficGenerator(AbstractMeasurer):
                 frame_size=frame_size, rate=rate, warmup=warmup_time, p_0=p_0,
                 p_1=p_1)
         if async_call:
                 frame_size=frame_size, rate=rate, warmup=warmup_time, p_0=p_0,
                 p_1=p_1)
         if async_call:
-            command += " --async"
+            command += " --async_start"
         if latency:
             command += " --latency"
         if unidirection:
         if latency:
             command += " --latency"
         if unidirection:
@@ -513,11 +525,20 @@ class TrafficGenerator(AbstractMeasurer):
         elif async_call:
             #no result
             self._start_time = time.time()
         elif async_call:
             #no result
             self._start_time = time.time()
-            self._rate = float(rate[:-3]) if "pps" in rate else rate
+            self._rate = float(rate[:-3]) if "pps" in rate else float(rate)
             self._received = None
             self._sent = None
             self._loss = None
             self._latency = None
             self._received = None
             self._sent = None
             self._loss = None
             self._latency = None
+            xstats = [None, None]
+            index = 0
+            for line in stdout.splitlines():
+                if "Xstats snapshot {i}: ".format(i=index) in line:
+                    xstats[index] = line[19:]
+                    index += 1
+                if index == 2:
+                    break
+            self._xstats = tuple(xstats)
         else:
             self._parse_traffic_results(stdout)
             self._start_time = None
         else:
             self._parse_traffic_results(stdout)
             self._start_time = None
@@ -526,12 +547,14 @@ class TrafficGenerator(AbstractMeasurer):
     def stop_traffic_on_tg(self):
         """Stop all traffic on TG.
 
     def stop_traffic_on_tg(self):
         """Stop all traffic on TG.
 
-        :returns: Nothing
+        :returns: Structure containing the result of the measurement.
+        :rtype: ReceiveRateMeasurement
         :raises RuntimeError: If TG is not set.
         """
         subtype = check_subtype(self._node)
         if subtype == NodeSubTypeTG.TREX:
             self.trex_stl_stop_remote_exec(self._node)
         :raises RuntimeError: If TG is not set.
         """
         subtype = check_subtype(self._node)
         if subtype == NodeSubTypeTG.TREX:
             self.trex_stl_stop_remote_exec(self._node)
+        return self.get_measurement_result()
 
     def send_traffic_on_tg(
             self, duration, rate, frame_size, traffic_profile, warmup_time=5,
 
     def send_traffic_on_tg(
             self, duration, rate, frame_size, traffic_profile, warmup_time=5,
@@ -539,6 +562,11 @@ class TrafficGenerator(AbstractMeasurer):
             rx_port=1):
         """Send traffic from all configured interfaces on TG.
 
             rx_port=1):
         """Send traffic from all configured interfaces on TG.
 
+        In async mode, xstats is stored internally,
+        to enable getting correct result when stopping the traffic.
+        In both modes, stdout is returned,
+        but _parse_traffic_results only works in sync output.
+
         Note that bidirectional traffic also contains flows
         transmitted from rx_port and received in tx_port.
         But some tests use asymmetric traffic, so those arguments are relevant.
         Note that bidirectional traffic also contains flows
         transmitted from rx_port and received in tx_port.
         But some tests use asymmetric traffic, so those arguments are relevant.
index d809376..05059b7 100644 (file)
 | Start Traffic on Background
 | | [Documentation]
 | | ... | Start traffic at specified rate then return control to Robot.
 | Start Traffic on Background
 | | [Documentation]
 | | ... | Start traffic at specified rate then return control to Robot.
-| | ... | Useful if the test needs to do something while traffic is running.
+| | ...
+| | ... | This keyword is useful if the test needs to do something
+| | ... | while traffic is running.
 | | ... | Just a wrapper around L1 keyword.
 | | ... |
 | | ... | TODO: How to make sure the traffic is stopped on any failure?
 | | ... | Just a wrapper around L1 keyword.
 | | ... |
 | | ... | TODO: How to make sure the traffic is stopped on any failure?
 | | [Documentation]
 | | ... | Stop the running traffic, return measurement result.
 | | ... | For bidirectional traffic, the reported values are bi-directional.
 | | [Documentation]
 | | ... | Stop the running traffic, return measurement result.
 | | ... | For bidirectional traffic, the reported values are bi-directional.
+| | ...
 | | ... | Just a wrapper around L1 keyword.
 | | ... |
 | | ... | TODO: Tolerate if traffic was not started.
 | | ...
 | | ... | Just a wrapper around L1 keyword.
 | | ... |
 | | ... | TODO: Tolerate if traffic was not started.
 | | ...
+| | ... | *Returns:*
+| | ... | - Measurement result. Type: ReceiveRateMeasurement
+| | ...
 | | ... | *Example:*
 | | ...
 | | ... | \${result}= \| Stop Running Traffic \|
 | | ...
 | | ... | *Example:*
 | | ...
 | | ... | \${result}= \| Stop Running Traffic \|
 | | ...
-| | Stop traffic on tg
-| | ${result}= | Get Measurement Result
+| | ${result}= | Stop traffic on tg
 | | Return From Keyword | ${result}
 | | Return From Keyword | ${result}
index 15e2157..525ebe6 100755 (executable)
@@ -18,9 +18,9 @@ the profile and sends the traffic. At the end, it measures the packet loss and
 latency.
 """
 
 latency.
 """
 
-import sys
 import argparse
 import json
 import argparse
 import json
+import sys
 
 sys.path.insert(0, "/opt/trex-core-2.54/scripts/automation/"
                    "trex_control_plane/interactive/")
 
 sys.path.insert(0, "/opt/trex-core-2.54/scripts/automation/"
                    "trex_control_plane/interactive/")
@@ -186,7 +186,14 @@ def simple_burst(profile_file, duration, framesize, rate, warmup_time, port_0,
         # Choose rate and start traffic:
         client.start(ports=ports, mult=rate, duration=duration)
 
         # Choose rate and start traffic:
         client.start(ports=ports, mult=rate, duration=duration)
 
-        if not async_start:
+        if async_start:
+            # For async stop, we need to export the current snapshot.
+            xsnap0 = client.ports[0].get_xstats().reference_stats
+            print("Xstats snapshot 0: {s!r}".format(s=xsnap0))
+            if not unidirection:
+                xsnap1 = client.ports[1].get_xstats().reference_stats
+                print("Xstats snapshot 1: {s!r}".format(s=xsnap1))
+        else:
             # Block until done:
             client.wait_on_traffic(ports=ports, timeout=duration+30)
 
             # Block until done:
             client.wait_on_traffic(ports=ports, timeout=duration+30)
 
@@ -226,7 +233,7 @@ def simple_burst(profile_file, duration, framesize, rate, warmup_time, port_0,
                 p_0=port_0, p_1=port_1, v=lost_a))
             if not unidirection:
                 print("packets lost from {p_1} --> {p_0}:   {v} pkts".format(
                 p_0=port_0, p_1=port_1, v=lost_a))
             if not unidirection:
                 print("packets lost from {p_1} --> {p_0}:   {v} pkts".format(
-                p_0=port_0, p_1=port_1, v=lost_b))
+                    p_0=port_0, p_1=port_1, v=lost_b))
 
     except STLError as ex_error:
         print(ex_error, file=sys.stderr)
 
     except STLError as ex_error:
         print(ex_error, file=sys.stderr)
@@ -279,7 +286,7 @@ def main():
                         required=True,
                         type=int,
                         help="Port 1 on the traffic generator.")
                         required=True,
                         type=int,
                         help="Port 1 on the traffic generator.")
-    parser.add_argument("--async",
+    parser.add_argument("--async_start",
                         action="store_true",
                         default=False,
                         help="Non-blocking call of the script.")
                         action="store_true",
                         default=False,
                         help="Non-blocking call of the script.")
@@ -307,7 +314,7 @@ def main():
                  port_0=args.port_0,
                  port_1=args.port_1,
                  latency=args.latency,
                  port_0=args.port_0,
                  port_1=args.port_1,
                  latency=args.latency,
-                 async_start=args.async,
+                 async_start=args.async_start,
                  unidirection=args.unidirection)
 
 
                  unidirection=args.unidirection)
 
 
index 9421356..b8b0212 100755 (executable)
@@ -23,8 +23,12 @@ Requirements:
 
 Functionality:
 1. Stop any running traffic
 
 Functionality:
 1. Stop any running traffic
+2. Optionally restore reference counter values.
+3. Return conter differences.
 """
 
 """
 
+import argparse
+from collections import OrderedDict  # Needed to parse xstats representation.
 import sys
 import json
 
 import sys
 import json
 
@@ -35,6 +39,13 @@ from trex.stl.api import *
 
 def main():
     """Stop traffic if any is running. Report xstats."""
 
 def main():
     """Stop traffic if any is running. Report xstats."""
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
+        "--xstat0", type=str, default="", help="Reference xstat object if any.")
+    parser.add_argument(
+        "--xstat1", type=str, default="", help="Reference xstat object if any.")
+    args = parser.parse_args()
+
     client = STLClient()
     try:
         # connect to server
     client = STLClient()
     try:
         # connect to server
@@ -44,7 +55,15 @@ def main():
         # TODO: Support unidirection.
         client.stop(ports=[0, 1])
 
         # TODO: Support unidirection.
         client.stop(ports=[0, 1])
 
-        # read the stats after the test
+        # Read the stats after the test,
+        # we need to update values before the last trial started.
+        if args.xstat0:
+            snapshot = eval(args.xstat0)
+            client.ports[0].get_xstats().reference_stats = snapshot
+        if args.xstat1:
+            snapshot = eval(args.xstat1)
+            client.ports[1].get_xstats().reference_stats = snapshot
+        # Now we can call the official method to get differences.
         xstats0 = client.get_xstats(0)
         xstats1 = client.get_xstats(1)
 
         xstats0 = client.get_xstats(0)
         xstats1 = client.get_xstats(1)