Small reorder in drivers inheritance + fix to e1000 CRC issue (trex-354)

Signed-off-by: Ido Barnea <ibarnea@cisco.com>

5 files changed, 52 insertions, 50 deletions

diff --git a/src/main_dpdk.cpp b/src/main_dpdk.cpp
index ca322312..f8ec0556 100644
--- a/src/main_dpdk.cpp
+++ b/src/main_dpdk.cpp
@@ -175,7 +175,6 @@ public:
     virtual CFlowStatParser *get_flow_stat_parser();
     virtual int set_rcv_all(CPhyEthIF * _if, bool set_on)=0;
     virtual TRexPortAttr * create_port_attr(uint8_t port_id) = 0;
-    virtual uint8_t get_num_crc_fix_bytes() {return 0;}
 
     /* Does this NIC type support automatic packet dropping in case of a link down?
        in case it is supported the packets will be dropped, else there would be a back pressure to tx queues
@@ -243,14 +242,9 @@ public:
     virtual int set_rcv_all(CPhyEthIF * _if, bool set_on);
 };
 
-class CTRexExtendedDriverVirtio : public CTRexExtendedDriverBase {
-
+// Base for all virtual drivers. No constructor. Should not create object from this type.
+class CTRexExtendedDriverVirtBase : public CTRexExtendedDriverBase {
 public:
-    CTRexExtendedDriverVirtio() {
-        /* we are working in mode that we have 1 queue for rx and one queue for tx*/
-        CGlobalInfo::m_options.preview.set_vm_one_queue_enable(true);
-    }
-
     TRexPortAttr * create_port_attr(uint8_t port_id) {
         return new DpdkTRexPortAttr(port_id, true, true);
     }
@@ -259,10 +253,6 @@ public:
         return false;
     }
 
-    static CTRexExtendedDriverBase * create(){
-        return ( new CTRexExtendedDriverVirtio() );
-    }
-
     virtual void update_global_config_fdir(port_cfg_t * cfg) {}
 
     virtual int get_min_sample_rate(void){
@@ -281,7 +271,7 @@ public:
     }
 
     virtual int stop_queue(CPhyEthIF * _if, uint16_t q_num);
-    virtual void get_extended_stats(CPhyEthIF * _if,CPhyEthIFStats *stats);
+    virtual void get_extended_stats(CPhyEthIF * _if,CPhyEthIFStats *stats)=0;
     virtual void clear_extended_stats(CPhyEthIF * _if);
     virtual int wait_for_stable_link();
     virtual int get_stat_counters_num() {return MAX_FLOW_STATS;}
@@ -292,7 +282,19 @@ public:
     virtual int set_rcv_all(CPhyEthIF * _if, bool set_on) {return 0;}
 };
 
