NFVBENCH-192: Complete/fix hdrh related processings to consider all cases

(multiple service chains, distribution n/a with intel VFs)

Signed-off-by: Pierrick Louin <pierrick.louin@orange.com>
Change-Id: I80e38601292a7777d37ed05959c8ef205505c2ac

8 files changed, 110 insertions, 76 deletions

diff --git a/nfvbench/credentials.py b/nfvbench/credentials.py
index d9a67e6..4e4985f 100644
--- a/nfvbench/credentials.py
+++ b/nfvbench/credentials.py
@@ -176,7 +176,7 @@ class Credentials(object):
             # Return HTTP 200 if user is admin
             self.get_session().get('/users', endpoint_filter=filter)
             self.is_admin = True
-        except Exception as e:
+        except Exception:
             try:
                 # vX/users URL returns exception (HTTP 403) if user is not admin.
                 self.get_session().get('/v' + str(self.rc_identity_api_version) + '/users',
diff --git a/nfvbench/nfvbench.py b/nfvbench/nfvbench.py
index 19c402f..a178d24 100644
--- a/nfvbench/nfvbench.py
+++ b/nfvbench/nfvbench.py
@@ -504,10 +504,12 @@ def _parse_opts_from_cli():
     parser.add_argument('--user-info', dest='user_info',
                         action='store',
                         metavar='<data>',
-                        help='Custom data to be included as is in the json report config branch - '
-                               + ' example, pay attention! no space: '
-                               + '--user-info=\'{"status":"explore","description":'
-                               + '{"target":"lab","ok":true,"version":2020}}\'')
+                        help='Custom data to be included as is '
+                             'in the json report config branch - '
+                             ' example, pay attention! no space: '
+                             '--user-info=\'{"status":"explore","description":'
+                             '{"target":"lab","ok":true,"version":2020}}\' - '
+                             'this option may be repeated; given data will be merged.')
 
     parser.add_argument('--vlan-tagging', dest='vlan_tagging',
                         type=bool_arg,
@@ -521,7 +523,7 @@ def _parse_opts_from_cli():
                         action='store',
                         default=None,
                         help='Override the NFVbench \'intf_speed\' '
-                                + 'parameter (e.g. 10Gbps, auto, 16.72Gbps)')
+                             'parameter (e.g. 10Gbps, auto, 16.72Gbps)')
 
     parser.add_argument('--cores', dest='cores',
                         type=int_arg,
@@ -580,16 +582,16 @@ def _parse_opts_from_cli():
                         default=None,
                         action='store_true',
                         help='Show the current TRex local server log file contents'
-                               + ' => diagnostic/help in case of configuration problems')
+                             ' => diagnostic/help in case of configuration problems')
 
     parser.add_argument('--debug-mask', dest='debug_mask',
                         type=int_arg,
                         metavar='<mask>',
                         action='store',
-                        default='0x00000000',
+                        default=None,
                         help='General purpose register (debugging flags), '
-                                + 'the hexadecimal notation (0x...) is accepted.'
-                                + 'Designed for development needs.')
+                             'the hexadecimal notation (0x...) is accepted.'
+                             'Designed for development needs (default: 0).')
 
     opts, unknown_opts = parser.parse_known_args()
     return opts, unknown_opts
diff --git a/nfvbench/packet_stats.py b/nfvbench/packet_stats.py
index d6b9a68..d3ec78a 100644
--- a/nfvbench/packet_stats.py
+++ b/nfvbench/packet_stats.py
@@ -239,18 +239,21 @@ class PacketPathStats(object):
             results = {'lat_min_usec': latency.min_usec,
                        'lat_max_usec': latency.max_usec,
                        'lat_avg_usec': latency.avg_usec}
-            if latency.hdrh:
+            if latency.hdrh_available():
                 results['hdrh'] = latency.hdrh
                 decoded_histogram = HdrHistogram.decode(latency.hdrh)
-                # override min max and avg from hdrh
-                results['lat_min_usec'] = decoded_histogram.get_min_value()
-                results['lat_max_usec'] = decoded_histogram.get_max_value()
-                results['lat_avg_usec'] = decoded_histogram.get_mean_value()
                 results['lat_percentile'] = {}
