From b22e3ed1570f5c36e012022b75d6d025c8cc44d5 Mon Sep 17 00:00:00 2001
From: imarom <imarom@cisco.com>
Date: Thu, 16 Feb 2017 11:38:23 +0200
Subject: tx_capture - code review

Signed-off-by: imarom <imarom@cisco.com>
---
 src/main_dpdk.cpp                           | 23 +++++++++++++++--------
 src/stateless/cp/trex_stateless_port.cpp    |  2 +-
 src/stateless/dp/trex_stateless_dp_core.cpp | 15 ++++++++++-----
 src/stateless/dp/trex_stateless_dp_core.h   |  4 ++--
 src/stateless/rx/trex_stateless_capture.cpp |  2 ++
 5 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/main_dpdk.cpp b/src/main_dpdk.cpp
index 7371b363..5c9d2498 100644
--- a/src/main_dpdk.cpp
+++ b/src/main_dpdk.cpp
@@ -2342,8 +2342,6 @@ int CCoreEthIF::send_pkt_lat(CCorePerPort *lp_port, rte_mbuf_t *m, CVirtualIFPer
     // We allow sending only from first core of each port. This is serious internal bug otherwise.
     assert(lp_port->m_tx_queue_id_lat != INVALID_Q_ID);
 
-    TrexStatelessCaptureMngr::getInstance().handle_pkt_tx(m, lp_port->m_port->get_port_id());
-    
     int ret = lp_port->m_port->tx_burst(lp_port->m_tx_queue_id_lat, &m, 1);
 
 #ifdef DELAY_IF_NEEDED
@@ -2474,22 +2472,28 @@ CCoreEthIFStateless::send_node_packet(CGenNodeStateless      *node_sl,
     }
 }
 
-int CCoreEthIFStateless::send_node(CGenNode *no) {
-    return send_node_common<false>(no);
+int CCoreEthIFStateless::send_node(CGenNode *node) {
+    return send_node_common<false>(node);
 }
 
-int CCoreEthIFStateless::send_node_service_mode(CGenNode *no) {
-    return send_node_common<true>(no);
+int CCoreEthIFStateless::send_node_service_mode(CGenNode *node) {
+    return send_node_common<true>(node);
 }
 
+/**
+ * this is the common function and it is templated 
+ * for two compiler evaluation for performance 
+ * 
+ */
 template <bool SERVICE_MODE>
-int CCoreEthIFStateless::send_node_common(CGenNode *no) {
-    CGenNodeStateless * node_sl = (CGenNodeStateless *) no;
+int CCoreEthIFStateless::send_node_common(CGenNode *node) {
+    CGenNodeStateless * node_sl = (CGenNodeStateless *) node;
     
     pkt_dir_t dir                     = (pkt_dir_t)node_sl->get_mbuf_cache_dir();
     CCorePerPort *lp_port             = &m_ports[dir];
     CVirtualIFPerSideStats *lp_stats  = &m_stats[dir];
     
+    /* generate packet (can never fail) */
     rte_mbuf_t *m = generate_node_pkt(node_sl);
     
     /* template boolean - this will be removed at compile time */
@@ -2497,6 +2501,7 @@ int CCoreEthIFStateless::send_node_common(CGenNode *no) {
         TrexStatelessCaptureMngr::getInstance().handle_pkt_tx(m, lp_port->m_port->get_port_id());
     }
     
+    /* send */
     return send_node_packet(node_sl, m, lp_port, lp_stats);
 }
 
@@ -2512,6 +2517,8 @@ CCoreEthIFStateless::generate_slow_path_node_pkt(CGenNodeStateless *node_sl) {
         return pcap_node->get_pkt();
     }
 
+    /* unhandled case of slow path node */
+    assert(0);
     return (NULL);
 }
 
diff --git a/src/stateless/cp/trex_stateless_port.cpp b/src/stateless/cp/trex_stateless_port.cpp
index 598577cc..423cd443 100644
--- a/src/stateless/cp/trex_stateless_port.cpp
+++ b/src/stateless/cp/trex_stateless_port.cpp
@@ -1001,7 +1001,7 @@ TrexStatelessPort::set_service_mode(bool enabled) {
         assert(0);
     }
     
-    /* update the dp cores */
+    /* update the all the relevant dp cores to move to service mode */
     TrexStatelessDpServiceMode *dp_msg = new TrexStatelessDpServiceMode(m_port_id, enabled);
     send_message_to_all_dp(dp_msg);
 }
diff --git a/src/stateless/dp/trex_stateless_dp_core.cpp b/src/stateless/dp/trex_stateless_dp_core.cpp
index 56184aeb..b6aa15be 100644
--- a/src/stateless/dp/trex_stateless_dp_core.cpp
+++ b/src/stateless/dp/trex_stateless_dp_core.cpp
@@ -27,11 +27,16 @@ limitations under the License.
 #include "trex_streams_compiler.h"
 #include "mbuf.h"
 
-
-class DPCoreWrapper : public CVirtualIF {
+/**
+ * a wrapper for service mode 
+ * it will move the fast send_node virtual call 
+ * to send_node_service_mode which does capturing 
+ * 
+ */
+class ServiceModeWrapper : public CVirtualIF {
 public:
-    
-    DPCoreWrapper() {
+
+    ServiceModeWrapper() {
         m_wrapped = nullptr;
     }
     
@@ -644,7 +649,7 @@ TrexStatelessDpCore::TrexStatelessDpCore() {
     m_core            = NULL;
     m_duration        = -1;
     m_is_service_mode = NULL;
-    m_wrapper         = new DPCoreWrapper();
+    m_wrapper         = new ServiceModeWrapper();
 }
 
 TrexStatelessDpCore::~TrexStatelessDpCore() {
diff --git a/src/stateless/dp/trex_stateless_dp_core.h b/src/stateless/dp/trex_stateless_dp_core.h
index 93128906..21a5a665 100644
--- a/src/stateless/dp/trex_stateless_dp_core.h
+++ b/src/stateless/dp/trex_stateless_dp_core.h
@@ -34,7 +34,7 @@ class CGenNodeStateless;
 class TrexStreamsCompiledObj;
 class TrexStream;
 class CGenNodePCAP;
-class DPCoreWrapper;
+class ServiceModeWrapper;
 
 class CDpOneStream  {
 public:
@@ -340,7 +340,7 @@ private:
 
     double                 m_duration;
     
-    DPCoreWrapper          *m_wrapper;
+    ServiceModeWrapper    *m_wrapper;
     bool                   m_is_service_mode;
 };
 
diff --git a/src/stateless/rx/trex_stateless_capture.cpp b/src/stateless/rx/trex_stateless_capture.cpp
index 3b0273a4..bbdb8f63 100644
--- a/src/stateless/rx/trex_stateless_capture.cpp
+++ b/src/stateless/rx/trex_stateless_capture.cpp
@@ -269,7 +269,9 @@ TrexStatelessCaptureMngr::reset() {
     }
 }
 
+/* define this macro to stress test the critical section */
 //#define STRESS_TEST
+
 void 
 TrexStatelessCaptureMngr::handle_pkt_slow_path(const rte_mbuf_t *m, int port, TrexPkt::origin_e origin) {
     
-- 
cgit 1.2.3-korg