changes from code review

Change-Id: I628608643d902bd6310b04b8036fc5f1fcc42309
Signed-off-by: Yaroslav Brustinov <ybrustin@cisco.com>

10 files changed, 114 insertions, 105 deletions

diff --git a/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_client.py b/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_client.py
index 2aa0450d..a42247e7 100755
--- a/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_client.py
+++ b/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_client.py
@@ -2509,7 +2509,8 @@ class STLClient(object):
                     Ports on which to execute the command
 
                 ipg_usec : float
-                    Inter-packet gap in microseconds
+                    Inter-packet gap in microseconds.
+                    Exclusive with min_ipg_usec
 
                 speedup : float
                     A factor to adjust IPG. effectively IPG = IPG / speedup
@@ -2528,6 +2529,7 @@ class STLClient(object):
 
                 min_ipg_usec : float
                     Minimum inter-packet gap in microseconds to guard from too small ipg.
+                    Exclusive with ipg_usec
 
             :raises:
                 + :exc:`STLError`
@@ -2596,7 +2598,8 @@ class STLClient(object):
                     Ports on which to execute the command
 
                 ipg_usec : float
-                    Inter-packet gap in microseconds
+                    Inter-packet gap in microseconds.
+                    Exclusive with min_ipg_usec
 
                 speedup : float
                     A factor to adjust IPG. effectively IPG = IPG / speedup
@@ -2624,6 +2627,7 @@ class STLClient(object):
 
                 min_ipg_usec : float
                     Minimum inter-packet gap in microseconds to guard from too small ipg.
+                    Exclusive with ipg_usec
 
             :raises:
                 + :exc:`STLError`
@@ -2640,6 +2644,8 @@ class STLClient(object):
         validate_type('vm', vm, (list, type(None)))
         validate_type('is_dual', is_dual, bool)
         validate_type('min_ipg_usec', min_ipg_usec, (float, int, type(None)))
+        if all([ipg_usec, min_ipg_usec]):
+            raise STLError('Please specify either ipg or minimal ipg, not both.')
 
 
         # no support for > 1MB PCAP - use push remote
@@ -3696,24 +3702,24 @@ class STLClient(object):
 
         if opts.remote:
             self.push_remote(opts.file[0],
-                             ports     = opts.ports,
-                             ipg_usec  = opts.ipg_usec,
+                             ports          = opts.ports,
+                             ipg_usec       = opts.ipg_usec,
                              min_ipg_usec   = opts.min_ipg_usec,
-                             speedup   = opts.speedup,
-                             count     = opts.count,
-                             duration  = opts.duration,
-                             is_dual   = opts.dual)
+                             speedup        = opts.speedup,
+                             count          = opts.count,
+                             duration       = opts.duration,
+                             is_dual        = opts.dual)
 
         else:
             self.push_pcap(opts.file[0],
-                           ports     = opts.ports,
-                           ipg_usec  = opts.ipg_usec,
+                           ports          = opts.ports,
+                           ipg_usec       = opts.ipg_usec,
                            min_ipg_usec   = opts.min_ipg_usec,
-                           speedup   = opts.speedup,
-                           count     = opts.count,
-                           duration  = opts.duration,
-                           force     = opts.force,
-                           is_dual   = opts.dual)
+                           speedup        = opts.speedup,
+                           count          = opts.count,
+                           duration       = opts.duration,
+                           force          = opts.force,
+                           is_dual        = opts.dual)
 
         
 
diff --git a/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_streams.py b/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_streams.py
index 3bce671a..26613e56 100755
--- a/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_streams.py
+++ b/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_streams.py
@@ -1028,6 +1028,11 @@ class STLProfile(object):
         # check filename
         if not os.path.isfile(pcap_file):
             raise STLError("file '{0}' does not exists".format(pcap_file))