-                for percentile in self.config.lat_percentiles:
-                    results['lat_percentile'][percentile] = decoded_histogram.\
-                        get_value_at_percentile(percentile)
-
+                # override min max and avg from hdrh (only if histogram is valid)
+                if decoded_histogram.get_total_count() != 0:
+                    results['lat_min_usec'] = decoded_histogram.get_min_value()
+                    results['lat_max_usec'] = decoded_histogram.get_max_value()
+                    results['lat_avg_usec'] = decoded_histogram.get_mean_value()
+                    for percentile in self.config.lat_percentiles:
+                        results['lat_percentile'][percentile] = decoded_histogram.\
+                            get_value_at_percentile(percentile)
+                else:
+                    for percentile in self.config.lat_percentiles:
+                        results['lat_percentile'][percentile] = 'n/a'
         else:
             results = {}
         results['packets'] = counters
diff --git a/nfvbench/summarizer.py b/nfvbench/summarizer.py
index bbd5908..7e2d129 100644
--- a/nfvbench/summarizer.py
+++ b/nfvbench/summarizer.py
@@ -263,8 +263,8 @@ class NFVBenchSummarizer(Summarizer):
         # add percentiles headers if hdrh enabled
         if not self.config.disable_hdrh:
             for percentile in self.config.lat_percentiles:
-                self.ndr_pdr_header.append((str(percentile) + ' %ile lat.', Formatter.standard))
-                self.single_run_header.append((str(percentile) + ' %ile lat.', Formatter.standard))
+                self.ndr_pdr_header.append(str(percentile) + ' %ile lat.', Formatter.standard)
+                self.single_run_header.append(str(percentile) + ' %ile lat.', Formatter.standard)
         # if sender is available initialize record
         if self.sender:
             self.__record_init()
@@ -481,6 +481,7 @@ class NFVBenchSummarizer(Summarizer):
                     self.extract_hdrh_percentiles(
                         analysis['stats']['overall']['rx']['lat_percentile'], row_data)
                 summary_table.add_row(row_data)
+
                 single_run_data = {
                     'type': 'single_run',
                     'offered_tx_rate_bps': analysis['stats']['offered_tx_rate_bps'],
@@ -556,29 +557,36 @@ class NFVBenchSummarizer(Summarizer):
                    'lat_min_usec': 'Min lat.',
                    'lat_max_usec': 'Max lat.'}
         if 'lat_avg_usec' in chains['0']:
-            lat_keys = ['lat_avg_usec', 'lat_min_usec', 'lat_max_usec', 'lat_percentile']
+            lat_keys = ['lat_avg_usec', 'lat_min_usec', 'lat_max_usec']
 
             if not self.config.disable_hdrh:
+                lat_keys.append('lat_percentile')
                 for percentile in self.config.lat_percentiles:
-                    lat_map['lat_' + str(percentile) + '_percentile'] = str(
-                        percentile) + ' %ile lat.'
+                    lat_map['lat_' + str(percentile) + '_percentile'] = \
+                        str(percentile) + ' %ile lat.'
 
             for key in lat_map:
-                header.append((lat_map[key], Formatter.standard))
+                header.append(lat_map[key], Formatter.standard)
 
         table = Table(header)
         for chain in sorted(list(chains.keys()), key=str):
             row = [chain] + chains[chain]['packets']
             for lat_key in lat_keys:
-                if chains[chain].get(lat_key, None):
-                    if lat_key == 'lat_percentile':
-                        if not self.config.disable_hdrh:
-                            for percentile in chains[chain][lat_key]:
-                                row.append(Formatter.standard(chains[chain][lat_key][percentile]))
-                    else:
+
+                if lat_key != 'lat_percentile':
+                    if chains[chain].get(lat_key, None):
                         row.append(Formatter.standard(chains[chain][lat_key]))
+                    else:
+                        row.append('n/a')
                 else:
-                    row.append('--')
+                    if not self.config.disable_hdrh:
+                        if chains[chain].get(lat_key, None):
+                            for percentile in chains[chain][lat_key]:
+                                row.append(Formatter.standard(
+                                    chains[chain][lat_key][percentile]))
+                        else:
+                            for percentile in self.config.lat_percentiles:
+                                row.append('n/a')
             table.add_row(row)
         return table
 
