flow stat parsers refactor and adding tests

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

9 files changed, 283 insertions, 352 deletions

diff --git a/src/common/Network/Packet/EthernetHeader.h b/src/common/Network/Packet/EthernetHeader.h
index cf93e850..f5de1e06 100755
--- a/src/common/Network/Packet/EthernetHeader.h
+++ b/src/common/Network/Packet/EthernetHeader.h
@@ -82,11 +82,8 @@ public:
     // Retrieve VLAN fields for tag and protocol information
     inline  uint16_t getVlanTag ();
     inline  uint16_t getVlanProtocol ();
-    inline  uint16_t getQinQTag ();
-    inline  uint16_t getQinQProtocol ();
     void    dump                (FILE*  fd);
 
-
 public:
     MacAddress          myDestination;
     MacAddress          mySource;
@@ -94,8 +91,6 @@ private:
     uint16_t              myProtocol;
     uint16_t              myVlanTag;
     uint16_t              myVlanProtocol;
-    uint16_t            myQinQTag;
-    uint16_t            myQinQProtocol;
 };
 
 #include "EthernetHeader.inl"
diff --git a/src/common/Network/Packet/EthernetHeader.inl b/src/common/Network/Packet/EthernetHeader.inl
index 4091b945..0d6e32c9 100755
--- a/src/common/Network/Packet/EthernetHeader.inl
+++ b/src/common/Network/Packet/EthernetHeader.inl
@@ -37,13 +37,3 @@ inline uint16_t EthernetHeader::getVlanTag()
 {
     return( PKT_HTONS(myVlanTag));
 }
-
-inline uint16_t EthernetHeader::getQinQProtocol()
-{
-    return( PKT_HTONS(myQinQProtocol));
-}
-
-inline uint16_t EthernetHeader::getQinQTag()
-{
-    return( PKT_HTONS(myQinQTag));
-}
diff --git a/src/flow_stat.cpp b/src/flow_stat.cpp
index fcbd8304..58d08c44 100644
--- a/src/flow_stat.cpp
+++ b/src/flow_stat.cpp
@@ -4,7 +4,7 @@
 */
 
 /*
-  Copyright (c) 2015-2016 Cisco Systems, Inc.
+  Copyright (c) 2015-2017 Cisco Systems, Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -500,7 +500,7 @@ void CFlowStatRuleMgr::create() {
     if ((CGlobalInfo::get_queues_mode() == CGlobalInfo::Q_MODE_ONE_QUEUE)
         || (CGlobalInfo::get_queues_mode() == CGlobalInfo::Q_MODE_RSS)) {
         set_mode(FLOW_STAT_MODE_PASS_ALL);
-        m_parser_ipid = new CFlowStatParserSW;
+        m_parser_ipid = new CFlowStatParser(CFlowStatParser::FLOW_STAT_PARSER_MODE_SW);
         m_parser_pl = new CPassAllParser;
     } else {
         m_parser_ipid = m_api->get_flow_stat_parser();
@@ -523,28 +523,17 @@ int CFlowStatRuleMgr::compile_stream(const TrexStream * stream, CFlowStatParser
     std::cout << __METHOD_NAME__ << " user id:" << stream->m_rx_check.m_pg_id << " en:";
     std::cout << stream->m_rx_check.m_enabled << std::endl;
 #endif
+    CFlowStatParser_err_t ret = parser->parse(stream->m_pkt.binary, stream->m_pkt.len);
 
-    if (parser->parse(stream->m_pkt.binary, stream->m_pkt.len) != 0) {
+    if (ret != FSTAT_PARSER_E_OK) {
         // if we could not parse the packet, but no stat count needed, it is probably OK.
         if (stream->m_rx_check.m_enabled) {
-            throw TrexFStatEx("Failed parsing given packet for flow stat. Please consult the manual for supported packet types for flow stat."
-                              , TrexException::T_FLOW_STAT_BAD_PKT_FORMAT);
+            throw TrexFStatEx(parser->get_error_str(ret), TrexException::T_FLOW_STAT_BAD_PKT_FORMAT);
         } else {
             return 0;
         }
     }
 
-    if (!parser->is_stat_supported()) {
-        if (! stream->m_rx_check.m_enabled) {
-            // flow stat not needed. Do nothing.
-            return 0;
-        } else {
-            // flow stat needed, but packet format is not supported
-            throw TrexFStatEx("Unsupported packet format for flow stat on given interface type"
-                              , TrexException::T_FLOW_STAT_UNSUPP_PKT_FORMAT);
-        }
-    }
-
     return 0;
 }
 
@@ -964,7 +953,7 @@ int CFlowStatRuleMgr::set_mode(enum flow_stat_mode_e mode) {
     case FLOW_STAT_MODE_PASS_ALL:
         delete m_parser_ipid;
         delete m_parser_pl;
-        m_parser_ipid = new CFlowStatParserSW;
+        m_parser_ipid = new CFlowStatParser(CFlowStatParser::FLOW_STAT_PARSER_MODE_SW);
         m_parser_pl = new CPassAllParser;
         break;
     case FLOW_STAT_MODE_NORMAL:
diff --git a/src/flow_stat_parser.cpp b/src/flow_stat_parser.cpp
index 417299ee..f9ac8c9b 100644
--- a/src/flow_stat_parser.cpp
+++ b/src/flow_stat_parser.cpp
@@ -4,7 +4,7 @@
 */
 
 /*
-  Copyright (c) 2016-2016 Cisco Systems, Inc.
+  Copyright (c) 2016-2017 Cisco Systems, Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -25,10 +25,30 @@
 #include "common/Network/Packet/IPHeader.h"
 #include "common/Network/Packet/IPv6Header.h"
 #include "common/Network/Packet/TcpHeader.h"
+#include "common/Network/Packet/VLANHeader.h"
 #include "pkt_gen.h"
 #include "flow_stat_parser.h"
 #include "bp_sim.h"
 
+CFlowStatParser::CFlowStatParser(CFlowStatParser_mode mode) {
+    reset();
+    switch(mode) {
+    case FLOW_STAT_PARSER_MODE_HW:
+        m_flags = FSTAT_PARSER_VLAN_SUPP;
+        break;
+    case FLOW_STAT_PARSER_MODE_SW:
+        m_flags = FSTAT_PARSER_VLAN_SUPP | FSTAT_PARSER_QINQ_SUPP;
+        break;
+    // In 82599 we configure the card to either always use VLAN, or never use
+    case FLOW_STAT_PARSER_MODE_82599:
+        m_flags = 0;
+        break;
+    case FLOW_STAT_PARSER_MODE_82599_vlan:
+        m_flags = FSTAT_PARSER_VLAN_SUPP | FSTAT_PARSER_VLAN_NEEDED;
+        break;
+    }
+}
+
 void CFlowStatParser::reset() {
     m_start = 0;
     m_len = 0;
@@ -36,72 +56,98 @@ void CFlowStatParser::reset() {
     m_ipv6 = 0;
     m_l4_proto = 0;
     m_l4 = 0;
-    m_vlan_offset = 0;
-    m_stat_supported = false;
 }
 
-int CFlowStatParser::parse(uint8_t *p, uint16_t len) {
+std::string CFlowStatParser::get_error_str(CFlowStatParser_err_t err) {
+    std::string base = "Failed parsing given packet for flow stat. ";
+
+    switch(err) {
+    case FSTAT_PARSER_E_OK:
+        return "";
+    case FSTAT_PARSER_E_TOO_SHORT:
+        return base + " Packet too short";
+    case FSTAT_PARSER_E_SHORT_IP_HDR:
+        return base + " Packet IP header too short";
+    case FSTAT_PARSER_E_VLAN_NOT_SUP:
+        return base + " NIC does not support vlan (for 82599, you can try starting TRex with --vlan)";
+    case FSTAT_PARSER_E_QINQ_NOT_SUP:
+        return base + " NIC does not support Q in Q";
+    case FSTAT_PARSER_E_MPLS_NOT_SUP:
+        return base + " NIC does not support MPLS";
+    case FSTAT_PARSER_E_UNKNOWN_HDR:
+        return base + " NIC does not support given L2 header type";
+    case FSTAT_PARSER_E_VLAN_NEEDED:
+        return base + " NIC does not support packets with no vlan (If you used --vlan command line arg, try to remove it)";
+        return "";
+    }
+
+    return "";
+}
+
+CFlowStatParser_err_t CFlowStatParser::parse(uint8_t *p, uint16_t len) {
     EthernetHeader *ether = (EthernetHeader *)p;
+    VLANHeader *vlan;
     int min_len = ETH_HDR_LEN;
+    uint16_t next_hdr = ether->getNextProtocol();
+    bool finished = false;
+    bool has_vlan = false;
     reset();
 
-    if (len < min_len)
-        return -1;
-
     m_start = p;
     m_len = len;
-    switch( ether->getNextProtocol() ) {
-    case EthernetHeader::Protocol::IP :
-        min_len += IPV4_HDR_LEN;
-        if (len < min_len)
-            return -1;
-        m_ipv4 = (IPHeader *)(p + ETH_HDR_LEN);
-        m_l4 = ((uint8_t *)m_ipv4) + m_ipv4->getHeaderLength();
-        m_l4_proto = m_ipv4->getProtocol();
-        m_stat_supported = true;
-        break;
-    case EthernetHeader::Protocol::IPv6 :
-        min_len += IPV6_HDR_LEN;
-        if (len < min_len)
-            return -1;
-        m_ipv6 = (IPv6Header *)(p + ETH_HDR_LEN);
-        m_stat_supported = true;
-        break;
-    case EthernetHeader::Protocol::VLAN :
-        m_vlan_offset = 4;
-        min_len += 4;
-        if (len < min_len)
-            return -1;
-        switch ( ether->getVlanProtocol() ){
-        case EthernetHeader::Protocol::IP:
+    if (len < min_len)
+        return FSTAT_PARSER_E_TOO_SHORT;
+
+    p += ETH_HDR_LEN;
+    while (! finished) {
+        switch( next_hdr ) {
+        case EthernetHeader::Protocol::IP :
             min_len += IPV4_HDR_LEN;
             if (len < min_len)
-                    return -1;
-            m_ipv4 = (IPHeader *)(p + ETH_HDR_LEN + 4);
+                return FSTAT_PARSER_E_SHORT_IP_HDR;
+            m_ipv4 = (IPHeader *) p;
             m_l4 = ((uint8_t *)m_ipv4) + m_ipv4->getHeaderLength();
             m_l4_proto = m_ipv4->getProtocol();
-            m_stat_supported = true;
+            finished = true;
             break;
         case EthernetHeader::Protocol::IPv6 :
             min_len += IPV6_HDR_LEN;
             if (len < min_len)
-                return -1;
-            m_ipv6 = (IPv6Header *)(p + ETH_HDR_LEN + 4);
-            m_stat_supported = true;
+                return FSTAT_PARSER_E_SHORT_IP_HDR;
+            m_ipv6 = (IPv6Header *) p;
+            finished = true;
+            break;
+        case EthernetHeader::Protocol::QINQ :
+            if (! (m_flags & FSTAT_PARSER_QINQ_SUPP))
+                return FSTAT_PARSER_E_QINQ_NOT_SUP;
+        case EthernetHeader::Protocol::VLAN :
+            if (! (m_flags & FSTAT_PARSER_VLAN_SUPP))
+                return FSTAT_PARSER_E_VLAN_NOT_SUP;
+            // In QINQ, we also allow multiple 0x8100 headers
+            if (has_vlan && (! (m_flags & FSTAT_PARSER_QINQ_SUPP)))
+                return FSTAT_PARSER_E_QINQ_NOT_SUP;
+            has_vlan = true;
+            min_len += sizeof(VLANHeader);
+            if (len < min_len)
+                return FSTAT_PARSER_E_TOO_SHORT;
+            vlan = (VLANHeader *)p;
+            p += sizeof(VLANHeader);
+            next_hdr = vlan->getNextProtocolHostOrder();
+            break;
+        case EthernetHeader::Protocol::MPLS_Unicast :
+        case EthernetHeader::Protocol::MPLS_Multicast :
+            if (! (m_flags & FSTAT_PARSER_MPLS_SUPP))
+                return FSTAT_PARSER_E_MPLS_NOT_SUP;
             break;
         default:
-            m_stat_supported = false;
-            return -1;
+            return FSTAT_PARSER_E_UNKNOWN_HDR;
         }
-
-        break;
-    default:
-        m_stat_supported = false;
-        return -1;
-        break;
     }
 
-    return 0;
+    if (unlikely(m_flags & FSTAT_PARSER_VLAN_NEEDED) && ! has_vlan) {
+        return FSTAT_PARSER_E_VLAN_NEEDED;
+    }
+    return FSTAT_PARSER_E_OK;
 }
 
 int CFlowStatParser::get_ip_id(uint32_t &ip_id) {
@@ -250,15 +296,16 @@ static const uint8_t TEST_L4_PROTO = IPPROTO_UDP;
 
 
 int CFlowStatParserTest::verify_pkt_one_parser(uint8_t * p, uint16_t pkt_size, uint16_t payload_len, uint32_t ip_id
-                                               , uint8_t l4_proto, CFlowStatParser &parser, bool sup_pkt) {
-    int ret;
+                                               , uint8_t l4_proto, CFlowStatParser &parser, CFlowStatParser_err_t exp_ret) {
+    CFlowStatParser_err_t ret;
     uint32_t pkt_ip_id = 0;
     uint8_t pkt_l4_proto;
     uint16_t pkt_payload_len;
 
     ret = parser.parse(p, pkt_size);
-    if (sup_pkt) {
-        assert (ret == 0);
+    assert(ret == exp_ret);
+
+    if (ret == FSTAT_PARSER_E_OK) {
         parser.get_ip_id(pkt_ip_id);
         assert(pkt_ip_id == ip_id);
         parser.set_ip_id(TEST_IP_ID2);
@@ -269,63 +316,71 @@ int CFlowStatParserTest::verify_pkt_one_parser(uint8_t * p, uint16_t pkt_size, u
             assert(parser.m_ipv4->isChecksumOK() == true);
         assert(parser.get_l4_proto(pkt_l4_proto) == 0);
         assert(pkt_l4_proto == l4_proto);
-        assert(parser.m_stat_supported == true);
-        ret = parser.get_payload_len(p, pkt_size, pkt_payload_len);
-        assert(ret == 0);
+        uint32_t ret2 = parser.get_payload_len(p, pkt_size, pkt_payload_len);
+        assert(ret2 == 0);
         assert(pkt_payload_len == payload_len);
-    } else {
-        assert(ret != 0);
-        assert(parser.m_stat_supported == false);
     }
 
     return 0;
 }
 
 int CFlowStatParserTest::verify_pkt(uint8_t *p, uint16_t pkt_size, uint16_t payload_len, uint32_t ip_id, uint8_t l4_proto
-                                    , uint16_t flags) {
-    int ret, ret_val;
-    CFlowStatParser parser;
-    C82599Parser parser82599(false);
-    C82599Parser parser82599_vlan(true);
+                                    , CFlowStatParserTest_exp_err_t exp_err) {
+    int ret;
+    int ret_val = 0;
+    CFlowStatParser parser_hw(CFlowStatParser::FLOW_STAT_PARSER_MODE_HW);
+    CFlowStatParser parser_sw(CFlowStatParser::FLOW_STAT_PARSER_MODE_SW);
+    CFlowStatParser parser82599(CFlowStatParser::FLOW_STAT_PARSER_MODE_82599);
+    CFlowStatParser parser82599_vlan(CFlowStatParser::FLOW_STAT_PARSER_MODE_82599_vlan);
 
     printf ("  ");
-    if ((flags & P_OK) || (flags & P_BAD)) {
-        printf("general parser");
-        ret = verify_pkt_one_parser(p, pkt_size, payload_len, ip_id, l4_proto, parser, flags & P_OK);
-        ret_val = ret;
-        if (ret == 0)
-            printf("-OK");
-        else {
-            printf("-BAD");
-        }
+    printf("Hardware mode parser");
+    ret = verify_pkt_one_parser(p, pkt_size, payload_len, ip_id, l4_proto, parser_hw, exp_err.m_hw);
+    ret_val = ret;
+    if (ret == 0)
+        printf("-OK");
+    else {
+        printf("-BAD");
+        ret_val = -1;
     }
-    if ((flags & P82599_OK) || (flags & P82599_BAD)) {
-        printf(", 82599 parser");
-        ret = verify_pkt_one_parser(p, pkt_size, payload_len, ip_id, l4_proto, parser82599, flags & P82599_OK);
-        ret_val |= ret;
-        if (ret == 0)
-            printf("-OK");
-        else {
-            printf("-BAD");
-        }
+
+    printf(", software mode parser");
+    ret = verify_pkt_one_parser(p, pkt_size, payload_len, ip_id, l4_proto, parser_sw, exp_err.m_sw);
+    ret_val = ret;
+    if (ret == 0)
+        printf("-OK");
+    else {
+        printf("-BAD");
+        ret_val = -1;
     }
-    if ((flags & P82599_VLAN_OK) || (flags & P82599_VLAN_BAD)) {
-        printf(", 82599 vlan parser");
-        ret = verify_pkt_one_parser(p, pkt_size, payload_len, ip_id, l4_proto, parser82599_vlan, flags & P82599_VLAN_OK);
-        ret_val |= ret;
-        if (ret == 0)
-            printf("-OK");
-        else {
-            printf("-BAD");
-        }
+
+    printf(", 82599 parser");
+    ret = verify_pkt_one_parser(p, pkt_size, payload_len, ip_id, l4_proto, parser82599, exp_err.m_82599);
+    ret_val |= ret;
+    if (ret == 0)
+        printf("-OK");
+    else {
+        printf("-BAD");
+        ret_val = -1;
+    }
+
+    printf(", 82599 vlan parser");
+    ret = verify_pkt_one_parser(p, pkt_size, payload_len, ip_id, l4_proto, parser82599_vlan, exp_err.m_82599_vlan);
+    ret_val |= ret;
+    if (ret == 0)
+        printf("-OK");
+    else {
+        printf("-BAD");
+        ret_val = -1;
     }
+
     printf("\n");
 
     return 0;
 }
 
-int CFlowStatParserTest::test_one_pkt(const char *name, uint16_t ether_type, uint8_t l4_proto, bool is_vlan
-                                      , uint16_t verify_flags) {
+int CFlowStatParserTest::test_one_pkt(const char *name, uint16_t ether_type, uint8_t l4_proto, int vlan_num
+                                      , CFlowStatParserTest_exp_err_t exp_err) {
     CTestPktGen gen;
     uint8_t *p;
     int pkt_size;
@@ -335,216 +390,90 @@ int CFlowStatParserTest::test_one_pkt(const char *name, uint16_t ether_type, uin
 
     printf("%s - ", name);
 
-    // in case of IPv6, we add rx_check header, just to make sure we now how to parse with multiple headers
-    if (is_vlan) {
+    // in case of IPv6, we add rx_check header, just to make sure we know how to parse with multiple headers
+    switch (vlan_num) {
+    case 1:
         pkt_flags = DPF_VLAN | DPF_RXCHECK;
-    } else {
+        break;
+    case 0:
         pkt_flags = DPF_RXCHECK;
+        break;
+    case 2:
+        pkt_flags = DPF_QINQ | DPF_RXCHECK;
+        break;
+    default:
+        printf("Internal error: vlan_num = %d\n", vlan_num);
+        exit(-1);
     }
 
     p = (uint8_t *)gen.create_test_pkt(ether_type, l4_proto, 255, TEST_IP_ID, pkt_flags, payload_len, pkt_size);
-    ret = verify_pkt(p, pkt_size, payload_len, TEST_IP_ID, l4_proto, verify_flags);
+    ret = verify_pkt(p, pkt_size, payload_len, TEST_IP_ID, l4_proto, exp_err);
     free(p);
 
     return ret;
 }
 
 int CFlowStatParserTest::test() {
-    bool vlan = true;
     uint8_t tcp = IPPROTO_TCP, udp = IPPROTO_UDP, icmp = IPPROTO_ICMP;
     uint16_t ipv4 = EthernetHeader::Protocol::IP, ipv6 = EthernetHeader::Protocol::IPv6;
-
-    test_one_pkt("IPv4 TCP", ipv4, tcp, !vlan, P_OK | P82599_OK | P82599_VLAN_BAD);
-    test_one_pkt("IPv4 TCP VLAN", ipv4, tcp, vlan, P_OK | P82599_BAD | P82599_VLAN_OK);
-    test_one_pkt("IPv4 UDP", ipv4, udp, !vlan, P_OK | P82599_OK | P82599_VLAN_BAD);
-    test_one_pkt("IPv4 UDP VLAN", ipv4, udp, vlan, P_OK | P82599_BAD | P82599_VLAN_OK);
-    test_one_pkt("IPv4 ICMP", ipv4, icmp, !vlan, P_OK | P82599_OK | P82599_VLAN_BAD);
-    test_one_pkt("IPv4 ICMP VLAN", ipv4, icmp, vlan, P_OK | P82599_BAD | P82599_VLAN_OK);
-    test_one_pkt("IPv6 TCP", ipv6, tcp, !vlan, P_OK | P82599_BAD | P82599_VLAN_BAD);
-    test_one_pkt("IPv6 TCP VLAN", ipv6, tcp, vlan, P_OK | P82599_BAD | P82599_VLAN_BAD);
-    test_one_pkt("IPv6 UDP", ipv6, udp, !vlan, P_OK | P82599_BAD | P82599_VLAN_BAD);
-    test_one_pkt("IPv6 UDP VLAN", ipv6, udp, vlan, P_OK | P82599_BAD | P82599_VLAN_BAD);
-    test_one_pkt("IPv4 IGMP", ipv4, IPPROTO_IGMP, !vlan, P_OK | P82599_OK | P82599_VLAN_BAD);
-    test_one_pkt("BAD l3 type", 0xaa, icmp, !vlan, P_BAD | P82599_BAD | P82599_VLAN_BAD);
-    test_one_pkt("VLAN + BAD l3 type", 0xaa, icmp, vlan, P_BAD | P82599_BAD | P82599_VLAN_BAD);
-
-    return 0;
-}
-
-int CFlowStatParserSW::parse(uint8_t *p, uint16_t len) {
-    EthernetHeader *ether = (EthernetHeader *)p;
-    int min_len = ETH_HDR_LEN;
-    reset();
-
-    if (len < min_len)
-        return -1;
-
-    m_start = p;
-    m_len = len;
-    switch( ether->getNextProtocol() ) {
-    case EthernetHeader::Protocol::IP :
-        min_len += IPV4_HDR_LEN;
-        if (len < min_len)
-            return -1;
-        m_ipv4 = (IPHeader *)(p + ETH_HDR_LEN);
-        m_l4 = ((uint8_t *)m_ipv4) + m_ipv4->getHeaderLength();
-        m_l4_proto = m_ipv4->getProtocol();
-        m_stat_supported = true;
-        break;
-    case EthernetHeader::Protocol::IPv6 :
-        min_len += IPV6_HDR_LEN;
-        if (len < min_len)
-            return -1;
-        m_ipv6 = (IPv6Header *)(p + ETH_HDR_LEN);
-        m_stat_supported = true;
-        break;
-    case EthernetHeader::Protocol::VLAN :
-        m_vlan_offset = 4;
-        min_len += 4;
-        if (len < min_len)
-            return -1;
-
-        switch ( ether->getVlanProtocol() ){
-        case EthernetHeader::Protocol::IP:
-            min_len += IPV4_HDR_LEN;
-            if (len < min_len)
-                    return -1;
-            m_ipv4 = (IPHeader *)(p + ETH_HDR_LEN + 4);
-            m_l4 = ((uint8_t *)m_ipv4) + m_ipv4->getHeaderLength();
-            m_l4_proto = m_ipv4->getProtocol();
-            m_stat_supported = true;
-            break;
-        case EthernetHeader::Protocol::IPv6 :
-            min_len += IPV6_HDR_LEN;
-            if (len < min_len)
-                return -1;
-            m_ipv6 = (IPv6Header *)(p + ETH_HDR_LEN + 4);
-            m_stat_supported = true;
-            break;
-        case EthernetHeader::Protocol::VLAN :
-            m_vlan_offset = 8;
-            min_len += 8;
-            if (len < min_len)
-                return -1;
-
-            switch ( ether->getQinQProtocol() ){
-            case EthernetHeader::Protocol::IP:
-                min_len += IPV4_HDR_LEN;
-                if (len < min_len)
-                    return -1;
-                m_ipv4 = (IPHeader *)(p + ETH_HDR_LEN + 8);
-                m_l4 = ((uint8_t *)m_ipv4) + m_ipv4->getHeaderLength();
-                m_l4_proto = m_ipv4->getProtocol();
-                m_stat_supported = true;
-                break;
-            case EthernetHeader::Protocol::IPv6 :
-                min_len += IPV6_HDR_LEN;
-                if (len < min_len)
-                    return -1;
-                m_ipv6 = (IPv6Header *)(p + ETH_HDR_LEN + 8);
-                m_stat_supported = true;
-                break;
-            default:
-                m_stat_supported = false;
-                return -1;
-            }
-            break;
-
-        default:
-            m_stat_supported = false;
-            return -1;
-        }
-        break;
-
-    case EthernetHeader::Protocol::QINQ :
-        m_vlan_offset = 8;
-        min_len += 8;
-        if (len < min_len)
-            return -1;
-
-        switch ( ether->getQinQProtocol() ) {
-        case EthernetHeader::Protocol::IP:
-            min_len += IPV4_HDR_LEN;
-            if (len < min_len)
-                    return -1;
-            m_ipv4 = (IPHeader *)(p + ETH_HDR_LEN + 8);
-            m_l4 = ((uint8_t *)m_ipv4) + m_ipv4->getHeaderLength();
-            m_l4_proto = m_ipv4->getProtocol();
-            m_stat_supported = true;
-            break;
-        case EthernetHeader::Protocol::IPv6 :
-            min_len += IPV6_HDR_LEN;
-            if (len < min_len)
-                return -1;
-            m_ipv6 = (IPv6Header *)(p + ETH_HDR_LEN + 8);
-            m_stat_supported = true;
-            break;
-        default:
-            m_stat_supported = false;
-            return -1;
-        }
-        break;
-
-    default:
-        m_stat_supported = false;
-        return -1;
-        break;
-    }
+    CFlowStatParserTest_exp_err_t exp_ret;
+
+    // no vlan tests
+    exp_ret.m_hw = FSTAT_PARSER_E_OK;
+    exp_ret.m_sw = FSTAT_PARSER_E_OK;
+    exp_ret.m_82599 = FSTAT_PARSER_E_OK;
+    exp_ret.m_82599_vlan = FSTAT_PARSER_E_VLAN_NEEDED;
+    test_one_pkt("IPv4 TCP", ipv4, tcp, 0, exp_ret);
+    test_one_pkt("IPv4 UDP", ipv4, udp, 0, exp_ret);
+    test_one_pkt("IPv4 ICMP", ipv4, icmp, 0, exp_ret);
+    test_one_pkt("IPv6 TCP", ipv6, tcp, 0, exp_ret);
+    test_one_pkt("IPv6 UDP", ipv6, udp, 0, exp_ret);
+    test_one_pkt("IPv4 IGMP", ipv4, IPPROTO_IGMP, 0, exp_ret);
+
+    // vlan tests
+    exp_ret.m_hw = FSTAT_PARSER_E_OK;
+    exp_ret.m_sw = FSTAT_PARSER_E_OK;
+    exp_ret.m_82599 = FSTAT_PARSER_E_VLAN_NOT_SUP;
+    exp_ret.m_82599_vlan = FSTAT_PARSER_E_OK;;
+    test_one_pkt("IPv4 TCP VLAN", ipv4, tcp, 1, exp_ret);
+    test_one_pkt("IPv4 UDP VLAN", ipv4, udp, 1, exp_ret);
+    test_one_pkt("IPv4 ICMP VLAN", ipv4, icmp, 1, exp_ret);
+    test_one_pkt("IPv6 TCP VLAN", ipv6, tcp, 1, exp_ret);
+    test_one_pkt("IPv6 UDP VLAN", ipv6, udp, 1, exp_ret);
+
+    // qinq tests
+    exp_ret.m_hw = FSTAT_PARSER_E_QINQ_NOT_SUP;
+    exp_ret.m_sw = FSTAT_PARSER_E_OK;
+    exp_ret.m_82599 = FSTAT_PARSER_E_QINQ_NOT_SUP;
+    exp_ret.m_82599_vlan = FSTAT_PARSER_E_QINQ_NOT_SUP;
+    test_one_pkt("IPv4 TCP QINQ", ipv4, tcp, 2, exp_ret);
+    test_one_pkt("IPv4 UDP QINQ", ipv4, udp, 2, exp_ret);
+    test_one_pkt("IPv4 ICMP QINQ", ipv4, icmp, 2, exp_ret);
+    test_one_pkt("IPv6 TCP QINQ", ipv6, tcp, 2, exp_ret);
+    test_one_pkt("IPv6 UDP QINQ", ipv6, udp, 2, exp_ret);
+
+    // bad packets tests
+    exp_ret.m_hw = FSTAT_PARSER_E_UNKNOWN_HDR;
+    exp_ret.m_sw = FSTAT_PARSER_E_UNKNOWN_HDR;
+    exp_ret.m_82599 = FSTAT_PARSER_E_UNKNOWN_HDR;
+    exp_ret.m_82599_vlan = FSTAT_PARSER_E_UNKNOWN_HDR;
+    test_one_pkt("BAD l3 type", 0xaa, icmp, 0, exp_ret);
+
+    exp_ret.m_82599 = FSTAT_PARSER_E_VLAN_NOT_SUP;
+    test_one_pkt("VLAN + BAD l3 type", 0xaa, icmp, 1, exp_ret);
 
     return 0;
 }
 
-// In 82599 10G card we do not support VLANs
-int C82599Parser::parse(uint8_t *p, uint16_t len) {
-    EthernetHeader *ether = (EthernetHeader *)p;
-    int min_len = ETH_HDR_LEN + IPV4_HDR_LEN;
-    reset();
-
-    if (len < min_len)
-        return -1;
-
-    switch( ether->getNextProtocol() ) {
-    case EthernetHeader::Protocol::IP :
-        // In 82599 all streams should be with vlan, or without. Can't mix
-        if (m_vlan_supported)
-            return -1;
-        m_ipv4 = (IPHeader *)(p + ETH_HDR_LEN);
-        m_stat_supported = true;
-        break;
-    case EthernetHeader::Protocol::VLAN :
-        if (!m_vlan_supported)
-            return -1;
-        min_len += 4;
-        if (len < min_len)
-            return -1;
-        switch ( ether->getVlanProtocol() ){
-        case EthernetHeader::Protocol::IP:
-            m_ipv4 = (IPHeader *)(p + 18);
-            m_stat_supported = true;
-            break;
-        default:
-            m_stat_supported = false;
-            return -1;
-        }
-        break;
-    default:
-        m_stat_supported = false;
-        return -1;
-        break;
-    }
-
-    return 0;
-}
-
-int CPassAllParser::parse(uint8_t *pkt, uint16_t len) {
+CFlowStatParser_err_t CPassAllParser::parse(uint8_t *pkt, uint16_t len) {
     reset();
 
     if (len < ETH_HDR_LEN)
-        return -1;
+        return FSTAT_PARSER_E_TOO_SHORT;
 
     m_len = len;
 
-    return 0;
+    return FSTAT_PARSER_E_OK;
 }
 
 bool CSimplePacketParser::Parse(){
diff --git a/src/flow_stat_parser.h b/src/flow_stat_parser.h
index 682bc09f..c00d7e33 100644
--- a/src/flow_stat_parser.h
+++ b/src/flow_stat_parser.h
@@ -4,7 +4,7 @@
 */
 
 /*
-  Copyright (c) 2016-2016 Cisco Systems, Inc.
+  Copyright (c) 2016-2017 Cisco Systems, Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -28,15 +28,42 @@
 #include "common/Network/Packet/TcpHeader.h"
 #include "mbuf.h"
 
+typedef enum CFlowStatParser_err {
+    FSTAT_PARSER_E_OK = 0,
+    FSTAT_PARSER_E_TOO_SHORT,
+    FSTAT_PARSER_E_SHORT_IP_HDR,
+    FSTAT_PARSER_E_VLAN_NOT_SUP,
+    FSTAT_PARSER_E_QINQ_NOT_SUP,
+    FSTAT_PARSER_E_MPLS_NOT_SUP,
+    FSTAT_PARSER_E_UNKNOWN_HDR,
+    FSTAT_PARSER_E_VLAN_NEEDED,
+
+} CFlowStatParser_err_t;
+
 // Basic flow stat parser. Imitating HW behavior. It can parse only packets matched by HW fdir rules we define.
 // Relevant for xl710/x710, i350, Cisco VIC, Mellanox cards
+
 class CFlowStatParser {
     friend class CFlowStatParserTest;
+
+    enum CFlowStatParser_flags {
+        FSTAT_PARSER_VLAN_SUPP = 0x1,
+        FSTAT_PARSER_VLAN_NEEDED = 0x2,
+        FSTAT_PARSER_QINQ_SUPP = 0x4,
+        FSTAT_PARSER_MPLS_SUPP = 0x8,
+    };
  public:
+    enum CFlowStatParser_mode {
+        FLOW_STAT_PARSER_MODE_HW, // all NICs except Intel 82599
+        FLOW_STAT_PARSER_MODE_SW, // --software mode and virtual NICs
+        FLOW_STAT_PARSER_MODE_82599,
+        FLOW_STAT_PARSER_MODE_82599_vlan,
+    };
+    CFlowStatParser(CFlowStatParser_mode mode);
     virtual ~CFlowStatParser() {}
     virtual void reset();
-    virtual int parse(uint8_t *pkt, uint16_t len);
-    virtual bool is_stat_supported() {return m_stat_supported == true;}
+    std::string get_error_str(CFlowStatParser_err_t err);
+    virtual CFlowStatParser_err_t parse(uint8_t *pkt, uint16_t len);
     virtual int get_ip_id(uint32_t &ip_id);
     virtual int set_ip_id(uint32_t ip_id);
     virtual int get_l3_proto(uint16_t &proto);
@@ -93,34 +120,15 @@ class CFlowStatParser {
     IPHeader *m_ipv4;
     IPv6Header *m_ipv6;
     uint8_t *m_l4;
-    bool m_stat_supported;
     uint8_t m_l4_proto;
     uint8_t m_vlan_offset;
-};
-
-// parser used in --software mode and virtual cards. No hardware limitation. We can support any packert type here.
-class CFlowStatParserSW : public CFlowStatParser {
- public:
-    CFlowStatParserSW() {}
-    ~CFlowStatParserSW() {}
-    int parse(uint8_t *pkt, uint16_t len);
-};
-
-// relevant for 82599 card
-class C82599Parser : public CFlowStatParser {
- public:
-    C82599Parser(bool vlan_supported) {m_vlan_supported = vlan_supported;}
-    ~C82599Parser() {}
-    int parse(uint8_t *pkt, uint16_t len);
-
- private:
-    bool m_vlan_supported;
+    uint16_t m_flags;
 };
 
 class CPassAllParser : public CFlowStatParser {
  public:
-    virtual int parse(uint8_t *pkt, uint16_t len);
-    virtual bool is_stat_supported() {return true;}
+ CPassAllParser() : CFlowStatParser (CFlowStatParser::FLOW_STAT_PARSER_MODE_SW) {}
+    virtual CFlowStatParser_err_t parse(uint8_t *pkt, uint16_t len);
     virtual int get_ip_id(uint32_t &ip_id) { ip_id = 0; return 0;}
     virtual int set_ip_id(uint32_t ip_id){return 0;}
     virtual int get_l3_proto(uint16_t &proto){proto = 0; return 0;}
@@ -182,6 +190,13 @@ class CSimplePacketParser {
 };
 
 class CFlowStatParserTest {
+    typedef struct {
+        CFlowStatParser_err_t m_hw;
+        CFlowStatParser_err_t m_sw;
+        CFlowStatParser_err_t m_82599;
+        CFlowStatParser_err_t m_82599_vlan;
+    } CFlowStatParserTest_exp_err_t;
+
     enum {
         P_OK = 0x1,
         P_BAD = 0x2,
@@ -189,16 +204,20 @@ class CFlowStatParserTest {
         P82599_BAD = 0x8,
         P82599_VLAN_OK = 0x10,
         P82599_VLAN_BAD = 0x20,
+        P_SW_OK = 0x40,
+        P_SW_BAD = 0x80,
     };
 
  public:
     int test();
 
  private:
-    int test_one_pkt(const char *name, uint16_t ether_type, uint8_t l4_proto, bool is_vlan, uint16_t verify_flags);
-    int verify_pkt(uint8_t *p, uint16_t pkt_size, uint16_t payload_len, uint32_t ip_id, uint8_t l4_proto, uint16_t flags);
+    int test_one_pkt(const char *name, uint16_t ether_type, uint8_t l4_proto, int vlan_num
+                     , CFlowStatParserTest_exp_err_t exp_err);
+    int verify_pkt(uint8_t *p, uint16_t pkt_size, uint16_t payload_len, uint32_t ip_id, uint8_t l4_proto
+                   , CFlowStatParserTest_exp_err_t exp_err);
     int verify_pkt_one_parser(uint8_t * p, uint16_t pkt_size, uint16_t payload_len, uint32_t ip_id, uint8_t l4_proto
-                              , CFlowStatParser &parser, bool sup_pkt);
+                              , CFlowStatParser &parser, CFlowStatParser_err_t exp_ret);
 };
 
 #endif
diff --git a/src/internal_api/trex_platform_api.h b/src/internal_api/trex_platform_api.h
index 5bd8ff99..16752652 100644
--- a/src/internal_api/trex_platform_api.h
+++ b/src/internal_api/trex_platform_api.h
@@ -4,7 +4,7 @@
 */
 
 /*
-Copyright (c) 2015-2015 Cisco Systems, Inc.
+Copyright (c) 2015-2017 Cisco Systems, Inc.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -263,7 +263,8 @@ public:
     int get_active_pgids(flow_stat_active_t &result) const {return 0;}
     int get_cpu_util_full(cpu_util_full_t &result) const {return 0;}
     int get_mbuf_util(Json::Value &result) const {return 0;}
-    CFlowStatParser *get_flow_stat_parser() const {return new CFlowStatParser();}
+    CFlowStatParser *get_flow_stat_parser() const {
+        return new CFlowStatParser(CFlowStatParser::FLOW_STAT_PARSER_MODE_SW);}
     TRexPortAttr *getPortAttrObj(uint8_t port_id) const {return m_port_attr;}
 
     void mark_for_shutdown() const {}
diff --git a/src/main_dpdk.cpp b/src/main_dpdk.cpp
index 59536867..434254a2 100644
--- a/src/main_dpdk.cpp
+++ b/src/main_dpdk.cpp
@@ -5997,7 +5997,7 @@ void CTRexExtendedDriverBase::wait_after_link_up() {
 }
 
 CFlowStatParser *CTRexExtendedDriverBase::get_flow_stat_parser() {
-    CFlowStatParser *parser = new CFlowStatParser();
+    CFlowStatParser *parser = new CFlowStatParser(CFlowStatParser::FLOW_STAT_PARSER_MODE_HW);
     assert (parser);
     return parser;
 }
@@ -6551,8 +6551,10 @@ int CTRexExtendedDriverBase10G::wait_for_stable_link(){
 }
 
 CFlowStatParser *CTRexExtendedDriverBase10G::get_flow_stat_parser() {
-    CFlowStatParser *parser = new C82599Parser((CGlobalInfo::m_options.preview.get_vlan_mode()
-                                                != CPreviewMode::VLAN_MODE_NONE) ? true:false);
+    CFlowStatParser *parser = new CFlowStatParser((CGlobalInfo::m_options.preview.get_vlan_mode()
+                                                   != CPreviewMode::VLAN_MODE_NONE)
+                                                  ? CFlowStatParser::FLOW_STAT_PARSER_MODE_82599_vlan
+                                                  : CFlowStatParser::FLOW_STAT_PARSER_MODE_82599);
     assert (parser);
     return parser;
 }
@@ -6874,7 +6876,7 @@ int CTRexExtendedDriverBase40G::verify_fw_ver(int port_id) {
 }
 
 CFlowStatParser *CTRexExtendedDriverBase40G::get_flow_stat_parser() {
-    CFlowStatParser *parser = new CFlowStatParser();
+    CFlowStatParser *parser = new CFlowStatParser(CFlowStatParser::FLOW_STAT_PARSER_MODE_HW);
     assert (parser);
     return parser;
 }
@@ -7070,7 +7072,7 @@ int CTRexExtendedDriverBaseMlnx5G::wait_for_stable_link(){
 }
 
 CFlowStatParser *CTRexExtendedDriverBaseMlnx5G::get_flow_stat_parser() {
-    CFlowStatParser *parser = new CFlowStatParser();
+    CFlowStatParser *parser = new CFlowStatParser(CFlowStatParser::FLOW_STAT_PARSER_MODE_HW);
     assert (parser);
     return parser;
 }
@@ -7257,7 +7259,7 @@ int CTRexExtendedDriverBaseVIC::dump_fdir_global_stats(CPhyEthIF * _if, FILE *fd
 }
 
 CFlowStatParser *CTRexExtendedDriverBaseVIC::get_flow_stat_parser() {
-    CFlowStatParser *parser = new CFlowStatParser();
+    CFlowStatParser *parser = new CFlowStatParser(CFlowStatParser::FLOW_STAT_PARSER_MODE_HW);
     assert (parser);
     return parser;
 }
@@ -7291,7 +7293,7 @@ int CTRexExtendedDriverVirtBase::wait_for_stable_link(){
 }
 
 CFlowStatParser *CTRexExtendedDriverVirtBase::get_flow_stat_parser() {
-    CFlowStatParser *parser = new CFlowStatParserSW();
+    CFlowStatParser *parser = new CFlowStatParser(CFlowStatParser::FLOW_STAT_PARSER_MODE_SW);
     assert (parser);
     return parser;
 }
diff --git a/src/pkt_gen.cpp b/src/pkt_gen.cpp
index 9f6a3d34..9dc5ade1 100644
--- a/src/pkt_gen.cpp
+++ b/src/pkt_gen.cpp
@@ -4,7 +4,7 @@
 */
 
 /*
-  Copyright (c) 2016-2016 Cisco Systems, Inc.
+  Copyright (c) 2016-2017 Cisco Systems, Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -45,8 +45,12 @@ char *CTestPktGen::create_test_pkt(uint16_t l3_type, uint16_t l4_proto, uint8_t
     // ASA 1
     //        uint8_t dst_mac[6] = {0xd4, 0x8c, 0xb5, 0xc9, 0x54, 0x2b};
     //      uint8_t src_mac[6] = {0xa0, 0x36, 0x9f, 0x38, 0xa4, 0x0};
-    if (flags & DPF_VLAN) {
-        l2_proto = 0x0081;
+    if (flags & (DPF_VLAN | DPF_QINQ)) {
+        if (flags & DPF_QINQ) {
+            l2_proto = htons(EthernetHeader::Protocol::QINQ);
+        } else {
+            l2_proto = htons(EthernetHeader::Protocol::VLAN);
+        }
     } else {
         l2_proto = htons(l3_type);
     }
@@ -106,7 +110,7 @@ char *CTestPktGen::create_test_pkt(uint16_t l3_type, uint16_t l4_proto, uint8_t
 
     pkt_size = 14;
 
-    if (flags & DPF_VLAN) {
+    if (flags & (DPF_VLAN | DPF_QINQ)) {
         pkt_size += 4;
         if (flags & DPF_QINQ) {
             pkt_size += 4;
@@ -159,8 +163,10 @@ char *CTestPktGen::create_test_pkt(uint16_t l3_type, uint16_t l4_proto, uint8_t
     memcpy(p, src_mac, sizeof(src_mac)); p += sizeof(src_mac);
     memcpy(p, &l2_proto, sizeof(l2_proto)); p += sizeof(l2_proto);
 
-    if (flags & DPF_VLAN) {
+    if (flags & (DPF_VLAN | DPF_QINQ)) {
         if (flags & DPF_QINQ) {
+            uint16_t vlan_type = htons(EthernetHeader::Protocol::VLAN);
+            memcpy(&vlan_header2[2], &vlan_type, sizeof(vlan_type));
             memcpy(p, &vlan_header2, sizeof(vlan_header2)); p += sizeof(vlan_header2);
         }
 
diff --git a/src/stateless/rx/trex_stateless_rx_port_mngr.cpp b/src/stateless/rx/trex_stateless_rx_port_mngr.cpp
index ca06619b..0beeae69 100644
--- a/src/stateless/rx/trex_stateless_rx_port_mngr.cpp
+++ b/src/stateless/rx/trex_stateless_rx_port_mngr.cpp
@@ -54,7 +54,7 @@ RXLatency::create(CRFC2544Info *rfc2544, CRxCoreErrCntrs *err_cntrs) {
 
 void 
 RXLatency::handle_pkt(const rte_mbuf_t *m) {
-    CFlowStatParserSW parser;
+    CFlowStatParser parser(CFlowStatParser::FLOW_STAT_PARSER_MODE_SW);
     int ret = parser.parse(rte_pktmbuf_mtod(m, uint8_t *), m->pkt_len);
 
     if (m_rcv_all ||  (ret == 0)) {