-class CTRexExtendedDriverVmxnet3 : public CTRexExtendedDriverVirtio {
+class CTRexExtendedDriverVirtio : public CTRexExtendedDriverVirtBase {
+public:
+    CTRexExtendedDriverVirtio() {
+        /* we are working in mode that we have 1 queue for rx and one queue for tx*/
+        CGlobalInfo::m_options.preview.set_vm_one_queue_enable(true);
+    }
+    static CTRexExtendedDriverBase * create(){
+        return ( new CTRexExtendedDriverVirtio() );
+    }
+    virtual void get_extended_stats(CPhyEthIF * _if,CPhyEthIFStats *stats);
+};
+
+class CTRexExtendedDriverVmxnet3 : public CTRexExtendedDriverVirtBase {
 public:
     CTRexExtendedDriverVmxnet3(){
         /* we are working in mode in which we have 1 queue for rx and one queue for tx*/
@@ -302,11 +304,11 @@ public:
     static CTRexExtendedDriverBase * create() {
         return ( new CTRexExtendedDriverVmxnet3() );
     }
-
+    virtual void get_extended_stats(CPhyEthIF * _if,CPhyEthIFStats *stats);
     virtual void update_configuration(port_cfg_t * cfg);
 };
 
-class CTRexExtendedDriverI40evf : public CTRexExtendedDriverVirtio {
+class CTRexExtendedDriverI40evf : public CTRexExtendedDriverVirtBase {
 public:
     CTRexExtendedDriverI40evf(){
         /* we are working in mode in which we have 1 queue for rx and one queue for tx*/
@@ -342,7 +344,7 @@ public:
     }
 };
 
-class CTRexExtendedDriverBaseE1000 : public CTRexExtendedDriverVirtio {
+class CTRexExtendedDriverBaseE1000 : public CTRexExtendedDriverVirtBase {
     CTRexExtendedDriverBaseE1000() {
         // E1000 driver is only relevant in VM in our case
         CGlobalInfo::m_options.preview.set_vm_one_queue_enable(true);
@@ -352,8 +354,7 @@ public:
         return ( new CTRexExtendedDriverBaseE1000() );
     }
     // e1000 driver handing us packets with ethernet CRC, so we need to chop them
-    virtual uint8_t get_num_crc_fix_bytes() {return 4;}
-
+    virtual void update_configuration(port_cfg_t * cfg);
     virtual void get_extended_stats(CPhyEthIF * _if,CPhyEthIFStats *stats);
 
 };
@@ -3777,7 +3778,6 @@ void CGlobalTRex::rx_sl_configure(void) {
 
     rx_sl_cfg.m_max_ports = m_max_ports;
     rx_sl_cfg.m_tx_cores  = get_cores_tx();
-    rx_sl_cfg.m_num_crc_fix_bytes = get_ex_drv()->get_num_crc_fix_bytes();
         
     if ( get_vm_one_queue_enable() ) {
         /* vm mode, indirect queues  */
@@ -7260,7 +7260,7 @@ CFlowStatParser *CTRexExtendedDriverBaseVIC::get_flow_stat_parser() {
 
 
 /////////////////////////////////////////////////////////////////////////////////////
-void CTRexExtendedDriverVirtio::update_configuration(port_cfg_t * cfg){
+void CTRexExtendedDriverVirtBase::update_configuration(port_cfg_t * cfg){
     cfg->m_tx_conf.tx_thresh.pthresh = TX_PTHRESH_1G;
     cfg->m_tx_conf.tx_thresh.hthresh = TX_HTHRESH;
     cfg->m_tx_conf.tx_thresh.wthresh = 0;
@@ -7268,31 +7268,42 @@ void CTRexExtendedDriverVirtio::update_configuration(port_cfg_t * cfg){
     cfg->m_tx_conf.txq_flags |= ETH_TXQ_FLAGS_NOXSUMS;
 }
 
-int CTRexExtendedDriverVirtio::configure_rx_filter_rules(CPhyEthIF * _if){
+int CTRexExtendedDriverVirtBase::configure_rx_filter_rules(CPhyEthIF * _if){
     return (0);
 }
 
-void CTRexExtendedDriverVirtio::clear_extended_stats(CPhyEthIF * _if){
+void CTRexExtendedDriverVirtBase::clear_extended_stats(CPhyEthIF * _if){
     rte_eth_stats_reset(_if->get_port_id());
 }
 
-int CTRexExtendedDriverVirtio::stop_queue(CPhyEthIF * _if, uint16_t q_num) {
+int CTRexExtendedDriverVirtBase::stop_queue(CPhyEthIF * _if, uint16_t q_num) {
     return (0);
 }
 
-void CTRexExtendedDriverBaseE1000::get_extended_stats(CPhyEthIF * _if,CPhyEthIFStats *stats){
-    get_extended_stats_fixed(_if, stats, 0, 4);
+int CTRexExtendedDriverVirtBase::wait_for_stable_link(){
+    wait_x_sec(CGlobalInfo::m_options.m_wait_before_traffic);
+    return (0);
 }
 
 void CTRexExtendedDriverVirtio::get_extended_stats(CPhyEthIF * _if,CPhyEthIFStats *stats) {
     get_extended_stats_fixed(_if, stats, 4, 4);
 }
 
-int CTRexExtendedDriverVirtio::wait_for_stable_link(){
-    wait_x_sec(CGlobalInfo::m_options.m_wait_before_traffic);
-    return (0);
+void CTRexExtendedDriverVmxnet3::get_extended_stats(CPhyEthIF * _if,CPhyEthIFStats *stats) {
+    get_extended_stats_fixed(_if, stats, 4, 4);
+}
+
+void CTRexExtendedDriverBaseE1000::get_extended_stats(CPhyEthIF * _if,CPhyEthIFStats *stats){
+    get_extended_stats_fixed(_if, stats, 0, 4);
 }
 
+void CTRexExtendedDriverBaseE1000::update_configuration(port_cfg_t * cfg) {
+    // We configure hardware not to strip CRC. Then DPDK driver removes the CRC.
+    // If configuring "hardware" to remove CRC, due to bug in ESXI e1000 emulation, we got packets with CRC.
+    cfg->m_port_conf.rxmode.hw_strip_crc = 0;
+}
+
+
 /////////////////////////////////////////////////////////// VMxnet3
 void CTRexExtendedDriverVmxnet3::update_configuration(port_cfg_t * cfg){
     cfg->m_tx_conf.tx_thresh.pthresh = TX_PTHRESH_1G;
diff --git a/src/stateless/rx/trex_stateless_rx_core.cpp b/src/stateless/rx/trex_stateless_rx_core.cpp
index 03351036..016b7193 100644
--- a/src/stateless/rx/trex_stateless_rx_core.cpp
+++ b/src/stateless/rx/trex_stateless_rx_core.cpp
@@ -91,8 +91,7 @@ void CRxCoreStateless::create(const CRxSlCfg &cfg) {
                                  cfg.m_ports[i],
                                  m_rfc2544,
                                  &m_err_cntrs,
-                                 &m_cpu_dp_u,
-                                 cfg.m_num_crc_fix_bytes);
+                                 &m_cpu_dp_u);
     }
 }
 
diff --git a/src/stateless/rx/trex_stateless_rx_defs.h b/src/stateless/rx/trex_stateless_rx_defs.h
index 367cf4e3..ce134605 100644
--- a/src/stateless/rx/trex_stateless_rx_defs.h
+++ b/src/stateless/rx/trex_stateless_rx_defs.h
@@ -37,7 +37,6 @@ class CRxSlCfg {
     CRxSlCfg (){
         m_max_ports = 0;
         m_cps = 0.0;
-        m_num_crc_fix_bytes = 0;
         m_tx_cores = 0;
     }
 
@@ -46,7 +45,6 @@ class CRxSlCfg {
     uint32_t             m_tx_cores;
     double               m_cps;
     CPortLatencyHWBase * m_ports[TREX_MAX_PORTS];
-    uint8_t              m_num_crc_fix_bytes;
 };
 
 /**
diff --git a/src/stateless/rx/trex_stateless_rx_port_mngr.cpp b/src/stateless/rx/trex_stateless_rx_port_mngr.cpp
index 6ebe0a97..4e24a4a7 100644
--- a/src/stateless/rx/trex_stateless_rx_port_mngr.cpp
+++ b/src/stateless/rx/trex_stateless_rx_port_mngr.cpp
@@ -102,6 +102,12 @@ RXLatency::handle_pkt(const rte_mbuf_t *m) {
                                     curr_rfc2544->inc_seq_err(pkt_seq - exp_seq);
                                     curr_rfc2544->inc_seq_err_too_big();
                                     curr_rfc2544->set_seq(pkt_seq + 1);
+#ifdef LAT_DEBUG
+                                    printf("magic:%x flow_seq:%x hw_id:%x seq %x exp %x\n"
+                                           , fsp_head->magic, fsp_head->flow_seq, fsp_head->hw_id, pkt_seq
+                                           , exp_seq);
+                                    utl_DumpBuffer(stdout, rte_pktmbuf_mtod(m, uint8_t *), m->pkt_len, 0);
+#endif
                                 } else {
                                     if (pkt_seq == (exp_seq - 1)) {
                                         curr_rfc2544->inc_dup();
@@ -128,6 +134,9 @@ RXLatency::handle_pkt(const rte_mbuf_t *m) {
                                     curr_rfc2544->inc_seq_err(pkt_seq - exp_seq);
                                     curr_rfc2544->inc_seq_err_too_big();
                                     curr_rfc2544->set_seq(pkt_seq + 1);
+#ifdef LAT_DEBUG
+                                    printf("2:seq %d exp %d\n", exp_seq, pkt_seq);
+#endif
                                 }
                             }
                         } else {
@@ -574,13 +583,10 @@ RXPortManager::create(const TRexPortAttr *port_attr,
                       CPortLatencyHWBase *io,
                       CRFC2544Info *rfc2544,
                       CRxCoreErrCntrs *err_cntrs,
-                      CCpuUtlDp *cpu_util,
-                      uint8_t crc_bytes_num) {
-    
+                      CCpuUtlDp *cpu_util) {
     m_port_id = port_attr->get_port_id();
     m_io = io;
     m_cpu_dp_u = cpu_util;
-    m_num_crc_fix_bytes = crc_bytes_num;
     
     /* init features */
     m_latency.create(rfc2544, err_cntrs);
@@ -628,11 +634,6 @@ int RXPortManager::process_all_pending_pkts(bool flush_rx) {
         rte_mbuf_t *m = rx_pkts[j];
 
         if (!flush_rx) {
-            // patch relevant only for e1000 driver
-            if (m_num_crc_fix_bytes) {
-                rte_pktmbuf_trim(m, m_num_crc_fix_bytes);
-            }
-
             handle_pkt(m);
         }
 
diff --git a/src/stateless/rx/trex_stateless_rx_port_mngr.h b/src/stateless/rx/trex_stateless_rx_port_mngr.h
index b318d973..ea360490 100644
--- a/src/stateless/rx/trex_stateless_rx_port_mngr.h
+++ b/src/stateless/rx/trex_stateless_rx_port_mngr.h
@@ -201,15 +201,12 @@ public:
                 CPortLatencyHWBase *io,
                 CRFC2544Info *rfc2544,
                 CRxCoreErrCntrs *err_cntrs,
-                CCpuUtlDp *cpu_util,
-                uint8_t crc_bytes_num);
+                CCpuUtlDp *cpu_util);
 
-    
     void clear_stats() {
         m_latency.reset_stats();
     }
 
-    
     void get_latency_stats(rx_per_flow_t *rx_stats,
                            int min,
                            int max,
@@ -344,10 +341,6 @@ private:
     RXQueue                      m_queue;
     RXServer                     m_server;
     RXGratARP                    m_grat_arp;
-    
-    // compensate for the fact that hardware send us packets without Ethernet CRC, and we report with it
-    uint8_t m_num_crc_fix_bytes;
-    
     CCpuUtlDp                   *m_cpu_dp_u;
     CPortLatencyHWBase          *m_io;
     CManyIPInfo                  m_src_addr;