diff --git a/nfvbench/traffic_client.py b/nfvbench/traffic_client.py
index 6972509..ae8af8d 100755
--- a/nfvbench/traffic_client.py
+++ b/nfvbench/traffic_client.py
@@ -16,6 +16,7 @@
 import socket
 import struct
 import time
+import sys
 
 from attrdict import AttrDict
 import bitmath
@@ -1112,20 +1113,25 @@ class TrafficClient(object):
             for key in ['pkt_bit_rate', 'pkt_rate']:
                 for dirc in ['tx', 'rx']:
                     retDict['overall'][dirc][key] /= 2.0
-                retDict['overall']['hdrh'] = stats.get('hdrh', None)
-                if retDict['overall']['hdrh']:
-                    decoded_histogram = HdrHistogram.decode(retDict['overall']['hdrh'])
-                    # override min max and avg from hdrh
-                    retDict['overall']['rx']['min_delay_usec'] = decoded_histogram.get_min_value()
-                    retDict['overall']['rx']['max_delay_usec'] = decoded_histogram.get_max_value()
-                    retDict['overall']['rx']['avg_delay_usec'] = decoded_histogram.get_mean_value()
-                    retDict['overall']['rx']['lat_percentile'] = {}
-                    for percentile in self.config.lat_percentiles:
-                        retDict['overall']['rx']['lat_percentile'][percentile] = \
-                            decoded_histogram.get_value_at_percentile(percentile)
         else:
             retDict['overall'] = retDict[ports[0]]
         retDict['overall']['drop_rate_percent'] = self.__get_dropped_rate(retDict['overall'])
+
+        if 'overall_hdrh' in stats:
+            retDict['overall']['hdrh'] = stats.get('overall_hdrh', None)
+            decoded_histogram = HdrHistogram.decode(retDict['overall']['hdrh'])
+            retDict['overall']['rx']['lat_percentile'] = {}
+            # override min max and avg from hdrh (only if histogram is valid)
+            if decoded_histogram.get_total_count() != 0:
+                retDict['overall']['rx']['min_delay_usec'] = decoded_histogram.get_min_value()
+                retDict['overall']['rx']['max_delay_usec'] = decoded_histogram.get_max_value()
+                retDict['overall']['rx']['avg_delay_usec'] = decoded_histogram.get_mean_value()
+                for percentile in self.config.lat_percentiles:
+                    retDict['overall']['rx']['lat_percentile'][percentile] = \
+                        decoded_histogram.get_value_at_percentile(percentile)
+            else:
+                for percentile in self.config.lat_percentiles:
+                    retDict['overall']['rx']['lat_percentile'][percentile] = 'n/a'
         return retDict
 
     def __convert_rates(self, rate):
@@ -1154,19 +1160,21 @@ class TrafficClient(object):
             }
 
             if key == 'overall':
-                stats[key]['hdrh'] = interface.get('hdrh', None)
-                if stats[key]['hdrh']:
+                if 'hdrh' in interface:
+                    stats[key]['hdrh'] = interface.get('hdrh', None)
                     decoded_histogram = HdrHistogram.decode(stats[key]['hdrh'])
-                    # override min max and avg from hdrh
-                    stats[key]['min_delay_usec'] = decoded_histogram.get_min_value()
-                    stats[key]['max_delay_usec'] = decoded_histogram.get_max_value()
-                    stats[key]['avg_delay_usec'] = decoded_histogram.get_mean_value()
                     stats[key]['lat_percentile'] = {}
-                    for percentile in self.config.lat_percentiles:
-                        stats[key]['lat_percentile'][percentile] = decoded_histogram.\
-                            get_value_at_percentile(percentile)
-
-
+                    # override min max and avg from hdrh (only if histogram is valid)
+                    if decoded_histogram.get_total_count() != 0:
+                        stats[key]['min_delay_usec'] = decoded_histogram.get_min_value()
+                        stats[key]['max_delay_usec'] = decoded_histogram.get_max_value()
+                        stats[key]['avg_delay_usec'] = decoded_histogram.get_mean_value()
+                        for percentile in self.config.lat_percentiles:
+                            stats[key]['lat_percentile'][percentile] = decoded_histogram.\
+                                get_value_at_percentile(percentile)
+                    else:
+                        for percentile in self.config.lat_percentiles:
+                            stats[key]['lat_percentile'][percentile] = 'n/a'
         return stats
 
     def __targets_found(self, rate, targets, results):