+        if speedup <= 0:
+            raise STLError('Speedup should not be negative.')
+        if min_ipg_usec and min_ipg_usec < 0:
+            raise STLError('min_ipg_usec should not be negative.')
+
 
         # make sure IPG is not less than 0.001 usec
         if (ipg_usec is not None and (ipg_usec < 0.001 * speedup) and
@@ -1092,34 +1097,24 @@ class STLProfile(object):
     def __pkts_to_streams (pkts, ipg_usec, min_ipg_usec, speedup, loop_count, vm, packet_hook, start_delay_usec = 0):
 
         streams = []
-        if speedup == 0:
-            raise STLError('Speedup should not be 0')
-        if min_ipg_usec and min_ipg_usec < 0:
-            raise STLError('min_ipg_usec should not be negative.')
-
         if packet_hook:
             pkts = [(packet_hook(cap), meta) for (cap, meta) in pkts]
 
-        if ipg_usec == None:
-            constant_diff = None
-        else:
-            constant_diff = ipg_usec / float(speedup)
-            if min_ipg_usec is not None:
-                constant_diff = max(constant_diff, min_ipg_usec)
-
         for i, (cap, meta) in enumerate(pkts, start = 1):
             # IPG - if not provided, take from cap
-            if constant_diff is None:
+            if ipg_usec is None:
                 packet_time = meta[0] * 1e6 + meta[1]
                 if i == 1:
-                    isg = min_ipg_usec if min_ipg_usec else 0
-                else:
-                    isg = (packet_time - prev_time) / float(speedup)
-                    if min_ipg_usec:
-                        isg = max(isg, min_ipg_usec)
+                    prev_time = packet_time
+                isg = (packet_time - prev_time) / float(speedup)
+                if min_ipg_usec and isg < min_ipg_usec:
+                    isg = min_ipg_usec
                 prev_time = packet_time
-            else:
-                isg = constant_diff
+            else: # user specified ipg
+                if min_ipg_usec:
+                    isg = min_ipg_usec
+                else:
+                    isg = ipg_usec / float(speedup)
 
             # handle last packet
             if i == len(pkts):
@@ -1128,28 +1123,12 @@ class STLProfile(object):
             else:
                 next = i + 1
                 action_count = 0
-            self_start = False if i != 1 else True
-
-            # add stream with delay that will not be part of loop: "delayed_start" -> 1 -> 2 -> 3 -> ... -> 1 -> 2
-            if start_delay_usec and i == 1:
-                if loop_count == 1: # no loop actually
-                    isg = start_delay_usec
-                else:
-                    streams.append(STLStream(name = 'delayed_start',
-                                             packet = STLPktBuilder(pkt_buffer = cap, vm = vm),
-                                             mode = STLTXSingleBurst(total_pkts = 1, percentage = 100),
-                                             self_start = True,
-                                             isg = start_delay_usec,
-                                             action_count = action_count,
-                                             next = next))
-                    action_count = max(0, action_count - 1)
-                    self_start = False
 
             streams.append(STLStream(name = i,
                                      packet = STLPktBuilder(pkt_buffer = cap, vm = vm),
                                      mode = STLTXSingleBurst(total_pkts = 1, percentage = 100),
-                                     self_start = self_start,
-                                     isg = isg,
+                                     self_start = True if (i == 1) else False,
+                                     isg = isg,  # usec
                                      action_count = action_count,
                                      next = next))
 
diff --git a/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_types.py b/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_types.py
index 5ae3cb27..a60a7ede 100644
--- a/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_types.py
+++ b/scripts/automation/trex_control_plane/stl/trex_stl_lib/trex_stl_types.py
@@ -60,11 +60,10 @@ class RC():
             show_count = 10
             err_list = []
             err_count = 0
-            for x in self.rc_list:
-                if x.data and not x.rc:
-                    err_count += 1
-                    if len(err_list) < show_count:
-                        err_list.append(format_text(x.data, 'bold'))
+            for x in filter(len, listify(self.err())):
+                err_count += 1
+                if len(err_list) < show_count:
+                    err_list.append(format_text(x, 'bold'))
             s = '\n' if len(err_list) > 1 else ''
             if err_count > show_count:
                 s += format_text('Occurred %s errors, showing first %s:\n' % (err_count, show_count), 'bold')
diff --git a/scripts/automation/trex_control_plane/stl/trex_stl_lib/utils/text_opts.py b/scripts/automation/trex_control_plane/stl/trex_stl_lib/utils/text_opts.py
index 6c5dd2c3..63b05bf4 100644
--- a/scripts/automation/trex_control_plane/stl/trex_stl_lib/utils/text_opts.py
+++ b/scripts/automation/trex_control_plane/stl/trex_stl_lib/utils/text_opts.py
@@ -131,21 +131,13 @@ def yellow(text):
 def underline(text):
     return text_attribute(text, 'underline')
 
-
-start_end_newlines = re.compile('^(\n)*([^\n].*[^\n])?(\n)*$', re.DOTALL)
+# apply attribute on each non-empty line
 def text_attribute(text, attribute):
-    match = start_end_newlines.match(text)
-    try:
-        startpad, msg, endpad = match.groups('')
-    except:
-        startpad = endpad = ''
-        msg = text
-    return "{startpad}{startattr}{txt}{endattr}{endpad}".format(
-                startpad = startpad,
-                startattr = TEXT_CODES[attribute]['start'],
-                txt = msg,
-                endattr = TEXT_CODES[attribute]['end'],
-                endpad = endpad)
+    return '\n'.join(['{start}{txt}{end}'.format(
+            start = TEXT_CODES[attribute]['start'],
+            txt = line,
+            end = TEXT_CODES[attribute]['end'])
+                    if line else '' for line in ('%s' % text).split('\n')])
 
 
 FUNC_DICT = {'blue': blue,
diff --git a/src/rpc-server/commands/trex_rpc_cmd_general.cpp b/src/rpc-server/commands/trex_rpc_cmd_general.cpp
index b3b1b3b4..d4854a79 100644
--- a/src/rpc-server/commands/trex_rpc_cmd_general.cpp
+++ b/src/rpc-server/commands/trex_rpc_cmd_general.cpp
@@ -631,11 +631,8 @@ TrexRpcCmdPushRemote::_run(const Json::Value &params, Json::Value &result) {
     uint8_t port_id = parse_port(params, result);
     std::string  pcap_filename  = parse_string(params, "pcap_filename", result);
     double       ipg_usec       = parse_double(params, "ipg_usec", result);
-    double       min_ipg_sec    = 0;
-    if (params.isMember("min_ipg_usec")) {
-        min_ipg_sec = usec_to_sec(parse_double(params, "min_ipg_usec", result));
-    }
-    double       speedup        = parse_double(params, "speedup", result);
+    double       min_ipg_sec    = usec_to_sec(parse_udouble(params, "min_ipg_usec", result, 0));
+    double       speedup        = parse_udouble(params, "speedup", result);
     uint32_t     count          = parse_uint32(params, "count", result);
     double       duration       = parse_double(params, "duration", result);
     bool         is_dual        = parse_bool(params,   "is_dual", result, false);
diff --git a/src/rpc-server/commands/trex_rpc_cmd_stream.cpp b/src/rpc-server/commands/trex_rpc_cmd_stream.cpp
index 9a57c5f9..3f73a5d7 100644
--- a/src/rpc-server/commands/trex_rpc_cmd_stream.cpp
+++ b/src/rpc-server/commands/trex_rpc_cmd_stream.cpp
@@ -40,7 +40,7 @@ TrexRpcCmdAddStream::_run(const Json::Value &params, Json::Value &result) {
 
     uint8_t port_id = parse_port(params, result);
 
-    uint32_t stream_id  = parse_int(params, "stream_id", result);
+    uint32_t stream_id  = parse_uint32(params, "stream_id", result);
 
     const Json::Value &section = parse_object(params, "stream", result);
 
@@ -62,7 +62,7 @@ TrexRpcCmdAddStream::_run(const Json::Value &params, Json::Value &result) {
     stream->m_random_seed     = parse_uint32(section, "random_seed", result,0); /* default is zero */
 
     /* inter stream gap */
-    stream->m_isg_usec  = parse_double(section, "isg", result);
+    stream->m_isg_usec  = parse_udouble(section, "isg", result);
 
     stream->m_next_stream_id = parse_int(section, "next_stream_id", result);
 
@@ -114,7 +114,7 @@ TrexRpcCmdAddStream::_run(const Json::Value &params, Json::Value &result) {
             generate_parse_err(result, "RX stats is not supported on this interface");
         }
 
-        stream->m_rx_check.m_pg_id      = parse_int(rx, "stream_id", result);
+        stream->m_rx_check.m_pg_id      = parse_uint32(rx, "stream_id", result);
         std::string type = parse_string(rx, "rule_type", result);
         if (type == "latency") {
             stream->m_rx_check.m_rule_type = TrexPlatformApi::IF_STAT_PAYLOAD;
@@ -155,7 +155,7 @@ TrexRpcCmdAddStream::allocate_new_stream(const Json::Value &section, uint8_t por
 
     } else if (type == "single_burst") {
 
-        uint32_t total_pkts      = parse_int(mode, "total_pkts", result);
+        uint32_t total_pkts      = parse_uint32(mode, "total_pkts", result);
 
         stream.reset(new TrexStream(TrexStream::stSINGLE_BURST, port_id, stream_id));
         stream->set_single_burst(total_pkts);
@@ -163,9 +163,9 @@ TrexRpcCmdAddStream::allocate_new_stream(const Json::Value &section, uint8_t por
 
     } else if (type == "multi_burst") {
 
-        double    ibg_usec         = parse_double(mode, "ibg", result);
-        uint32_t  num_bursts       = parse_int(mode, "count", result);
-        uint32_t  pkts_per_burst   = parse_int(mode, "pkts_per_burst", result);
+        double    ibg_usec         = parse_udouble(mode, "ibg", result);
+        uint32_t  num_bursts       = parse_uint32(mode, "count", result);
+        uint32_t  pkts_per_burst   = parse_uint32(mode, "pkts_per_burst", result);
 
         stream.reset(new TrexStream(TrexStream::stMULTI_BURST,port_id, stream_id ));
         stream->set_multi_burst(pkts_per_burst,num_bursts,ibg_usec);
@@ -186,12 +186,7 @@ TrexRpcCmdAddStream::allocate_new_stream(const Json::Value &section, uint8_t por
 void 
 TrexRpcCmdAddStream::parse_rate(const Json::Value &rate, std::unique_ptr<TrexStream> &stream, Json::Value &result) {
 
-    double value = parse_double(rate, "value", result);
-    if (value <= 0) {
-        std::stringstream ss;
-        ss << "rate value must be a positive number - got: '" << value << "'";
-        generate_parse_err(result, ss.str());
-    }
+    double value = parse_udouble(rate, "value", result);
 
     auto rate_types = {"pps", "bps_L1", "bps_L2", "percentage"};
     std::string rate_type = parse_choice(rate, "type", rate_types, result);
@@ -533,7 +528,7 @@ TrexRpcCmdRemoveStream::_run(const Json::Value &params, Json::Value &result) {
     uint8_t port_id = parse_port(params, result);
     TrexStatelessPort *port = get_stateless_obj()->get_port_by_id(port_id);
 
-    uint32_t stream_id = parse_int(params, "stream_id", result);
+    uint32_t stream_id = parse_uint32(params, "stream_id", result);
     TrexStream *stream = port->get_stream_by_id(stream_id);
 
     if (!stream) {
@@ -615,7 +610,7 @@ TrexRpcCmdGetStream::_run(const Json::Value &params, Json::Value &result) {
     TrexStatelessPort *port = get_stateless_obj()->get_port_by_id(port_id);
 
     bool     get_pkt   = parse_bool(params, "get_pkt", result);
-    uint32_t stream_id = parse_int(params, "stream_id", result);
+    uint32_t stream_id = parse_uint32(params, "stream_id", result);
 
     TrexStream *stream = port->get_stream_by_id(stream_id);
 
@@ -660,9 +655,9 @@ TrexRpcCmdStartTraffic::_run(const Json::Value &params, Json::Value &result) {
 
     std::string type   = parse_choice(mul_obj, "type", TrexPortMultiplier::g_types, result);
     std::string op     = parse_string(mul_obj, "op", result);
-    double      value  = parse_double(mul_obj, "value", result);
+    double      value  = parse_udouble(mul_obj, "value", result);
     
-    if ( value <=0 ){
+    if ( value == 0 ){
         generate_parse_err(result, "multiplier can't be zero");
     }
 
diff --git a/src/rpc-server/trex_rpc_cmd.cpp b/src/rpc-server/trex_rpc_cmd.cpp
index 265d426b..6c56a59f 100644
--- a/src/rpc-server/trex_rpc_cmd.cpp
+++ b/src/rpc-server/trex_rpc_cmd.cpp
@@ -153,6 +153,8 @@ TrexRpcCommand::type_to_str(field_type_e type) {
         return "int";
     case FIELD_TYPE_DOUBLE:
         return "double";
+    case FIELD_TYPE_UDOUBLE:
+        return "unsigned double";
     case FIELD_TYPE_OBJ:
         return "object";
     case FIELD_TYPE_STR:
@@ -176,7 +178,7 @@ TrexRpcCommand::json_type_to_name(const Json::Value &value) {
     case Json::uintValue:
         return "uint";
     case Json::realValue:
-        return "real";
+        return "double";
     case Json::stringValue:
         return "string";
     case Json::booleanValue:
@@ -223,31 +225,41 @@ TrexRpcCommand::check_field_type(const Json::Value &parent, const std::string &n
 
 void 
 TrexRpcCommand::check_field_type_common(const Json::Value &field, const std::string &name, field_type_e type, Json::Value &result) {
-    std::stringstream ss;
+    std::string specific_err;
 
     /* first check if field exists */
     if (field == Json::Value::null) {
-        ss << "field '" << name << "' is missing";
-        generate_parse_err(result, ss.str());
+        specific_err = "field '" + name + "' is missing";
+        generate_parse_err(result, specific_err);
     }
 
     bool rc = true;
+    specific_err = "is '" + std::string(json_type_to_name(field)) + "', expecting '" + std::string(type_to_str(type)) + "'";
 
     switch (type) {
     case FIELD_TYPE_BYTE:
-        if ( (!field.isUInt()) || (field.asInt() > 0xFF)) {
+        if (!field.isUInt64()) {
+            rc = false;
+        } else if (field.asUInt64() > 0xFF) {
+            specific_err = "has size bigger than uint8.";
             rc = false;
         }
         break;
 
     case FIELD_TYPE_UINT16:
-        if ( (!field.isUInt()) || (field.asInt() > 0xFFFF)) {
+        if (!field.isUInt64()) {
+            rc = false;
+        } else if (field.asUInt64() > 0xFFFF) {
+            specific_err = "has size bigger than uint16.";
             rc = false;
         }
         break;
 
     case FIELD_TYPE_UINT32:
-        if ( (!field.isUInt()) || (field.asUInt() > 0xFFFFFFFF)) {
+        if (!field.isUInt64()) {
+            rc = false;
+        } else if (field.asUInt64() > 0xFFFFFFFF) {
+            specific_err = "has size bigger than uint32.";
             rc = false;
         }
         break;
@@ -276,6 +288,15 @@ TrexRpcCommand::check_field_type_common(const Json::Value &field, const std::str
         }
         break;
 
+    case FIELD_TYPE_UDOUBLE:
+        if (!field.isDouble()) {
+            rc = false;
+        } else if (field.asDouble() < 0) {
+            specific_err =  "has negative value.";
+            rc = false;
+        }
+        break;
+
     case FIELD_TYPE_OBJ:
         if (!field.isObject()) {
             rc = false;
@@ -300,8 +321,7 @@ TrexRpcCommand::check_field_type_common(const Json::Value &field, const std::str
 
     }
     if (!rc) {
-        ss << "error at offset: " << field.getOffsetStart() << " - '" << name << "' is '" << json_type_to_name(field) << "', expecting '" << type_to_str(type) << "'";
-        generate_parse_err(result, ss.str());
+        generate_parse_err(result, "error at offset: " + std::to_string(field.getOffsetStart()) + " - '" + name + "' " + specific_err);
     }
 
 }
diff --git a/src/rpc-server/trex_rpc_cmd_api.h b/src/rpc-server/trex_rpc_cmd_api.h
index de0f5b58..2536f69c 100644
--- a/src/rpc-server/trex_rpc_cmd_api.h
+++ b/src/rpc-server/trex_rpc_cmd_api.h
@@ -114,6 +114,7 @@ protected:
         FIELD_TYPE_UINT64,
         FIELD_TYPE_INT,
         FIELD_TYPE_DOUBLE,
+        FIELD_TYPE_UDOUBLE,
         FIELD_TYPE_BOOL,
         FIELD_TYPE_STR,
         FIELD_TYPE_OBJ,
@@ -184,6 +185,11 @@ protected:
         return parent[param].asDouble();
     }
 
+    template<typename T> double parse_udouble(const Json::Value &parent, const T &param, Json::Value &result) {
+        check_field_type(parent, param, FIELD_TYPE_UDOUBLE, result);
+        return parent[param].asDouble();
+    }
+
     template<typename T> bool parse_bool(const Json::Value &parent, const T &param, Json::Value &result) {
         check_field_type(parent, param, FIELD_TYPE_BOOL, result);
         return parent[param].asBool();
@@ -256,6 +262,20 @@ protected:
         return parse_double(parent, param, result);
     }
 
+    template<typename T> double parse_udouble(const Json::Value &parent, const T &param, Json::Value &result, double def) {
+        /* if not exists - default */
+        if (parent[param] == Json::Value::null) {
+            if (def < 0) {
+                std::stringstream ss;
+                ss << "default value of '" << param << "' is negative (please report)";
+                generate_parse_err(result, ss.str());
+            } else {
+                return def;
+            }
+        }
+        return parse_udouble(parent, param, result);
+    }
+
     template<typename T> bool parse_bool(const Json::Value &parent, const T &param, Json::Value &result, bool def) {
         /* if not exists - default */
         if (parent[param] == Json::Value::null) {
diff --git a/src/stateless/cp/trex_stateless_port.cpp b/src/stateless/cp/trex_stateless_port.cpp
index 170098a3..e9b3c6d4 100644
--- a/src/stateless/cp/trex_stateless_port.cpp
+++ b/src/stateless/cp/trex_stateless_port.cpp
@@ -909,8 +909,8 @@ TrexStatelessPort::add_stream(TrexStream *stream) {
 
     verify_state(PORT_STATE_IDLE | PORT_STATE_STREAMS, "add_stream");
 
-    if (m_stream_table.size() >= 20000) {
-        throw TrexException("Reached limit of 20k streams at the port.");
+    if (m_stream_table.size() >= MAX_STREAMS) {
+        throw TrexException("Reached limit of " + std::to_string(MAX_STREAMS) + " streams at the port.");
     }
     get_stateless_obj()->m_rx_flow_stat.add_stream(stream);
 
diff --git a/src/stateless/cp/trex_stateless_port.h b/src/stateless/cp/trex_stateless_port.h
index 404e1355..d4ac4018 100644
--- a/src/stateless/cp/trex_stateless_port.h
+++ b/src/stateless/cp/trex_stateless_port.h
@@ -514,6 +514,7 @@ private:
     TrexPortOwner       m_owner;
 
     int m_pending_async_stop_event;
+    static const uint32_t MAX_STREAMS = 20000;
 
 };