[sonic-swsss] Fix the issue of field "next_hop_ip" not getting update…

…d in state DB in ERSPAN Mirror (sonic-net#1375) * Fix the issue of field "next_hop_ip" not getting updated in State DB Table MIRROR_SESSION_TABLE Updated test case to check next_hop_ip in state db. Also test case enahanced to see if route change in propgated correctly. Signed-off-by: Abhishek Dosi <[email protected]> * Fix test failure and address review comments Signed-off-by: Abhishek Dosi <[email protected]> * Fix for Mirror ERSPAN Failure. Issue is we need to wait till we do not get expected (key,value) tuple instead of waiting for just key Danny was able to reproduce the issue on local setup. It is more timing issue where if dvs container is running fast we will not see this issue as value get updated quickly. Signed-off-by: Abhishek Dosi <[email protected]> * based on syslog on Jenkin route/nexthop update is taking more then 5 seconds. To fix that yse polling interval of 10 seconds. * Increase timeout to 60 seconds Signed-off-by: Abhishek Dosi <[email protected]> * Added Log Signed-off-by: Abhishek Dosi <[email protected]> * Revert some of changes done to debug/fix test failure issue as they are resolved with sonic-build-tools fix Signed-off-by: Abhishek Dosi <[email protected]>
theasianpianist · Aug 19, 2020 · 881c3d4 · 881c3d4
1 parent e3ed57b
commit 881c3d4
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 11 deletions.
diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp
@@ -1206,6 +1206,13 @@ void MirrorOrch::updateNextHop(const NextHopUpdate& update)
             session.nexthopInfo.nexthop = NextHopKey("0.0.0.0", "");
         }
 
+        // Update State DB Nexthop
+        setSessionState(name, session, MIRROR_SESSION_NEXT_HOP_IP);
+
+        SWSS_LOG_NOTICE("Updated mirror session state db %s nexthop to %s",
+                        name.c_str(), session.nexthopInfo.nexthop.to_string().c_str());
+
+
         // Resolve the neighbor of the new next hop
         updateSession(name, session);
     }

diff --git a/tests/conftest.py b/tests/conftest.py
@@ -946,6 +946,10 @@ def add_route(self, prefix, nexthop):
         self.runcmd("ip route add " + prefix + " via " + nexthop)
         time.sleep(1)
 
+    def change_route(self, prefix, nexthop):
+        self.runcmd("ip route change " + prefix + " via " + nexthop)
+        time.sleep(1)
+
     def remove_route(self, prefix):
         self.runcmd("ip route del " + prefix)
         time.sleep(1)

diff --git a/tests/dvslib/dvs_mirror.py b/tests/dvslib/dvs_mirror.py
@@ -67,13 +67,9 @@ def verify_port_mirror_config(self, dvs, ports, direction, session_oid="null"):
 
     def verify_session_db(self, dvs, name, asic_table=None, asic=None, state=None, asic_size=None):
         if asic:
-            fv_pairs = dvs.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION", asic_table)
-            assert all(fv_pairs.get(k) == v for k, v in asic.items())
-            if asic_size:
-                assert asic_size == len(fv_pairs)
+            dvs.asic_db.wait_for_field_match("ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION", asic_table, asic)
         if state:
-            fv_pairs = dvs.state_db.wait_for_entry("MIRROR_SESSION_TABLE", name)
-            assert all(fv_pairs.get(k) == v for k, v in state.items())
+            dvs.state_db.wait_for_field_match("MIRROR_SESSION_TABLE", name, state)
 
     def verify_session_policer(self, dvs, policer_oid, cir):
         if cir:

diff --git a/tests/test_mirror_port_erspan.py b/tests/test_mirror_port_erspan.py
@@ -36,13 +36,17 @@ def test_PortMirrorERSpanAddRemove(self, dvs, testlog):
         self.dvs_mirror.verify_session_status(session, status="inactive")
 
         # add IP address to Ethernet16
-        dvs.add_ip_address("Ethernet16", "10.0.0.0/31")
+        dvs.add_ip_address("Ethernet16", "10.0.0.0/30")
         self.dvs_mirror.verify_session_status(session, status="inactive")
 
         # add neighbor to Ethernet16
         dvs.add_neighbor("Ethernet16", "10.0.0.1", "02:04:06:08:10:12")
         self.dvs_mirror.verify_session_status(session, status="inactive")
 
+        # add another neighbor to Ethernet16
+        dvs.add_neighbor("Ethernet16", "10.0.0.2", "02:04:06:08:10:13")
+        self.dvs_mirror.verify_session_status(session, status="inactive")
+
         # add route to mirror destination via 10.0.0.1
         dvs.add_route("2.2.2.2", "10.0.0.1")
         src_mac = dvs.runcmd("bash -c \"ip link show eth0 | grep ether | awk '{print $2}'\"")[1].strip().upper()
@@ -57,23 +61,50 @@ def test_PortMirrorERSpanAddRemove(self, dvs, testlog):
                             "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": "02:04:06:08:10:12",
                             "SAI_MIRROR_SESSION_ATTR_SRC_MAC_ADDRESS": src_mac,
                             "SAI_MIRROR_SESSION_ATTR_GRE_PROTOCOL_TYPE": "25944"}
+
         expected_state_db = {"status": "active",
-                                "monitor_port": "Ethernet16",
-                                "dst_mac": "02:04:06:08:10:12",
-                                "route_prefix": "2.2.2.2/32"}
+                             "monitor_port": "Ethernet16",
+                             "dst_mac": "02:04:06:08:10:12",
+                             "route_prefix": "2.2.2.2/32",
+                             "next_hop_ip": "10.0.0.1@Ethernet16"}
+
         self.dvs_mirror.verify_session_status(session)
         self.dvs_mirror.verify_session(dvs, session, asic_db=expected_asic_db, state_db=expected_state_db, src_ports=src_asic_ports, asic_size=11)
+        # change route to mirror destination via 10.0.0.2
+        dvs.change_route("2.2.2.2", "10.0.0.2")
+        expected_asic_db = {"SAI_MIRROR_SESSION_ATTR_MONITOR_PORT": pmap.get("Ethernet16"),
+                            "SAI_MIRROR_SESSION_ATTR_TYPE": "SAI_MIRROR_SESSION_TYPE_ENHANCED_REMOTE",
+                            "SAI_MIRROR_SESSION_ATTR_ERSPAN_ENCAPSULATION_TYPE": "SAI_ERSPAN_ENCAPSULATION_TYPE_MIRROR_L3_GRE_TUNNEL",
+                            "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": "4",
+                            "SAI_MIRROR_SESSION_ATTR_TOS": "32",
+                            "SAI_MIRROR_SESSION_ATTR_TTL": "100",
+                            "SAI_MIRROR_SESSION_ATTR_SRC_IP_ADDRESS": "1.1.1.1",
+                            "SAI_MIRROR_SESSION_ATTR_DST_IP_ADDRESS": "2.2.2.2",
+                            "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": "02:04:06:08:10:13",
+                            "SAI_MIRROR_SESSION_ATTR_SRC_MAC_ADDRESS": src_mac,
+                            "SAI_MIRROR_SESSION_ATTR_GRE_PROTOCOL_TYPE": "25944"}
 
+        expected_state_db = {"status": "active",
+                             "monitor_port": "Ethernet16",
+                             "dst_mac": "02:04:06:08:10:13",
+                             "route_prefix": "2.2.2.2/32",
+                             "next_hop_ip": "10.0.0.2@Ethernet16"}
+
+        self.dvs_mirror.verify_session_status(session)
+        self.dvs_mirror.verify_session(dvs, session, asic_db=expected_asic_db, state_db=expected_state_db, src_ports=src_asic_ports, asic_size=11)
+
         # remove route
         dvs.remove_route("2.2.2.2")
         self.dvs_mirror.verify_session_status(session, status="inactive")
 
         # remove neighbor
         dvs.remove_neighbor("Ethernet16", "10.0.0.1")
         self.dvs_mirror.verify_session_status(session, status="inactive")
+        dvs.remove_neighbor("Ethernet16", "10.0.0.2")
+        self.dvs_mirror.verify_session_status(session, status="inactive")
 
         # remove IP address
-        dvs.remove_ip_address("Ethernet16", "10.0.0.0/31")
+        dvs.remove_ip_address("Ethernet16", "10.0.0.0/30")
         self.dvs_mirror.verify_session_status(session, status="inactive")
 
         # bring down Ethernet16