@@ -1295,6 +1303,9 @@ class TrafficClient(object):
         delta_tx = cur_tx - self.prev_tx
         delta_rx = cur_rx - self.prev_rx
         drops = delta_tx - delta_rx
+        if delta_tx == 0:
+            LOG.info("\x1b[1mConfiguration issue!\x1b[0m (no transmission)")
+            sys.exit(0)
         drop_rate_pct = 100 * (delta_tx - delta_rx)/delta_tx
         self.prev_tx = cur_tx
         self.prev_rx = cur_rx
diff --git a/nfvbench/traffic_gen/traffic_base.py b/nfvbench/traffic_gen/traffic_base.py
index df28772..30aec6e 100644
--- a/nfvbench/traffic_gen/traffic_base.py
+++ b/nfvbench/traffic_gen/traffic_base.py
@@ -15,10 +15,10 @@
 import abc
 import sys
 
-import bitmath
-
 from nfvbench.log import LOG
 from . import traffic_utils
+from hdrh.histogram import HdrHistogram
+from functools import reduce
 
 
 class Latency(object):
@@ -34,11 +34,23 @@ class Latency(object):
         self.avg_usec = 0
         self.hdrh = None
         if latency_list:
+            hdrh_list = []
             for lat in latency_list:
                 if lat.available():
                     self.min_usec = min(self.min_usec, lat.min_usec)
                     self.max_usec = max(self.max_usec, lat.max_usec)
                     self.avg_usec += lat.avg_usec
+                if lat.hdrh_available():
+                    hdrh_list.append(HdrHistogram.decode(lat.hdrh))
+
+            # aggregate histograms if any
+            if hdrh_list:
+                def add_hdrh(x, y):
+                    x.add(y)
+                    return x
+                decoded_hdrh = reduce(add_hdrh, hdrh_list)
+                self.hdrh = HdrHistogram.encode(decoded_hdrh).decode('utf-8')
+
             # round to nearest usec
             self.avg_usec = int(round(float(self.avg_usec) / len(latency_list)))
 
@@ -46,6 +58,9 @@ class Latency(object):
         """Return True if latency information is available."""
         return self.min_usec != sys.maxsize
 
+    def hdrh_available(self):
+        """Return True if latency histogram information is available."""
+        return self.hdrh is not None
 
 class TrafficGeneratorException(Exception):
     """Exception for traffic generator."""
@@ -133,15 +148,8 @@ class AbstractTrafficGenerator(object):
 
         result = {}
 
-        intf_speeds = self.get_port_speed_gbps()
-        tg_if_speed = bitmath.parse_string(str(intf_speeds[0]) + 'Gb').bits
-        intf_speed = tg_if_speed
-
-        if hasattr(self.config, 'intf_speed') and self.config.intf_speed is not None:
-            # in case of limitation due to config, TG speed is not accurate
-            # value is overridden by conf
-            if self.config.intf_speed != tg_if_speed:
-                intf_speed = bitmath.parse_string(self.config.intf_speed.replace('ps', '')).bits
+        # actual interface speed? (may be a virtual override)
+        intf_speed = self.config.intf_speed_used
 
         if hasattr(self.config, 'user_info') and self.config.user_info is not None:
             if "extra_encapsulation_bytes" in self.config.user_info:
diff --git a/nfvbench/traffic_gen/trex_gen.py b/nfvbench/traffic_gen/trex_gen.py
index 4e20f73..d5625eb 100644
--- a/nfvbench/traffic_gen/trex_gen.py
+++ b/nfvbench/traffic_gen/trex_gen.py
@@ -168,6 +168,8 @@ class TRex(AbstractTrafficGenerator):
         result["latency"] = in_stats["latency"]
 
         # Merge HDRHistogram to have an overall value for all chains and ports
+        # (provided that the histogram exists in the stats returned by T-Rex)
+        # Of course, empty histograms will produce an empty (invalid) histogram.
         try:
             hdrh_list = []
             if ifstats:
@@ -186,7 +188,7 @@ class TRex(AbstractTrafficGenerator):
                 x.add(y)
                 return x
             decoded_hdrh = reduce(add_hdrh, hdrh_list)
-            result["hdrh"] = HdrHistogram.encode(decoded_hdrh).decode('utf-8')
+            result["overall_hdrh"] = HdrHistogram.encode(decoded_hdrh).decode('utf-8')
         except KeyError:
             pass
 
@@ -589,8 +591,6 @@ class TRex(AbstractTrafficGenerator):
         """
         streams = []
         pg_id, lat_pg_id = self.get_pg_id(port, chain_id)
-        if self.config.no_flow_stats:
-            LOG.info("Traffic flow statistics are disabled.")
         if l2frame == 'IMIX':
             for ratio, l2_frame_size in zip(IMIX_RATIOS, IMIX_L2_SIZES):
                 pkt = self._create_pkt(stream_cfg, l2_frame_size)
@@ -602,12 +602,12 @@ class TRex(AbstractTrafficGenerator):
                         streams.append(STLStream(packet=pkt,
                                                  flow_stats=STLFlowStats(pg_id=pg_id,
                                                                          vxlan=True)
-                                                 if not self.config.no_flow_stats else None,
+                                                    if not self.config.no_flow_stats else None,
                                                  mode=STLTXCont(pps=ratio)))
                     else:
                         streams.append(STLStream(packet=pkt,
                                                  flow_stats=STLFlowStats(pg_id=pg_id)
-                                                 if not self.config.no_flow_stats else None,
+                                                    if not self.config.no_flow_stats else None,
                                                  mode=STLTXCont(pps=ratio)))
 
             if latency:
@@ -633,12 +633,12 @@ class TRex(AbstractTrafficGenerator):
                     streams.append(STLStream(packet=pkt,
                                              flow_stats=STLFlowStats(pg_id=pg_id,
                                                                      vxlan=True)
-                                             if not self.config.no_flow_stats else None,
+                                                if not self.config.no_flow_stats else None,
                                              mode=STLTXCont()))
                 else:
                     streams.append(STLStream(packet=pkt,
                                              flow_stats=STLFlowStats(pg_id=pg_id)
-                                             if not self.config.no_flow_stats else None,
+                                                if not self.config.no_flow_stats else None,
                                              mode=STLTXCont()))
             # for the latency stream, the minimum payload is 16 bytes even in case of vlan tagging
             # without vlan, the min l2 frame size is 64
@@ -662,12 +662,12 @@ class TRex(AbstractTrafficGenerator):
                 streams.append(STLStream(packet=pkt,
                                          flow_stats=STLFlowLatencyStats(pg_id=lat_pg_id,
                                                                         vxlan=True)
-                                         if not self.config.no_latency_stats else None,
+                                            if not self.config.no_latency_stats else None,
                                          mode=STLTXCont(pps=self.LATENCY_PPS)))
             else:
                 streams.append(STLStream(packet=pkt,
                                          flow_stats=STLFlowLatencyStats(pg_id=lat_pg_id)
-                                         if not self.config.no_latency_stats else None,
+                                            if not self.config.no_latency_stats else None,
                                          mode=STLTXCont(pps=self.LATENCY_PPS)))
         return streams
 
@@ -1005,6 +1005,8 @@ class TRex(AbstractTrafficGenerator):
         latency: True if latency measurement is needed
         e2e: True if performing "end to end" connectivity check
         """
+        if self.config.no_flow_stats:
+            LOG.info("Traffic flow statistics are disabled.")
         r = self.__is_rate_enough(l2frame_size, rates, bidirectional, latency)
         if not r['result']:
             raise TrafficGeneratorException(
diff --git a/nfvbench/traffic_server.py b/nfvbench/traffic_server.py
index bc79204..6074a6e 100644
--- a/nfvbench/traffic_server.py
+++ b/nfvbench/traffic_server.py
@@ -108,8 +108,8 @@ class TRexTrafficServer(TrafficServer):
                                          prefix=generator_config.name,
                                          limit_memory=generator_config.limit_memory,
                                          nb_cores=generator_config.cores,
-                                         use_vlan=generator_config.gen_config.get('vtep_vlan')
-                                         or generator_config.vlan_tagging,
+                                         use_vlan=generator_config.gen_config.get('vtep_vlan') or
+                                         generator_config.vlan_tagging,
                                          ifs=ifs)
 
         if hasattr(generator_config, 'mbuf_64') and generator_config.mbuf_64: