From c865803e8072a5576a396da2810f84815c4635ba Mon Sep 17 00:00:00 2001 From: mramezani95 Date: Mon, 18 Nov 2024 12:56:47 -0800 Subject: [PATCH 1/8] [mirror] erspan ipv6 underlay (see PR #1817) (#3317) * Applied changes from PR #1817 Why I did it Unlock erspan capability on td3 and beyond that supports erspan ipv6 encap. What I did Extend test_MirrorAddSetRemove, test_MirrorToVlanAddRemove, test_MirrorToLagAddRemove, test_MirrorDestMoveVlan, test_MirrorDestMoveLag to vs test erspan ipv6 encap On td3 --- orchagent/mirrororch.cpp | 28 +++-- tests/test_mirror.py | 248 ++++++++++++++++++++++++++------------- 2 files changed, 179 insertions(+), 97 deletions(-) diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp index d2981330e42..b05f1c3288b 100644 --- a/orchagent/mirrororch.cpp +++ b/orchagent/mirrororch.cpp @@ -34,7 +34,8 @@ #define MIRROR_SESSION_DEFAULT_VLAN_PRI 0 #define MIRROR_SESSION_DEFAULT_VLAN_CFI 0 -#define MIRROR_SESSION_DEFAULT_IP_HDR_VER 4 +#define MIRROR_SESSION_IP_HDR_VER_4 4 +#define MIRROR_SESSION_IP_HDR_VER_6 6 #define MIRROR_SESSION_DSCP_SHIFT 2 #define MIRROR_SESSION_DSCP_MIN 0 #define MIRROR_SESSION_DSCP_MAX 63 @@ -380,6 +381,9 @@ task_process_status MirrorOrch::createEntry(const string& key, const vector 0 return { fv[0]: fv[1] for fv in fvs } + def mirror_session_entry_exists(self, name): + tbl = swsscommon.Table(self.sdb, "MIRROR_SESSION_TABLE") + (status, _) = tbl.get(name) + return status + def check_syslog(self, dvs, marker, log, expected_cnt): (ec, out) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep \'%s\' | wc -l" % (marker, log)]) assert out.strip() == str(expected_cnt) - def test_MirrorAddRemove(self, dvs, testlog): + def test_MirrorInvalidEntry(self, dvs): + """ + This test ensures that an invalid mirror session entry is not created. + Here, "invalid" means an entry in which src IP is IPv4 while dst IP is IPv6 + (or vice versa). + """ + self.setup_db(dvs) + session = "TEST_SESSION" + self.create_mirror_session(session, "1.1.1.1", "fc00::2:2:2:2", "0x6558", "8", "100", "0") + assert self.mirror_session_entry_exists(session) == False + + def _test_MirrorAddRemove(self, dvs, testlog, v6_encap=False): """ This test covers the basic mirror session creation and removal operations Operation flow: @@ -119,34 +135,37 @@ def test_MirrorAddRemove(self, dvs, testlog): The session becomes inactive again till the end 4. Remove miror session """ - self.setup_db(dvs) - session = "TEST_SESSION" + src_ip = "1.1.1.1" if v6_encap == False else "fc00::1:1:1:1" + dst_ip = "2.2.2.2" if v6_encap == False else "fc00::2:2:2:2" + intf_addr = "10.0.0.0/31" if v6_encap == False else "fc00::/126" + nhop_ip = "10.0.0.1" if v6_encap == False else "fc00::1" marker = dvs.add_log_marker() # create mirror session - self.create_mirror_session(session, "1.1.1.1", "2.2.2.2", "0x6558", "8", "100", "0") + self.create_mirror_session(session, src_ip, dst_ip, "0x6558", "8", "100", "0") assert self.get_mirror_session_state(session)["status"] == "inactive" - self.check_syslog(dvs, marker, "Attached next hop observer .* for destination IP 2.2.2.2", 1) + assert self.get_mirror_session_state(session)["next_hop_ip"] == ("0.0.0.0@" if v6_encap == False else "::@") + self.check_syslog(dvs, marker, "Attached next hop observer .* for destination IP {}".format(dst_ip), 1) # bring up Ethernet16 self.set_interface_status(dvs, "Ethernet16", "up") assert self.get_mirror_session_state(session)["status"] == "inactive" # add IP address to Ethernet16 - self.add_ip_address("Ethernet16", "10.0.0.0/31") + self.add_ip_address("Ethernet16", intf_addr) assert self.get_mirror_session_state(session)["status"] == "inactive" # add neighbor to Ethernet16 - self.add_neighbor("Ethernet16", "10.0.0.1", "02:04:06:08:10:12") + self.add_neighbor("Ethernet16", nhop_ip, "02:04:06:08:10:12") assert self.get_mirror_session_state(session)["status"] == "inactive" - # add route to mirror destination via 10.0.0.1 - self.add_route(dvs, "2.2.2.2", "10.0.0.1") + # add route to mirror destination via next hop ip + self.add_route(dvs, dst_ip, nhop_ip) assert self.get_mirror_session_state(session)["status"] == "active" assert self.get_mirror_session_state(session)["monitor_port"] == "Ethernet16" assert self.get_mirror_session_state(session)["dst_mac"] == "02:04:06:08:10:12" - assert self.get_mirror_session_state(session)["route_prefix"] == "2.2.2.2/32" + assert self.get_mirror_session_state(session)["route_prefix"] == "{}/{}".format(dst_ip, 32 if v6_encap == False else 128) # check asic database tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION") @@ -164,15 +183,15 @@ def test_MirrorAddRemove(self, dvs, testlog): elif fv[0] == "SAI_MIRROR_SESSION_ATTR_ERSPAN_ENCAPSULATION_TYPE": assert fv[1] == "SAI_ERSPAN_ENCAPSULATION_TYPE_MIRROR_L3_GRE_TUNNEL" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": - assert fv[1] == "4" + assert fv[1] == "4" if v6_encap == False else "6" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_TOS": assert fv[1] == "32" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_TTL": assert fv[1] == "100" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_SRC_IP_ADDRESS": - assert fv[1] == "1.1.1.1" + assert fv[1] == src_ip elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_IP_ADDRESS": - assert fv[1] == "2.2.2.2" + assert fv[1] == dst_ip elif fv[0] == "SAI_MIRROR_SESSION_ATTR_SRC_MAC_ADDRESS": assert fv[1] == dvs.runcmd("bash -c \"ip link show eth0 | grep ether | awk '{print $2}'\"")[1].strip().upper() elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": @@ -183,15 +202,15 @@ def test_MirrorAddRemove(self, dvs, testlog): assert False # remove route - self.remove_route(dvs, "2.2.2.2") + self.remove_route(dvs, dst_ip) assert self.get_mirror_session_state(session)["status"] == "inactive" # remove neighbor - self.remove_neighbor("Ethernet16", "10.0.0.1") + self.remove_neighbor("Ethernet16", nhop_ip) assert self.get_mirror_session_state(session)["status"] == "inactive" # remove IP address - self.remove_ip_address("Ethernet16", "10.0.0.0/31") + self.remove_ip_address("Ethernet16", intf_addr) assert self.get_mirror_session_state(session)["status"] == "inactive" # bring down Ethernet16 @@ -201,7 +220,13 @@ def test_MirrorAddRemove(self, dvs, testlog): marker = dvs.add_log_marker() # remove mirror session self.remove_mirror_session(session) - self.check_syslog(dvs, marker, "Detached next hop observer for destination IP 2.2.2.2", 1) + self.check_syslog(dvs, marker, "Detached next hop observer for destination IP {}".format(dst_ip), 1) + + def test_MirrorAddRemove(self, dvs, testlog): + self.setup_db(dvs) + + self._test_MirrorAddRemove(dvs, testlog) + self._test_MirrorAddRemove(dvs, testlog, v6_encap=True) def create_vlan(self, dvs, vlan): #dvs.runcmd("ip link del Bridge") @@ -239,11 +264,7 @@ def remove_fdb(self, vlan, mac): tbl._del("Vlan" + vlan + ":" + mac) time.sleep(1) - - # Ignore testcase in Debian Jessie - # TODO: Remove this skip if Jessie support is no longer needed - @pytest.mark.skipif(StrictVersion(distro.linux_distribution()[1]) <= StrictVersion('8.9'), reason="Debian 8.9 or before has no support") - def test_MirrorToVlanAddRemove(self, dvs, testlog): + def _test_MirrorToVlanAddRemove(self, dvs, testlog, v6_encap=False): """ This test covers basic mirror session creation and removal operation with destination port sits in a VLAN @@ -254,15 +275,18 @@ def test_MirrorToVlanAddRemove(self, dvs, testlog): 3. Remove FDB; remove neighbor; remove IP; remove VLAN 4. Remove mirror session """ - self.setup_db(dvs) - session = "TEST_SESSION" + src_ip = "5.5.5.5" if v6_encap == False else "fc00::5:5:5:5" + # dst ip in directly connected vlan subnet + dst_ip = "6.6.6.6" if v6_encap == False else "fc00::6:6:6:6" + intf_addr = "6.6.6.0/24" if v6_encap == False else "fc00::6:6:6:0/112" marker = dvs.add_log_marker() # create mirror session - self.create_mirror_session(session, "5.5.5.5", "6.6.6.6", "0x6558", "8", "100", "0") + self.create_mirror_session(session, src_ip, dst_ip, "0x6558", "8", "100", "0") assert self.get_mirror_session_state(session)["status"] == "inactive" - self.check_syslog(dvs, marker, "Attached next hop observer .* for destination IP 6.6.6.6", 1) + assert self.get_mirror_session_state(session)["next_hop_ip"] == ("0.0.0.0@" if v6_encap == False else "::@") + self.check_syslog(dvs, marker, "Attached next hop observer .* for destination IP {}".format(dst_ip), 1) # create vlan; create vlan member self.create_vlan(dvs, "6") @@ -273,11 +297,11 @@ def test_MirrorToVlanAddRemove(self, dvs, testlog): self.set_interface_status(dvs, "Ethernet4", "up") # add ip address to vlan 6 - self.add_ip_address("Vlan6", "6.6.6.0/24") + self.add_ip_address("Vlan6", intf_addr) assert self.get_mirror_session_state(session)["status"] == "inactive" # create neighbor to vlan 6 - self.add_neighbor("Vlan6", "6.6.6.6", "66:66:66:66:66:66") + self.add_neighbor("Vlan6", dst_ip, "66:66:66:66:66:66") assert self.get_mirror_session_state(session)["status"] == "inactive" # create fdb entry to ethernet4 @@ -300,15 +324,15 @@ def test_MirrorToVlanAddRemove(self, dvs, testlog): elif fv[0] == "SAI_MIRROR_SESSION_ATTR_ERSPAN_ENCAPSULATION_TYPE": assert fv[1] == "SAI_ERSPAN_ENCAPSULATION_TYPE_MIRROR_L3_GRE_TUNNEL" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": - assert fv[1] == "4" + assert fv[1] == "4" if v6_encap == False else "6" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_TOS": assert fv[1] == "32" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_TTL": assert fv[1] == "100" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_SRC_IP_ADDRESS": - assert fv[1] == "5.5.5.5" + assert fv[1] == src_ip elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_IP_ADDRESS": - assert fv[1] == "6.6.6.6" + assert fv[1] == dst_ip elif fv[0] == "SAI_MIRROR_SESSION_ATTR_SRC_MAC_ADDRESS": assert fv[1] == dvs.runcmd("bash -c \"ip link show eth0 | grep ether | awk '{print $2}'\"")[1].strip().upper() elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": @@ -333,11 +357,11 @@ def test_MirrorToVlanAddRemove(self, dvs, testlog): assert self.get_mirror_session_state(session)["status"] == "inactive" # remove neighbor - self.remove_neighbor("Vlan6", "6.6.6.6") + self.remove_neighbor("Vlan6", dst_ip) assert self.get_mirror_session_state(session)["status"] == "inactive" # remove ip address - self.remove_ip_address("Vlan6", "6.6.6.0/24") + self.remove_ip_address("Vlan6", intf_addr) assert self.get_mirror_session_state(session)["status"] == "inactive" # bring down vlan and member @@ -351,7 +375,16 @@ def test_MirrorToVlanAddRemove(self, dvs, testlog): marker = dvs.add_log_marker() # remove mirror session self.remove_mirror_session(session) - self.check_syslog(dvs, marker, "Detached next hop observer for destination IP 6.6.6.6", 1) + self.check_syslog(dvs, marker, "Detached next hop observer for destination IP {}".format(dst_ip), 1) + + # Ignore testcase in Debian Jessie + # TODO: Remove this skip if Jessie support is no longer needed + @pytest.mark.skipif(StrictVersion(distro.linux_distribution()[1]) <= StrictVersion('8.9'), reason="Debian 8.9 or before has no support") + def test_MirrorToVlanAddRemove(self, dvs, testlog): + self.setup_db(dvs) + + self._test_MirrorToVlanAddRemove(dvs, testlog) + self._test_MirrorToVlanAddRemove(dvs, testlog, v6_encap=True) def create_port_channel(self, dvs, channel): tbl = swsscommon.ProducerStateTable(self.pdb, "LAG_TABLE") @@ -382,8 +415,7 @@ def remove_port_channel_member(self, channel, interface): tbl._del("PortChannel" + channel + ":" + interface) time.sleep(1) - - def test_MirrorToLagAddRemove(self, dvs, testlog): + def _test_MirrorToLagAddRemove(self, dvs, testlog, v6_encap=False): """ This test covers basic mirror session creation and removal operations with destination port sits in a LAG @@ -395,15 +427,18 @@ def test_MirrorToLagAddRemove(self, dvs, testlog): 4. Remove mirror session """ - self.setup_db(dvs) - session = "TEST_SESSION" + src_ip = "10.10.10.10" if v6_encap == False else "fc00::10:10:10:10" + dst_ip = "11.11.11.11" if v6_encap == False else "fc00::11:11:11:11" + # dst ip in directly connected subnet + intf_addr = "11.11.11.0/24" if v6_encap == False else "fc00::11:11:11:0/112" marker = dvs.add_log_marker() # create mirror session - self.create_mirror_session(session, "10.10.10.10", "11.11.11.11", "0x6558", "8", "100", "0") + self.create_mirror_session(session, src_ip, dst_ip, "0x6558", "8", "100", "0") assert self.get_mirror_session_state(session)["status"] == "inactive" - self.check_syslog(dvs, marker, "Attached next hop observer .* for destination IP 11.11.11.11", 1) + assert self.get_mirror_session_state(session)["next_hop_ip"] == ("0.0.0.0@" if v6_encap == False else "::@") + self.check_syslog(dvs, marker, "Attached next hop observer .* for destination IP {}".format(dst_ip), 1) # create port channel; create port channel member self.create_port_channel(dvs, "008") @@ -414,11 +449,11 @@ def test_MirrorToLagAddRemove(self, dvs, testlog): self.set_interface_status(dvs, "Ethernet88", "up") # add ip address to port channel 008 - self.add_ip_address("PortChannel008", "11.11.11.0/24") + self.add_ip_address("PortChannel008", intf_addr) assert self.get_mirror_session_state(session)["status"] == "inactive" # create neighbor to port channel 008 - self.add_neighbor("PortChannel008", "11.11.11.11", "88:88:88:88:88:88") + self.add_neighbor("PortChannel008", dst_ip, "88:88:88:88:88:88") assert self.get_mirror_session_state(session)["status"] == "active" # check asic database @@ -432,13 +467,15 @@ def test_MirrorToLagAddRemove(self, dvs, testlog): assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet88" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": assert fv[1] == "88:88:88:88:88:88" + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": + assert fv[1] == "4" if v6_encap == False else "6" # remove neighbor - self.remove_neighbor("PortChannel008", "11.11.11.11") + self.remove_neighbor("PortChannel008", dst_ip) assert self.get_mirror_session_state(session)["status"] == "inactive" # remove ip address - self.remove_ip_address("PortChannel008", "11.11.11.0/24") + self.remove_ip_address("PortChannel008", intf_addr) assert self.get_mirror_session_state(session)["status"] == "inactive" # bring down port channel and port channel member @@ -452,13 +489,15 @@ def test_MirrorToLagAddRemove(self, dvs, testlog): marker = dvs.add_log_marker() # remove mirror session self.remove_mirror_session(session) - self.check_syslog(dvs, marker, "Detached next hop observer for destination IP 11.11.11.11", 1) + self.check_syslog(dvs, marker, "Detached next hop observer for destination IP {}".format(dst_ip), 1) + def test_MirrorToLagAddRemove(self, dvs, testlog): + self.setup_db(dvs) - # Ignore testcase in Debian Jessie - # TODO: Remove this skip if Jessie support is no longer needed - @pytest.mark.skipif(StrictVersion(distro.linux_distribution()[1]) <= StrictVersion('8.9'), reason="Debian 8.9 or before has no support") - def test_MirrorDestMoveVlan(self, dvs, testlog): + self._test_MirrorToLagAddRemove(dvs, testlog) + self._test_MirrorToLagAddRemove(dvs, testlog, v6_encap=True) + + def _test_MirrorDestMoveVlan(self, dvs, testlog, v6_encap=False): """ This test tests mirror session destination move from non-VLAN to VLAN and back to non-VLAN port @@ -471,19 +510,25 @@ def test_MirrorDestMoveVlan(self, dvs, testlog): 7. Disable non-VLAN monitor port 8. Remove mirror session """ - self.setup_db(dvs) - session = "TEST_SESSION" + src_ip = "7.7.7.7" if v6_encap == False else "fc00::7:7:7:7" + dst_ip = "8.8.8.8" if v6_encap == False else "fc00::8:8:8:8" + port_intf_addr = "80.0.0.0/31" if v6_encap == False else "fc00::80:0:0:0/126" + port_nhop_ip = "80.0.0.1" if v6_encap == False else "fc00::80:0:0:1" + port_ip_prefix = "8.8.0.0/16" if v6_encap == False else "fc00::8:8:0:0/96" + # dst ip moves to directly connected vlan subnet + vlan_intf_addr = "8.8.8.0/24" if v6_encap == False else "fc00::8:8:8:0/112" # create mirror session - self.create_mirror_session(session, "7.7.7.7", "8.8.8.8", "0x6558", "8", "100", "0") + self.create_mirror_session(session, src_ip, dst_ip, "0x6558", "8", "100", "0") assert self.get_mirror_session_state(session)["status"] == "inactive" + assert self.get_mirror_session_state(session)["next_hop_ip"] == ("0.0.0.0@" if v6_encap == False else "::@") # bring up port; add ip; add neighbor; add route self.set_interface_status(dvs, "Ethernet32", "up") - self.add_ip_address("Ethernet32", "80.0.0.0/31") - self.add_neighbor("Ethernet32", "80.0.0.1", "02:04:06:08:10:12") - self.add_route(dvs, "8.8.0.0/16", "80.0.0.1") + self.add_ip_address("Ethernet32", port_intf_addr) + self.add_neighbor("Ethernet32", port_nhop_ip, "02:04:06:08:10:12") + self.add_route(dvs, port_ip_prefix, port_nhop_ip) assert self.get_mirror_session_state(session)["status"] == "active" # check monitor port @@ -496,6 +541,8 @@ def test_MirrorDestMoveVlan(self, dvs, testlog): assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet32" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_VLAN_HEADER_VALID": assert fv[1] == "false" + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": + assert fv[1] == "4" if v6_encap == False else "6" # mirror session move round 1 # create vlan; create vlan member; bring up vlan and member @@ -506,11 +553,13 @@ def test_MirrorDestMoveVlan(self, dvs, testlog): assert self.get_mirror_session_state(session)["status"] == "active" # add ip address to vlan 9 - self.add_ip_address("Vlan9", "8.8.8.0/24") + self.add_ip_address("Vlan9", vlan_intf_addr) + time.sleep(2) + # inactive due to no neighbor mac or fdb entry assert self.get_mirror_session_state(session)["status"] == "inactive" # create neighbor to vlan 9 - self.add_neighbor("Vlan9", "8.8.8.8", "88:88:88:88:88:88") + self.add_neighbor("Vlan9", dst_ip, "88:88:88:88:88:88") assert self.get_mirror_session_state(session)["status"] == "inactive" # create fdb entry to ethernet48 @@ -535,6 +584,8 @@ def test_MirrorDestMoveVlan(self, dvs, testlog): assert fv[1] == "0" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_VLAN_CFI": assert fv[1] == "0" + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": + assert fv[1] == "4" if v6_encap == False else "6" # mirror session move round 2 # remove fdb entry @@ -542,11 +593,11 @@ def test_MirrorDestMoveVlan(self, dvs, testlog): assert self.get_mirror_session_state(session)["status"] == "inactive" # remove neighbor - self.remove_neighbor("Vlan9", "8.8.8.8") + self.remove_neighbor("Vlan9", dst_ip) assert self.get_mirror_session_state(session)["status"] == "inactive" # remove ip address - self.remove_ip_address("Vlan9", "8.8.8.0/24") + self.remove_ip_address("Vlan9", vlan_intf_addr) assert self.get_mirror_session_state(session)["status"] == "active" # check monitor port @@ -559,6 +610,8 @@ def test_MirrorDestMoveVlan(self, dvs, testlog): assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet32" elif fv[0] == "SAI_MIRROR_SESSION_ATTR_VLAN_HEADER_VALID": assert fv[1] == "false" + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": + assert fv[1] == "4" if v6_encap == False else "6" # bring down vlan and member; remove vlan member; remove vlan self.set_interface_status(dvs, "Ethernet48", "down") @@ -567,16 +620,24 @@ def test_MirrorDestMoveVlan(self, dvs, testlog): self.remove_vlan("9") # remove route; remove neighbor; remove ip; bring down port - self.remove_route(dvs, "8.8.8.0/24") - self.remove_neighbor("Ethernet32", "80.0.0.1") - self.remove_ip_address("Ethernet32", "80.0.0.0/31") + self.remove_route(dvs, vlan_intf_addr) + self.remove_neighbor("Ethernet32", port_nhop_ip) + self.remove_ip_address("Ethernet32", port_intf_addr) self.set_interface_status(dvs, "Ethernet32", "down") # remove mirror session self.remove_mirror_session(session) + # Ignore testcase in Debian Jessie + # TODO: Remove this skip if Jessie support is no longer needed + @pytest.mark.skipif(StrictVersion(distro.linux_distribution()[1]) <= StrictVersion('8.9'), reason="Debian 8.9 or before has no support") + def test_MirrorDestMoveVlan(self, dvs, testlog): + self.setup_db(dvs) + + self._test_MirrorDestMoveVlan(dvs, testlog) + self._test_MirrorDestMoveVlan(dvs, testlog, v6_encap=True) - def test_MirrorDestMoveLag(self, dvs, testlog): + def _test_MirrorDestMoveLag(self, dvs, testlog, v6_encap=False): """ This test tests mirror session destination move from non-LAG to LAG and back to non-LAG port @@ -589,19 +650,26 @@ def test_MirrorDestMoveLag(self, dvs, testlog): 7. Disable non-LAG monitor port 8. Remove mirror session """ - self.setup_db(dvs) - session = "TEST_SESSION" + src_ip = "12.12.12.12" if v6_encap == False else "fc00::12:12:12:12" + dst_ip = "13.13.13.13" if v6_encap == False else "fc00::13:13:13:13" + port_intf_addr = "100.0.0.0/31" if v6_encap == False else "fc00::100:0:0:0/126" + port_nhop_ip = "100.0.0.1" if v6_encap == False else "fc00::100:0:0:1" + port_ip_prefix = "13.13.0.0/16" if v6_encap == False else "fc00::13:13:0:0/96" + lag_intf_addr = "200.0.0.0/31" if v6_encap == False else "fc00::200:0:0:0/126" + lag_nhop_ip = "200.0.0.1" if v6_encap == False else "fc00::200:0:0:1" + lag_ip_prefix = "13.13.13.0/24" if v6_encap == False else "fc00::13:13:13:0/112" # create mirror session - self.create_mirror_session(session, "12.12.12.12", "13.13.13.13", "0x6558", "8", "100", "0") + self.create_mirror_session(session, src_ip, dst_ip, "0x6558", "8", "100", "0") assert self.get_mirror_session_state(session)["status"] == "inactive" + assert self.get_mirror_session_state(session)["next_hop_ip"] == ("0.0.0.0@" if v6_encap == False else "::@") # bring up port; add ip; add neighbor; add route self.set_interface_status(dvs, "Ethernet64", "up") - self.add_ip_address("Ethernet64", "100.0.0.0/31") - self.add_neighbor("Ethernet64", "100.0.0.1", "02:04:06:08:10:12") - self.add_route(dvs, "13.13.0.0/16", "100.0.0.1") + self.add_ip_address("Ethernet64", port_intf_addr) + self.add_neighbor("Ethernet64", port_nhop_ip, "02:04:06:08:10:12") + self.add_route(dvs, port_ip_prefix, port_nhop_ip) assert self.get_mirror_session_state(session)["status"] == "active" # check monitor port @@ -612,8 +680,10 @@ def test_MirrorDestMoveLag(self, dvs, testlog): for fv in fvs: if fv[0] == "SAI_MIRROR_SESSION_ATTR_MONITOR_PORT": assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet64" - if fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": assert fv[1] == "02:04:06:08:10:12" + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": + assert fv[1] == "4" if v6_encap == False else "6" # mirror session move round 1 # create port channel; create port channel member; bring up @@ -623,12 +693,12 @@ def test_MirrorDestMoveLag(self, dvs, testlog): self.set_interface_status(dvs, "Ethernet32", "up") # add ip address to port channel 080; create neighbor to port channel 080 - self.add_ip_address("PortChannel080", "200.0.0.0/31") - self.add_neighbor("PortChannel080", "200.0.0.1", "12:10:08:06:04:02") + self.add_ip_address("PortChannel080", lag_intf_addr) + self.add_neighbor("PortChannel080", lag_nhop_ip, "12:10:08:06:04:02") assert self.get_mirror_session_state(session)["status"] == "active" # add route - self.add_route(dvs, "13.13.13.0/24", "200.0.0.1") + self.add_route(dvs, lag_ip_prefix, lag_nhop_ip) assert self.get_mirror_session_state(session)["status"] == "active" # check monitor port @@ -639,8 +709,10 @@ def test_MirrorDestMoveLag(self, dvs, testlog): for fv in fvs: if fv[0] == "SAI_MIRROR_SESSION_ATTR_MONITOR_PORT": assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet32" - if fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": assert fv[1] == "12:10:08:06:04:02" + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": + assert fv[1] == "4" if v6_encap == False else "6" # mirror session move round 2 # remove port channel member @@ -660,15 +732,16 @@ def test_MirrorDestMoveLag(self, dvs, testlog): for fv in fvs: if fv[0] == "SAI_MIRROR_SESSION_ATTR_MONITOR_PORT": assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet32" - if fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": assert fv[1] == "12:10:08:06:04:02" + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": + assert fv[1] == "4" if v6_encap == False else "6" # mirror session move round 4 # remove route - self.remove_route(dvs, "13.13.13.0/24") + self.remove_route(dvs, lag_ip_prefix) assert self.get_mirror_session_state(session)["status"] == "active" - port_oid = "" # check monitor port tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_MIRROR_SESSION") assert len(tbl.getKeys()) == 1 @@ -677,12 +750,14 @@ def test_MirrorDestMoveLag(self, dvs, testlog): for fv in fvs: if fv[0] == "SAI_MIRROR_SESSION_ATTR_MONITOR_PORT": assert dvs.asicdb.portoidmap[fv[1]] == "Ethernet64" - if fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_DST_MAC_ADDRESS": assert fv[1] == "02:04:06:08:10:12" + elif fv[0] == "SAI_MIRROR_SESSION_ATTR_IPHDR_VERSION": + assert fv[1] == "4" if v6_encap == False else "6" # remove neighbor; remove ip address to port channel 080 - self.remove_neighbor("PortChannel080", "200.0.0.1") - self.remove_ip_address("PortChannel080", "200.0.0.0/31") + self.remove_neighbor("PortChannel080", lag_nhop_ip) + self.remove_ip_address("PortChannel080", lag_intf_addr) # bring down; remove port channel member; remove port channel self.set_interface_status(dvs, "Ethernet32", "down") @@ -692,15 +767,20 @@ def test_MirrorDestMoveLag(self, dvs, testlog): assert self.get_mirror_session_state(session)["status"] == "active" # remove route; remove neighbor; remove ip; bring down port - self.remove_route(dvs, "13.13.0.0/16") - self.remove_neighbor("Ethernet64", "100.0.0.1") - self.remove_ip_address("Ethernet64", "100.0.0.0/31") + self.remove_route(dvs, port_ip_prefix) + self.remove_neighbor("Ethernet64", port_nhop_ip) + self.remove_ip_address("Ethernet64", port_intf_addr) self.set_interface_status(dvs, "Ethernet64", "down") assert self.get_mirror_session_state(session)["status"] == "inactive" # remove mirror session self.remove_mirror_session(session) + def test_MirrorDestMoveLag(self, dvs, testlog): + self.setup_db(dvs) + + self._test_MirrorDestMoveLag(dvs, testlog) + self._test_MirrorDestMoveLag(dvs, testlog, v6_encap=True) def create_acl_table(self, table, interfaces): tbl = swsscommon.Table(self.cdb, "ACL_TABLE") From 0b331f0db4b842ff9b857e462e52b3a17ab65802 Mon Sep 17 00:00:00 2001 From: krismarvell <108510436+krismarvell@users.noreply.github.com> Date: Tue, 19 Nov 2024 22:55:47 +0530 Subject: [PATCH 2/8] platform/innovium renaming to platform/marvell-teralynx (#3252) What I did asic type checks for innovium platforms renamed innovium to marvell-teralynx Why I did it asictype is being renamed in sonic-buildimage repo and hence respective platform checks needs changes in swss --- orchagent/Makefile.am | 2 +- orchagent/aclorch.cpp | 4 ++-- orchagent/copporch.cpp | 4 ++-- orchagent/orch.h | 2 +- orchagent/orchdaemon.cpp | 8 +++++--- ...etect_innovium.lua => pfc_detect_marvell_teralynx.lua} | 0 6 files changed, 11 insertions(+), 9 deletions(-) rename orchagent/{pfc_detect_innovium.lua => pfc_detect_marvell_teralynx.lua} (100%) diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 5224751f73b..ae83c16d8ff 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -17,7 +17,7 @@ swssdir = $(datadir)/swss dist_swss_DATA = \ eliminate_events.lua \ rif_rates.lua \ - pfc_detect_innovium.lua \ + pfc_detect_marvell_teralynx.lua \ pfc_detect_mellanox.lua \ pfc_detect_broadcom.lua \ pfc_detect_marvell.lua \ diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index d3536719fba..ef9391ae42d 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -3128,7 +3128,7 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr platform == MLNX_PLATFORM_SUBSTRING || platform == BFN_PLATFORM_SUBSTRING || platform == MRVL_PLATFORM_SUBSTRING || - platform == INVM_PLATFORM_SUBSTRING || + platform == MRVL_TL_PLATFORM_SUBSTRING || platform == NPS_PLATFORM_SUBSTRING || platform == XS_PLATFORM_SUBSTRING || platform == VS_PLATFORM_SUBSTRING) @@ -3149,7 +3149,7 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr } if ( platform == MRVL_PLATFORM_SUBSTRING || - platform == INVM_PLATFORM_SUBSTRING || + platform == MRVL_TL_PLATFORM_SUBSTRING || platform == VS_PLATFORM_SUBSTRING) { m_L3V4V6Capability = diff --git a/orchagent/copporch.cpp b/orchagent/copporch.cpp index 749a8aae7ef..634a8a8ed61 100644 --- a/orchagent/copporch.cpp +++ b/orchagent/copporch.cpp @@ -1014,8 +1014,8 @@ bool CoppOrch::getAttribsFromTrapGroup (vector &fv_tuple, { /* Mellanox platform doesn't support trap priority setting */ /* Marvell platform doesn't support trap priority. */ - char *platform = getenv("platform"); - if (!platform || (!strstr(platform, MLNX_PLATFORM_SUBSTRING) && (!strstr(platform, MRVL_PLATFORM_SUBSTRING)))) + char *platform = getenv("platform"); + if (!platform || (!strstr(platform, MLNX_PLATFORM_SUBSTRING) && (!strstr(platform, MRVL_PLATFORM_SUBSTRING)))) { attr.id = SAI_HOSTIF_TRAP_ATTR_TRAP_PRIORITY, attr.value.u32 = (uint32_t)stoul(fvValue(*i)); diff --git a/orchagent/orch.h b/orchagent/orch.h index bdbecf5f5f8..5e8939c53f2 100644 --- a/orchagent/orch.h +++ b/orchagent/orch.h @@ -34,7 +34,7 @@ const char range_specifier = '-'; const char config_db_key_delimiter = '|'; const char state_db_key_delimiter = '|'; -#define INVM_PLATFORM_SUBSTRING "innovium" +#define MRVL_TL_PLATFORM_SUBSTRING "marvell-teralynx" #define MLNX_PLATFORM_SUBSTRING "mellanox" #define BRCM_PLATFORM_SUBSTRING "broadcom" #define BRCM_DNX_PLATFORM_SUBSTRING "broadcom-dnx" diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 0d2ab1c2000..f6e9428ab67 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -590,9 +590,9 @@ bool OrchDaemon::init() queueAttrIds, PFC_WD_POLL_MSECS)); } - else if ((platform == INVM_PLATFORM_SUBSTRING) + else if ((platform == MRVL_TL_PLATFORM_SUBSTRING) + || (platform == MRVL_PLATFORM_SUBSTRING) || (platform == BFN_PLATFORM_SUBSTRING) - || (platform == MRVL_PLATFORM_SUBSTRING) || (platform == NPS_PLATFORM_SUBSTRING)) { @@ -624,7 +624,9 @@ bool OrchDaemon::init() static const vector queueAttrIds; - if ((platform == INVM_PLATFORM_SUBSTRING) || (platform == NPS_PLATFORM_SUBSTRING) || (platform == MRVL_PLATFORM_SUBSTRING)) + if ((platform == MRVL_PLATFORM_SUBSTRING) || + (platform == MRVL_TL_PLATFORM_SUBSTRING) || + (platform == NPS_PLATFORM_SUBSTRING)) { m_orchList.push_back(new PfcWdSwOrch( m_configDb, diff --git a/orchagent/pfc_detect_innovium.lua b/orchagent/pfc_detect_marvell_teralynx.lua similarity index 100% rename from orchagent/pfc_detect_innovium.lua rename to orchagent/pfc_detect_marvell_teralynx.lua From c86efa18fb7675ea35558f526b89859b2371629e Mon Sep 17 00:00:00 2001 From: siqbal1986 Date: Tue, 19 Nov 2024 17:02:47 -0800 Subject: [PATCH 3/8] Added support for "UNDERLAY_SET_DSCP" and "UNDERLAY_SET_DSCPV6" tables (#3307) What I did This PR adds the logical table UNDERLAY_SET_DSCP and UNDERLAY_SET_DSCPV6. This feature allows SONIC to match an ingress L4 packet and change the DSCP field of the outer header when the packet is egressing. These tables are only created on Cisco-8000, Mlnx and VS platforms. The PR also adds support to check if the platform supports the following SAI attributes. SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META --- orchagent/aclorch.cpp | 855 ++++++++++++++++++++++++++++++++++++++-- orchagent/aclorch.h | 85 +++- orchagent/acltable.h | 5 + tests/dvslib/dvs_acl.py | 24 ++ tests/test_acl_mark.py | 447 +++++++++++++++++++++ 5 files changed, 1388 insertions(+), 28 deletions(-) create mode 100644 tests/test_acl_mark.py diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index ef9391ae42d..8a4ebbecc77 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -34,6 +34,7 @@ extern string gMySwitchType; #define MIN_VLAN_ID 1 // 0 is a reserved VLAN ID #define MAX_VLAN_ID 4095 // 4096 is a reserved VLAN ID + #define STATE_DB_ACL_ACTION_FIELD_IS_ACTION_LIST_MANDATORY "is_action_list_mandatory" #define STATE_DB_ACL_ACTION_FIELD_ACTION_LIST "action_list" #define STATE_DB_ACL_L3V4V6_SUPPORTED "supported_L3V4V6" @@ -42,6 +43,10 @@ extern string gMySwitchType; #define ACL_COUNTER_DEFAULT_POLLING_INTERVAL_MS 10000 // ms #define ACL_COUNTER_DEFAULT_ENABLED_STATE false + +#define EGR_SET_DSCP_TABLE_ID "EgressSetDSCP" +#define MAX_META_DATA_VALUE 4095 + const int TCP_PROTOCOL_NUM = 6; // TCP protocol number acl_rule_attr_lookup_t aclMatchLookup = @@ -76,7 +81,8 @@ acl_rule_attr_lookup_t aclMatchLookup = { MATCH_INNER_L4_DST_PORT, SAI_ACL_ENTRY_ATTR_FIELD_INNER_L4_DST_PORT }, { MATCH_BTH_OPCODE, SAI_ACL_ENTRY_ATTR_FIELD_BTH_OPCODE}, { MATCH_AETH_SYNDROME, SAI_ACL_ENTRY_ATTR_FIELD_AETH_SYNDROME}, - { MATCH_TUNNEL_TERM, SAI_ACL_ENTRY_ATTR_FIELD_TUNNEL_TERMINATED} + { MATCH_TUNNEL_TERM, SAI_ACL_ENTRY_ATTR_FIELD_TUNNEL_TERMINATED}, + { MATCH_METADATA, SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META} }; static acl_range_type_lookup_t aclRangeTypeLookup = @@ -126,6 +132,12 @@ static acl_packet_action_lookup_t aclPacketActionLookup = { PACKET_ACTION_COPY, SAI_PACKET_ACTION_COPY }, }; +static acl_rule_attr_lookup_t aclMetadataDscpActionLookup = +{ + { ACTION_META_DATA, SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA}, + { ACTION_DSCP, SAI_ACL_ENTRY_ATTR_ACTION_SET_DSCP} +}; + static acl_dtel_flow_op_type_lookup_t aclDTelFlowOpTypeLookup = { { DTEL_FLOW_OP_NOP, SAI_ACL_DTEL_FLOW_OP_NOP }, @@ -353,6 +365,42 @@ static acl_table_action_list_lookup_t defaultAclActionList = } } } + }, + { + // MARK_META + TABLE_TYPE_MARK_META, + { + { + ACL_STAGE_INGRESS, + { + SAI_ACL_ACTION_TYPE_SET_ACL_META_DATA + } + } + } + }, + { + // MARK_METAV6 + TABLE_TYPE_MARK_META_V6, + { + { + ACL_STAGE_INGRESS, + { + SAI_ACL_ACTION_TYPE_SET_ACL_META_DATA + } + } + } + }, + { + // EGR_SET_DSCP + TABLE_TYPE_EGR_SET_DSCP, + { + { + ACL_STAGE_EGRESS, + { + SAI_ACL_ACTION_TYPE_SET_DSCP + } + } + } } }; @@ -418,6 +466,18 @@ static acl_table_match_field_lookup_t stageMandatoryMatchFields = } } } + }, + { + // EGR_SET_DSCP + TABLE_TYPE_EGR_SET_DSCP, + { + { + ACL_STAGE_EGRESS, + { + SAI_ACL_TABLE_ATTR_FIELD_ACL_USER_META + } + } + } } }; @@ -710,7 +770,7 @@ bool AclTableTypeParser::parseAclTableTypeActions(const std::string& value, AclT auto mirrorAction = aclMirrorStageLookup.find(action); auto dtelAction = aclDTelActionLookup.find(action); auto otherAction = aclOtherActionLookup.find(action); - + auto metadataAction = aclMetadataDscpActionLookup.find(action); if (l3Action != aclL3ActionLookup.end()) { saiActionAttr = l3Action->second; @@ -727,6 +787,10 @@ bool AclTableTypeParser::parseAclTableTypeActions(const std::string& value, AclT { saiActionAttr = otherAction->second; } + else if (metadataAction != aclMetadataDscpActionLookup.end()) + { + saiActionAttr = metadataAction->second; + } else { SWSS_LOG_ERROR("Unknown action %s", action.c_str()); @@ -1057,6 +1121,18 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) return false; } } + else if (attr_name == MATCH_METADATA) + { + matchData.data.u32 = to_uint(attr_value); + matchData.mask.u32 = 0xFFFFFFFF; + + if (matchData.data.u32 < m_pAclOrch->getAclMetaDataMin() || matchData.data.u32 > m_pAclOrch->getAclMetaDataMax()) + { + SWSS_LOG_ERROR("Invalid MATCH_METADATA configuration: %s, expected value between %d - %d", attr_value.c_str(), + m_pAclOrch->getAclMetaDataMin(), m_pAclOrch->getAclMetaDataMax()); + return false; + } + } } catch (exception &e) { @@ -1624,7 +1700,7 @@ bool AclRule::getCreateCounter() const return m_createCounter; } -shared_ptr AclRule::makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple& data) +shared_ptr AclRule::makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple& data, MetaDataMgr * m_metadataMgr) { shared_ptr aclRule; @@ -1640,6 +1716,10 @@ shared_ptr AclRule::makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOr { return make_shared(acl, rule, table); } + else if (acl->isUsingEgrSetDscp(table) || table == EGR_SET_DSCP_TABLE_ID) + { + return make_shared(acl, rule, table, m_metadataMgr); + } else if (aclDTelActionLookup.find(action) != aclDTelActionLookup.cend()) { if (!dtel) @@ -2188,6 +2268,105 @@ void AclRuleMirror::onUpdate(SubjectType type, void *cntx) } } +AclRuleUnderlaySetDscp::AclRuleUnderlaySetDscp(AclOrch *aclOrch, string rule, string table, MetaDataMgr* m_metaDataMgr, bool createCounter): + AclRule(aclOrch, rule, table, createCounter), + table_id(table), + m_metaDataMgr(m_metaDataMgr) +{ +} + +uint32_t AclRuleUnderlaySetDscp::getDscpValue() const +{ + return cachedDscpValue; +} + +uint32_t AclRuleUnderlaySetDscp::getMetadata() const +{ + return cachedMetadata; +} + +bool AclRuleUnderlaySetDscp::validateAddAction(string attr_name, string _attr_value) +{ + SWSS_LOG_ENTER(); + + string attr_value = to_upper(_attr_value); + + sai_object_id_t table_oid = m_pAclOrch->getTableById(table_id); + auto aclTable = m_pAclOrch->getTableByOid(table_oid); + string type = aclTable->type.getName(); + string key = table_id + ":" + m_id; + // we handle the allocation of metadata for here. based on SET_DSCP action, we check if a metadata is already allocated then we reuse it + // otherwise we allocate a new metadata. This metadata is then set an the action for the Rule of this table. We also cache the SET_DSCP + // value and the allocated metadata in a the rule structure itself so that when we go to addRule we can use these to add the + // egr_set_dscp rule + if (attr_name == ACTION_DSCP && (type == TABLE_TYPE_MARK_META || type == TABLE_TYPE_MARK_META_V6)) + { + if (!m_pAclOrch->isUsingEgrSetDscp(table_id)) + { + + SWSS_LOG_ERROR("Unexpected Error. Table %s not asssociated with EGR_SET_DSCP table", table_id.c_str()); + return false; + } + if (m_pAclOrch->hasMetaDataRefCount(key) > 0) + { + SWSS_LOG_ERROR("Metadata already allocated for Rule %s in table %s. Remove and Re-add the rule.", m_id.c_str(), table_id.c_str()); + return false; + } + u_int8_t actionDscpValue = uint8_t(std::stoi(attr_value)); + cachedDscpValue = actionDscpValue; + auto metadata = m_metaDataMgr->getFreeMetaData(actionDscpValue); + + if (!m_metaDataMgr->isValidMetaData(metadata)) + { + SWSS_LOG_ERROR("Failed to get free metadata for DSCP value %d", actionDscpValue); + return false; + } + SWSS_LOG_ERROR("Allocated metadata %d for DSCP value %d", metadata, actionDscpValue); + cachedMetadata = metadata; + attr_name = ACTION_META_DATA; + attr_value = std::to_string(metadata); + m_pAclOrch->addMetaDataRef(key, metadata); + } + + + sai_acl_action_data_t actionData; + actionData.parameter.u32 = 0; + + SWSS_LOG_INFO("attr_name: %s, attr_value: %s int val %d", attr_name.c_str(), attr_value.c_str(), to_uint(attr_value)); + // we only handle DSCP and META_DATA actions for now. + if (attr_name == ACTION_DSCP || attr_name == ACTION_META_DATA) + { + actionData.parameter.u32 = to_uint(attr_value); + if (attr_name == ACTION_META_DATA && (actionData.parameter.u32 < m_pAclOrch->getAclMetaDataMin() || actionData.parameter.u32 > m_pAclOrch->getAclMetaDataMax())) + { + return false; + } + } + else + { + return false; + } + + actionData.enable = true; + return setAction(aclMetadataDscpActionLookup[attr_name], actionData); +} + +bool AclRuleUnderlaySetDscp::validate() +{ + SWSS_LOG_ENTER(); + if ( m_actions.size() != 1) + { + return false; + } + + return true; +} + +void AclRuleUnderlaySetDscp::onUpdate(SubjectType, void *) +{ + // Do nothing +} + AclTable::AclTable(AclOrch *pAclOrch, string id) noexcept : m_pAclOrch(pAclOrch), id(id) { @@ -3195,6 +3374,108 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr } + if (platform == VS_PLATFORM_SUBSTRING) + { + // For testing on VS the following values will be used. + m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_RANGE_CAPABLE] = "true"; + m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MIN] = "1"; + m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MAX] = "7"; + m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ATTR_META_CAPABLE] = "true"; + m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ACTION_META_CAPABLE] = "true"; + m_metaDataMgr.populateRange(1,7); + } + else + { + // check switch capability of Metadata attribute, action and range. + // SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE support and range values. + // SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA + // SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META + + sai_status_t status = SAI_STATUS_SUCCESS; + sai_attr_capability_t capability; + uint16_t metadataMin = 0; + uint16_t metadataMax = 0; + m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MIN] = "0"; + m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MAX] = "0"; + m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_RANGE_CAPABLE] = "false"; + m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ATTR_META_CAPABLE] = "false"; + m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ACTION_META_CAPABLE] = "false"; + + status = sai_query_attribute_capability(gSwitchId, SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE, &capability); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("Could not query ACL_USER_META_DATA_RANGE %d", status); + } + else + { + if (capability.get_implemented) + { + sai_attribute_t attrs[1]; + attrs[0].id = SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE; + sai_status_t status = sai_switch_api->get_switch_attribute(gSwitchId, 1, attrs); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("Could not get range for ACL_USER_META_DATA_RANGE %d", status); + } + else + { + SWSS_LOG_NOTICE("ACL_USER_META_DATA_RANGE, min: %u, max: %u", attrs[0].value.u32range.min, attrs[0].value.u32range.max); + m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_RANGE_CAPABLE] = "true"; + if (attrs[0].value.u32range.min > MAX_META_DATA_VALUE) + { + SWSS_LOG_ERROR("Unsupported ACL_USER_META_DATA_RANGE min value"); + metadataMin = 0; + } + else + { + metadataMin = uint16_t(attrs[0].value.u32range.min); + } + if (attrs[0].value.u32range.max > MAX_META_DATA_VALUE) + { + metadataMax = MAX_META_DATA_VALUE; + } + else + { + metadataMax = uint16_t(attrs[0].value.u32range.max); + } + m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MIN] = std::to_string(metadataMin); + m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MAX] = std::to_string(metadataMax); + } + + } + SWSS_LOG_NOTICE("ACL_USER_META_DATA_RANGE capability %d", capability.get_implemented); + } + status = sai_query_attribute_capability(gSwitchId, SAI_OBJECT_TYPE_ACL_ENTRY, SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META, &capability); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("Could not query ACL_ENTRY_ATTR_FIELD_ACL_USER_META %d", status); + } + else + { + if (capability.set_implemented) + { + m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ATTR_META_CAPABLE] = "true"; + } + SWSS_LOG_NOTICE("ACL_ENTRY_ATTR_FIELD_ACL_USER_META capability %d", capability.set_implemented); + } + + status = sai_query_attribute_capability(gSwitchId, SAI_OBJECT_TYPE_ACL_ENTRY, SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA, &capability); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_WARN("Could not query ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA %d", status); + } + else + { + if (capability.set_implemented) + { + m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ACTION_META_CAPABLE] = "true"; + } + SWSS_LOG_NOTICE("ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA capability %d", capability.set_implemented); + } + + m_metaDataMgr.populateRange(metadataMin, metadataMax); + + } // Store the capabilities in state database // TODO: Move this part of the code into syncd vector fvVector; @@ -3463,7 +3744,44 @@ void AclOrch::initDefaultTableTypes(const string& platform, const string& sub_pl .build() ); } + if (isAclMetaDataSupported()) + { + addAclTableType( + builder.withName(TABLE_TYPE_MARK_META) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DSCP)) + .build() + ); + + addAclTableType( + builder.withName(TABLE_TYPE_MARK_META_V6) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_DSCP)) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS)) + .build() + ); + addAclTableType( + builder.withName(TABLE_TYPE_EGR_SET_DSCP) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_ACL_USER_META)) + .build() + ); + } // Placeholder for control plane tables addAclTableType(builder.withName(TABLE_TYPE_CTRLPLANE).build()); } @@ -3561,8 +3879,12 @@ void AclOrch::putAclActionCapabilityInDB(acl_stage_type_t stage) string delimiter; ostringstream acl_action_value_stream; ostringstream is_action_list_mandatory_stream; - - for (const auto& action_map: {aclL3ActionLookup, aclMirrorStageLookup, aclDTelActionLookup}) + acl_rule_attr_lookup_t metadataActionLookup = {}; + if (isAclMetaDataSupported()) + { + metadataActionLookup = aclMetadataDscpActionLookup; + } + for (const auto& action_map: {aclL3ActionLookup, aclMirrorStageLookup, aclDTelActionLookup, metadataActionLookup}) { for (const auto& it: action_map) { @@ -3802,6 +4124,27 @@ void AclOrch::getAddDeletePorts(AclTable &newT, newPortSet.insert(p); } + // if the table type is TABLE_TYPE_EGR_SET_DSCP we use a single instance of this + // table with all the tables of type TABLE_TYPE_MARK_META/v6 therefoere we need to + // to collect all the ports from the tables of type TABLE_TYPE_MARK_META/v6 and + // put them in the newPortSet. + if (curT.id == EGR_SET_DSCP_TABLE_ID) + { + for(auto iter : m_egrSetDscpRef) + { + auto tableOid = getTableById(iter); + auto existingtable = m_AclTables.at(tableOid); + for (auto p : existingtable.pendingPortSet) + { + newPortSet.insert(p); + } + for (auto p : existingtable.portSet) + { + newPortSet.insert(p); + } + } + } + // Collect current ports for (auto p : curT.pendingPortSet) { @@ -3897,7 +4240,6 @@ bool AclOrch::updateAclTablePorts(AclTable &newTable, AclTable &curTable) bool AclOrch::updateAclTable(AclTable ¤tTable, AclTable &newTable) { SWSS_LOG_ENTER(); - currentTable.description = newTable.description; if (!updateAclTablePorts(newTable, currentTable)) { @@ -3908,10 +4250,193 @@ bool AclOrch::updateAclTable(AclTable ¤tTable, AclTable &newTable) return true; } -bool AclOrch::updateAclTable(string table_id, AclTable &table) +EgressSetDscpTableStatus AclOrch::addEgrSetDscpTable(string table_id, AclTable &table, string orignalTableTypeName) { SWSS_LOG_ENTER(); + EgressSetDscpTableStatus status = EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_NOT_REQUIRED; + AclTable egrSetDscpTable(this); + // we only add the EGR_SET_DSCP table if the table type is TABLE_TYPE_UNDERLAY_SET_DSCP or TABLE_TYPE_UNDERLAY_SET_DSCPV6 + // otherwise we return EGRESS_SET_DSCP_TABLE_NOT_REQUIRED. + if (orignalTableTypeName == TABLE_TYPE_UNDERLAY_SET_DSCP || orignalTableTypeName == TABLE_TYPE_UNDERLAY_SET_DSCPV6) + { + if (!isAclMetaDataSupported()) + { + SWSS_LOG_ERROR("Platform does not support MARK_META/MARK_METAV6 tables."); + return EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_NOT_SUPPORTED; + } + AclTable egrSetDscpTable(this); + + // copy ports from the TABLE_TYPE_UNDERLAY_SET_DSCP/v6 to the egrSetDscpTable. + std::set ports; + ports.insert(table.portSet.begin(), table.portSet.end()); + ports.insert(table.pendingPortSet.begin(), table.pendingPortSet.end()); + + for (auto alias : ports) + { + Port port; + if (!gPortsOrch->getPort(alias, port)) + { + SWSS_LOG_INFO("Add unready port %s to pending list for ACL table %s", + alias.c_str(), EGR_SET_DSCP_TABLE_ID); + egrSetDscpTable.pendingPortSet.emplace(alias); + continue; + } + + sai_object_id_t bind_port_id; + if (!getAclBindPortId(port, bind_port_id)) + { + SWSS_LOG_ERROR("Failed to get port %s bind port ID for ACL table %s", + alias.c_str(), EGR_SET_DSCP_TABLE_ID); + return EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_FAILED; + } + egrSetDscpTable.link(bind_port_id); + egrSetDscpTable.portSet.emplace(alias); + } + + egrSetDscpTable.id = EGR_SET_DSCP_TABLE_ID; + egrSetDscpTable.stage = ACL_STAGE_EGRESS; + auto egrSetDscpTableType = getAclTableType(TABLE_TYPE_EGR_SET_DSCP); + sai_object_id_t egrSetDscp_oid = getTableById(EGR_SET_DSCP_TABLE_ID); + // create the EGR_SET_DSCP fisrt time if not present. Otherwise update the existing table. + if (m_egrSetDscpRef.empty()) + { + // Create EGR_SET_DSCP table + egrSetDscpTable.validateAddType(*egrSetDscpTableType); + egrSetDscpTable.addMandatoryActions(); + if (!egrSetDscpTable.validate()) + { + SWSS_LOG_ERROR("Failed to validate ACL table %s", + EGR_SET_DSCP_TABLE_ID); + // since we failed to create the table, there is no need for rollback. + return EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_FAILED; + } + if (!addAclTable(egrSetDscpTable)) + { + SWSS_LOG_ERROR("Failed to create ACL table EgressSetDSCP"); + // since we failed to create the table, there is no need for rollback. + return EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_FAILED; + } + } + else + { + if (updateAclTable(m_AclTables[egrSetDscp_oid], egrSetDscpTable)) + { + SWSS_LOG_NOTICE("Successfully updated existing ACL table EgressSetDSCP"); + // We do not set the status here as we still have to update + // TABLE_TYPE_MARK_META/V6 table. + } + else + { + SWSS_LOG_ERROR("Failed to update existing ACL table EgressSetDSCP"); + // there is no need for roollback as we have not made any changes to the MARK_META/V6 tables. + // We can simply return false. + return EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_FAILED; + } + } + // keep track of the fact that this table is now associated with the EGR_SET_DSCP table. + + m_egrSetDscpRef.insert(table_id); + SWSS_LOG_NOTICE("Added ACL table %s to EgrSetDscpRef", table_id.c_str()); + status = EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_SUCCESS; + + } + + return status; +} + +bool AclOrch::removeEgrSetDscpTable(string table_id) +{ + m_egrSetDscpRef.erase(table_id); + SWSS_LOG_INFO("attempting to remove %s reference", table_id.c_str()); + if (m_egrSetDscpRef.size() == 0) + { + if (!removeAclTable(EGR_SET_DSCP_TABLE_ID)) + { + m_egrSetDscpRef.insert(table_id); + SWSS_LOG_ERROR("Failed to remove ACL table %s", EGR_SET_DSCP_TABLE_ID); + return false; + } + } + else + { + //create a dummy table with no ports. The updateAclTable will remove the + // unique ports which were associated with table_id. + // The way this works is as follows. + // The getAddDeletePorts function collects all the ports of the tables which + // are in m_egrSetDscpRef set and adds those ports to the EGR_SET_DSCP. + // As a result the EGR_SET_DSCP is associated with all the ports to which the + // TABLE_TYPE_UNDERLAY_SET_DSCP/V6 tables are attached. + // + // when we want to remove one of the tables referencing the EGR_SET_DSCP. + // we remove it from m_egrSetDscpRef, then send a updateAclTable with a + // EGR_SET_DSCP table with no assiciated ports. + // The getAddDeletePorts collects all the ports except for the one assocated + // with the table we just removed from m_egrSetDscpRef and updated the EGR_SET_DSCP + // with new port set. + AclTable dummyTable(this); + dummyTable.id = EGR_SET_DSCP_TABLE_ID; + dummyTable.stage = ACL_STAGE_EGRESS; + if (!updateAclTable(EGR_SET_DSCP_TABLE_ID, dummyTable, "")) + { + m_egrSetDscpRef.insert(table_id); + SWSS_LOG_ERROR("Failed to remove ACL table %s", EGR_SET_DSCP_TABLE_ID); + return false; + } + SWSS_LOG_NOTICE("Successfully removed the %s table from the reference of %s", table_id.c_str(), EGR_SET_DSCP_TABLE_ID); + } + return true; +} + +bool AclOrch::addEgrSetDscpRule(string key, string dscpAction) +{ + auto metadata = m_egrDscpRuleMetadata[key]; + + if (m_metadataEgrDscpRule[metadata].size() == 1) + { + // Create EGR_SET_DSCP rule. set the match criteria to metadata value and action to dscpAction. + auto egrSetDscpRule = make_shared(this, std::to_string(metadata), EGR_SET_DSCP_TABLE_ID, &m_metaDataMgr); + egrSetDscpRule->validateAddMatch(MATCH_METADATA, std::to_string(metadata)); + egrSetDscpRule->validateAddAction(ACTION_DSCP, dscpAction); + + if (egrSetDscpRule->validate()) + { + if (!addAclRule(egrSetDscpRule, EGR_SET_DSCP_TABLE_ID)) + { + SWSS_LOG_ERROR("Failed to create ACL rule %d in table %s", metadata, EGR_SET_DSCP_TABLE_ID); + return false; + } + } + else + { + SWSS_LOG_ERROR("Failed to validate ACL rule %d in table %s", metadata, EGR_SET_DSCP_TABLE_ID); + return false; + } + } + return true; +} + +bool AclOrch::removeEgrSetDscpRule(string key) +{ + auto metadata = m_egrDscpRuleMetadata[key]; + if (getMetaDataRefCount(metadata) == 1) + { + if(!removeAclRule(EGR_SET_DSCP_TABLE_ID, std::to_string(metadata))) + { + SWSS_LOG_ERROR("Failed to remove ACL rule %s in table %s", key.c_str(), EGR_SET_DSCP_TABLE_ID); + return false; + } + } + removeMetaDataRef(key, metadata); + m_metaDataMgr.recycleMetaData(metadata); + SWSS_LOG_ERROR("Freeing metadata %d for Rule %s", metadata, key.c_str()); + + return true; +} + +bool AclOrch::updateAclTable(string table_id, AclTable &table) +{ + SWSS_LOG_ENTER(); auto tableOid = getTableById(table_id); if (tableOid == SAI_NULL_OBJECT_ID) { @@ -3928,6 +4453,34 @@ bool AclOrch::updateAclTable(string table_id, AclTable &table) return true; } +bool AclOrch::updateAclTable(string table_id, AclTable &table, string orignalTableTypeName) +{ + SWSS_LOG_ENTER(); + + // we call the addEgrSetDscpTable to add the EGR_SET_DSCP table if the table type is TABLE_TYPE_UNDERLAY_SET_DSCP or TABLE_TYPE_UNDERLAY_SET_DSCPV6 + // for other tables it simply retuns EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_NOT_REQUIRED + EgressSetDscpTableStatus egrSetDscpStatus = addEgrSetDscpTable(table_id, table, orignalTableTypeName); + bool status = false; + if (egrSetDscpStatus == EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_FAILED || + egrSetDscpStatus == EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_NOT_SUPPORTED) + { + SWSS_LOG_INFO("Failed to add/update ACL table %s with %s",table_id.c_str(), orignalTableTypeName.c_str()); + return false; + } + + status = updateAclTable(table_id,table); + // if we have not updated the EGR_SET_DSCP, we simply need to return the status. + // otherewise we need to undo the changes we made to the EGR_SET_DSCP if the update + // of the MARK_META table failed. + if (egrSetDscpStatus == EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_SUCCESS && !status) + { + // This is the scenario where we have successfully updated the EGR_SET_DSCP but failed to update the MARK_META table. + SWSS_LOG_ERROR("Reverting changes to EGR_SET_DSCP because update of %s failed", table_id.c_str()); + removeEgrSetDscpTable(table_id); + } + return status; +} + bool AclOrch::addAclTable(AclTable &newTable) { SWSS_LOG_ENTER(); @@ -4057,10 +4610,44 @@ bool AclOrch::addAclTable(AclTable &newTable) } } +bool AclOrch::addAclTable(string table_id, AclTable &newTable, string orignalTableTypeName) +{ + SWSS_LOG_ENTER(); + // we call the addEgrSetDscpTable to add the EGR_SET_DSCP table if the table type is TABLE_TYPE_UNDERLAY_SET_DSCP + // or TABLE_TYPE_UNDERLAY_SET_DSCPV6. For other tables it simply retuns EGRESS_SET_DSCP_TABLE_NOT_REQUIRED. + EgressSetDscpTableStatus egrSetDscpStatus = addEgrSetDscpTable(table_id, newTable, orignalTableTypeName); + bool status = false; + if (egrSetDscpStatus == EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_FAILED || + egrSetDscpStatus == EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_NOT_SUPPORTED) + { + return false; + } + status = addAclTable(newTable); + // if we have not updated the EGR_SET_DSCP, we simply need to return the status. + // otherewise we need to undo the changes we made to the EGR_SET_DSCP if the update + // of the MARK_META table failed. + if (egrSetDscpStatus == EgressSetDscpTableStatus::EGRESS_SET_DSCP_TABLE_SUCCESS && !status) + { + // This is the scenario where we have successfully updated the EGR_SET_DSCP but failed to update the MARK_META table. + SWSS_LOG_ERROR("Reverting changes to EGR_SET_DSCP because update of %s failed", table_id.c_str()); + removeEgrSetDscpTable(table_id); + } + return status; +} + bool AclOrch::removeAclTable(string table_id) { SWSS_LOG_ENTER(); + if (m_egrSetDscpRef.find(table_id) != m_egrSetDscpRef.end()) + { + if (!removeEgrSetDscpTable(table_id)) + { + SWSS_LOG_ERROR("Failed to remove Egress Set DSCP table associated with ACL table %s", table_id.c_str()); + return false; + } + } + sai_object_id_t table_oid = getTableById(table_id); if (table_oid == SAI_NULL_OBJECT_ID) { @@ -4070,8 +4657,11 @@ bool AclOrch::removeAclTable(string table_id) /* If ACL rules associate with this table, remove the rules first.*/ bool suc = m_AclTables[table_oid].clear(); - if (!suc) return false; - + if (!suc) + { + SWSS_LOG_ERROR("Failed to remove ACL rules in table %s", table_id.c_str()); + return false; + } // Unbind table from switch if needed. AclTable &table = m_AclTables.at(table_oid); if (table.bindToSwitch) @@ -4080,6 +4670,7 @@ bool AclOrch::removeAclTable(string table_id) assert(table->stage == ACL_STAGE_EGRESS); if(!gSwitchOrch->unbindAclTableFromSwitch(ACL_STAGE_EGRESS, table.getOid())) { + SWSS_LOG_ERROR("Failed to unbind ACL table %s from switch", table_id.c_str()); return false; } } @@ -4167,28 +4758,56 @@ bool AclOrch::removeAclTableType(const string& tableTypeName) bool AclOrch::addAclRule(shared_ptr newRule, string table_id) { + SWSS_LOG_ENTER(); + bool needsEgrSetDscp = false; + string key = table_id + ":" + newRule->getId(); + // if the table is using EGR_SET_DSCP, we need to add the EGR_SET_DSCP rule. + if (isUsingEgrSetDscp(table_id)) + { + needsEgrSetDscp = true; + string dscpAction = std::to_string(std::static_pointer_cast(newRule)->getDscpValue()); + if (!addEgrSetDscpRule(key, dscpAction)) + { + SWSS_LOG_ERROR("Failed to add Egress Set Dscp rule for Rule %s in table %s.", + newRule->getId().c_str(), table_id.c_str()); + return false; + } + } + // add the regular rule. + bool status = true; sai_object_id_t table_oid = getTableById(table_id); if (table_oid == SAI_NULL_OBJECT_ID) { SWSS_LOG_ERROR("Failed to add ACL rule in ACL table %s. Table doesn't exist", table_id.c_str()); - return false; + status = false; } - - if (!m_AclTables[table_oid].add(newRule)) + if (status && !m_AclTables[table_oid].add(newRule)) { - return false; + status = false; } - if (newRule->hasCounter()) + if (status && newRule->hasCounter()) { registerFlexCounter(*newRule); } - - return true; + if(!status && needsEgrSetDscp) + { + removeEgrSetDscpRule(key); + return false; + } + return status; } bool AclOrch::removeAclRule(string table_id, string rule_id) { + string key = table_id + ":" + rule_id; + if (m_egrDscpRuleMetadata.find(key) != m_egrDscpRuleMetadata.end()) + { + if (!removeEgrSetDscpRule(key)) + { + return false; + } + } sai_object_id_t table_oid = getTableById(table_id); if (table_oid == SAI_NULL_OBJECT_ID) { @@ -4445,6 +5064,94 @@ bool AclOrch::isAclActionEnumValueSupported(sai_acl_action_type_t action, sai_ac return it->second.find(param.s32) != it->second.cend(); } +bool AclOrch::isAclMetaDataSupported() const +{ + if (m_switchMetaDataCapabilities.at(TABLE_ACL_USER_META_DATA_RANGE_CAPABLE) == "true" && + m_switchMetaDataCapabilities.at(TABLE_ACL_ENTRY_ATTR_META_CAPABLE) == "true" && + m_switchMetaDataCapabilities.at(TABLE_ACL_ENTRY_ACTION_META_CAPABLE) == "true") + { + return true; + } + return false; +} + +uint16_t AclOrch::getAclMetaDataMin() const +{ + if (m_switchMetaDataCapabilities.at(TABLE_ACL_USER_META_DATA_RANGE_CAPABLE) == "true") + { + return uint16_t(std::stoi(m_switchMetaDataCapabilities.at(TABLE_ACL_USER_META_DATA_MIN))); + } + return 0; +} + +uint16_t AclOrch::getAclMetaDataMax() const +{ + if (m_switchMetaDataCapabilities.at(TABLE_ACL_USER_META_DATA_RANGE_CAPABLE) == "true") + { + return uint16_t(std::stoi(m_switchMetaDataCapabilities.at(TABLE_ACL_USER_META_DATA_MAX))); + } + return 0; +} + +bool AclOrch::isUsingEgrSetDscp(const string& table) const +{ + if (m_egrSetDscpRef.find(table) != m_egrSetDscpRef.end()) + { + return true; + } + return false; +} + +string AclOrch::translateUnderlaySetDscpTableTypeName(const string& tableTypeName) const +{ + // The TABLE_TYPE_UNDERLAY_SET_DSCP/V6 is translated to table translates into TABLE_TYPE_MARK_META/V6 + if (tableTypeName == TABLE_TYPE_UNDERLAY_SET_DSCP) + { + return TABLE_TYPE_MARK_META; + } + else if(tableTypeName == TABLE_TYPE_UNDERLAY_SET_DSCPV6) + { + return TABLE_TYPE_MARK_META_V6; + } + return tableTypeName; +} + +void AclOrch::addMetaDataRef(string key, uint16_t metadata) +{ + m_egrDscpRuleMetadata[key] = metadata; + if (m_metadataEgrDscpRule.find(metadata) == m_metadataEgrDscpRule.end()) + { + m_metadataEgrDscpRule[metadata] = set(); + } + m_metadataEgrDscpRule[metadata].insert(key); + +} + +void AclOrch::removeMetaDataRef(string key, uint16_t metadata) +{ + m_metadataEgrDscpRule[metadata].erase(key); + m_egrDscpRuleMetadata.erase(key); +} + +uint32_t AclOrch::getMetaDataRefCount(uint16_t metadata) +{ + if (m_metadataEgrDscpRule.find(metadata) != m_metadataEgrDscpRule.end()) + { + return uint32_t(m_metadataEgrDscpRule[metadata].size()); + } + return 0; +} + +uint32_t AclOrch::hasMetaDataRefCount(string key) +{ + if (m_egrDscpRuleMetadata.find(key) != m_egrDscpRuleMetadata.end()) + { + auto md = m_egrDscpRuleMetadata[key]; + return uint32_t(m_metadataEgrDscpRule[md].size()); + } + return 0; +} + void AclOrch::doAclTableTask(Consumer &consumer) { SWSS_LOG_ENTER(); @@ -4522,6 +5229,15 @@ void AclOrch::doAclTableTask(Consumer &consumer) } } + // For the case of Table type TABLE_TYPE_UNDERLAY_SET_DSCP/V6 we need to translate + // it to TABLE_TYPE_MARK_META/V6. We retain the original table type name in orignalTableTypeName + // and pass it ot the updateAclTable/ addAclTable functions. There based on the orignalTableTypeName + // we create/update the EgrSetDscp table. + string firstTableTypeName; + string unused; + string orignalTableTypeName = tableTypeName; + tableTypeName = translateUnderlaySetDscpTableTypeName(tableTypeName); + auto tableType = getAclTableType(tableTypeName); if (!tableType) { @@ -4547,7 +5263,7 @@ void AclOrch::doAclTableTask(Consumer &consumer) m_AclTables[table_oid])) { // Update the existing table using the info in newTable - if (updateAclTable(m_AclTables[table_oid], newTable)) + if (updateAclTable(table_id, newTable, orignalTableTypeName)) { SWSS_LOG_NOTICE("Successfully updated existing ACL table %s", table_id.c_str()); @@ -4564,7 +5280,7 @@ void AclOrch::doAclTableTask(Consumer &consumer) } else { - if (addAclTable(newTable)) + if (addAclTable(table_id, newTable, orignalTableTypeName)) { // Mark ACL table as ACTIVE setAclTableStatus(table_id, AclObjectStatus::ACTIVE); @@ -4572,6 +5288,7 @@ void AclOrch::doAclTableTask(Consumer &consumer) } else { + //we have failed to create the MarkMeta table, we need to remove the EgrSetDscp table setAclTableStatus(table_id, AclObjectStatus::PENDING_CREATION); it++; } @@ -4667,7 +5384,7 @@ void AclOrch::doAclRuleTask(Consumer &consumer) try { - newRule = AclRule::makeShared(this, m_mirrorOrch, m_dTelOrch, rule_id, table_id, t); + newRule = AclRule::makeShared(this, m_mirrorOrch, m_dTelOrch, rule_id, table_id, t, &m_metaDataMgr); } catch (exception &e) { @@ -4746,13 +5463,13 @@ void AclOrch::doAclRuleTask(Consumer &consumer) } if (bHasIPV4 && bHasIPV6) - { - if (type == TABLE_TYPE_L3V4V6) - { - SWSS_LOG_ERROR("Rule '%s' is invalid since it has both v4 and v6 matchfields.", rule_id.c_str()); - bAllAttributesOk = false; - } - } + { + if (type == TABLE_TYPE_L3V4V6) + { + SWSS_LOG_ERROR("Rule '%s' is invalid since it has both v4 and v6 matchfields.", rule_id.c_str()); + bAllAttributesOk = false; + } + } // validate and create ACL rule if (bAllAttributesOk && newRule->validate()) @@ -5197,3 +5914,89 @@ void AclOrch::removeAllAclRuleStatus() m_aclRuleStateTable.del(key); } } + +void MetaDataMgr::populateRange(uint16_t min, uint16_t max) +{ + metaMin = min; + metaMax = max; + SWSS_LOG_INFO("Metadata range %d to %d", metaMin,metaMax); + for (uint16_t i = metaMin; i <= metaMax; i++) + { + m_freeMetadata.push_back(i); + } + initComplete = true; +} + +bool MetaDataMgr::isValidMetaData(uint16_t metadata) +{ + if (metadata >= metaMin && metadata <= metaMax) + { + return true; + } + return false; +} + +uint16_t MetaDataMgr::getFreeMetaData(uint8_t dscp) +{ + uint16_t metadata = metaMax + 1; + SWSS_LOG_INFO("Metadata Request for dscp %d", dscp); + + if (initComplete) + { + if (m_dscpMetadata.find(dscp) != m_dscpMetadata.end()) + { + // dscp value has a metadata value assigned to it. + metadata = m_dscpMetadata[dscp]; + SWSS_LOG_INFO("Metadata %d has already been allocated for dscp %d, refcount %d", metadata, dscp, m_MetadataRef[metadata]+1); + + } + else + { + if (m_freeMetadata.empty()) + { + SWSS_LOG_ERROR("Metadata Value not available for allocation."); + return metadata; + } + metadata = m_freeMetadata.front(); + m_freeMetadata.erase(m_freeMetadata.begin()); + m_dscpMetadata[dscp] = metadata; + SWSS_LOG_INFO("New Metadata %d allocated for dscp %d", metadata, dscp); + } + m_MetadataRef[metadata] += 1; + } + else + { + SWSS_LOG_ERROR("Metadata request before Initialization complete."); + } + return metadata; +} + +void MetaDataMgr::recycleMetaData(uint16_t metadata) +{ + if (initComplete) + { + m_MetadataRef[metadata] -= 1; + SWSS_LOG_INFO("Freeing Metadata %d refcount %d", metadata, m_MetadataRef[metadata]); + if (m_MetadataRef[metadata] == 0) + { + + for (auto iter = m_dscpMetadata.begin(); iter != m_dscpMetadata.end();) + { + if ( iter->second == metadata) + { + m_dscpMetadata.erase(iter++); + m_freeMetadata.push_front(metadata); + break; + } + else + { + ++iter; + } + } + } + } + else + { + SWSS_LOG_ERROR("Unexpected: Metadata free before Initialization complete."); + } +} diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 4dcc450173b..9e27be2ac05 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -53,6 +53,7 @@ #define MATCH_BTH_OPCODE "BTH_OPCODE" #define MATCH_AETH_SYNDROME "AETH_SYNDROME" #define MATCH_TUNNEL_TERM "TUNNEL_TERM" +#define MATCH_METADATA "META_DATA" #define BIND_POINT_TYPE_PORT "PORT" #define BIND_POINT_TYPE_PORTCHANNEL "PORTCHANNEL" @@ -70,6 +71,8 @@ #define ACTION_DTEL_FLOW_SAMPLE_PERCENT "FLOW_SAMPLE_PERCENT" #define ACTION_DTEL_REPORT_ALL_PACKETS "REPORT_ALL_PACKETS" #define ACTION_COUNTER "COUNTER" +#define ACTION_META_DATA "META_DATA_ACTION" +#define ACTION_DSCP "DSCP_ACTION" #define PACKET_ACTION_FORWARD "FORWARD" #define PACKET_ACTION_DROP "DROP" @@ -104,6 +107,12 @@ #define ACL_COUNTER_FLEX_COUNTER_GROUP "ACL_STAT_COUNTER" +#define TABLE_ACL_USER_META_DATA_RANGE_CAPABLE "ACL_USER_META_DATA_RANGE_CAPABLE" +#define TABLE_ACL_USER_META_DATA_MIN "ACL_USER_META_DATA_MIN" +#define TABLE_ACL_USER_META_DATA_MAX "ACL_USER_META_DATA_MAX" +#define TABLE_ACL_ENTRY_ATTR_META_CAPABLE "ACL_ENTRY_ATTR_META_CAPABLE" +#define TABLE_ACL_ENTRY_ACTION_META_CAPABLE "ACL_ENTRY_ACTION_META_CAPABLE" + enum AclObjectStatus { ACTIVE = 0, @@ -112,6 +121,14 @@ enum AclObjectStatus PENDING_REMOVAL }; +enum EgressSetDscpTableStatus +{ + EGRESS_SET_DSCP_TABLE_FAILED = 0, + EGRESS_SET_DSCP_TABLE_SUCCESS, + EGRESS_SET_DSCP_TABLE_NOT_REQUIRED, + EGRESS_SET_DSCP_TABLE_NOT_SUPPORTED +}; + struct AclActionCapabilities { set actionList; @@ -168,6 +185,24 @@ class AclTableRangeMatch: public AclTableMatchInterface private: vector m_rangeList; }; + +class MetaDataMgr +{ +public: + void populateRange(uint16_t min, uint16_t max); + uint16_t getFreeMetaData(uint8_t dscp); + void recycleMetaData(uint16_t metadata); + bool isValidMetaData(uint16_t metadata); + +private: + bool initComplete = false; + uint16_t metaMin = 0; + uint16_t metaMax = 0; + list m_freeMetadata; + map m_dscpMetadata; + map m_MetadataRef; +}; + class AclTableType { public: @@ -281,7 +316,13 @@ class AclRule bool getCreateCounter() const; const vector& getRangeConfig() const; - static shared_ptr makeShared(AclOrch *acl, MirrorOrch *mirror, DTelOrch *dtel, const string& rule, const string& table, const KeyOpFieldsValuesTuple&); + static shared_ptr makeShared(AclOrch *acl, + MirrorOrch *mirror, + DTelOrch *dtel, + const string& rule, + const string& table, + const KeyOpFieldsValuesTuple&, + MetaDataMgr * m_metadataMgr); virtual ~AclRule() {} protected: @@ -381,6 +422,23 @@ class AclRuleDTelWatchListEntry: public AclRule bool INT_session_valid; }; +class AclRuleUnderlaySetDscp: public AclRule +{ +public: + AclRuleUnderlaySetDscp(AclOrch *m_pAclOrch, string rule, string table, MetaDataMgr* m_metaDataMgr, bool createCounter = true); + + bool validateAddAction(string attr_name, string attr_value); + bool validate(); + void onUpdate(SubjectType, void *) override; + uint32_t getDscpValue() const; + uint32_t getMetadata() const; +protected: + uint32_t cachedDscpValue; + uint32_t cachedMetadata; + string table_id; + MetaDataMgr* m_metaDataMgr; +}; + class AclTable { public: @@ -494,6 +552,14 @@ class AclOrch : public Orch, public Observer bool addAclTable(AclTable &aclTable); bool removeAclTable(string table_id); + bool addAclTable(string table_id, AclTable &aclTable, string orignalTableTypeName); + bool updateAclTable(string table_id, AclTable &table, string orignalTableTypeName); + EgressSetDscpTableStatus addEgrSetDscpTable(string table_id, AclTable &table, string orignalTableTypeName); + + bool removeEgrSetDscpTable(string table_id); + bool addEgrSetDscpRule(string key, string dscpAction); + bool removeEgrSetDscpRule(string key); + bool addAclTableType(const AclTableType& tableType); bool removeAclTableType(const string& tableTypeName); bool updateAclTable(AclTable ¤tTable, AclTable &newTable); @@ -513,11 +579,22 @@ class AclOrch : public Orch, public Observer bool isAclActionListMandatoryOnTableCreation(acl_stage_type_t stage) const; bool isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const; bool isAclActionEnumValueSupported(sai_acl_action_type_t action, sai_acl_action_parameter_t param) const; + bool isUsingEgrSetDscp(const string& table) const; + string translateUnderlaySetDscpTableTypeName(const string& tableTypeName) const; + bool isAclMetaDataSupported() const; + uint16_t getAclMetaDataMin() const; + uint16_t getAclMetaDataMax() const; + + void addMetaDataRef(string key, uint16_t metadata); + void removeMetaDataRef(string key, uint16_t metadata); + uint32_t getMetaDataRefCount(uint16_t metadata); + uint32_t hasMetaDataRefCount(string key); bool m_isCombinedMirrorV6Table = true; map m_mirrorTableCapabilities; map m_L3V4V6Capability; - + map m_switchMetaDataCapabilities; + void registerFlexCounter(const AclRule& rule); void deregisterFlexCounter(const AclRule& rule); @@ -593,8 +670,12 @@ class AclOrch : public Orch, public Observer Table m_aclTableStateTable; Table m_aclRuleStateTable; + MetaDataMgr m_metaDataMgr; map m_mirrorTableId; map m_mirrorV6TableId; + set m_egrSetDscpRef; + map> m_metadataEgrDscpRule; + map m_egrDscpRuleMetadata; acl_capabilities_t m_aclCapabilities; acl_action_enum_values_capabilities_t m_aclEnumActionCapabilities; diff --git a/orchagent/acltable.h b/orchagent/acltable.h index 1b1cdeb29ae..8f51fd9a096 100644 --- a/orchagent/acltable.h +++ b/orchagent/acltable.h @@ -35,6 +35,11 @@ extern "C" { #define TABLE_TYPE_MCLAG "MCLAG" #define TABLE_TYPE_MUX "MUX" #define TABLE_TYPE_DROP "DROP" +#define TABLE_TYPE_MARK_META "MARK_META" +#define TABLE_TYPE_MARK_META_V6 "MARK_METAV6" +#define TABLE_TYPE_EGR_SET_DSCP "EGR_SET_DSCP" +#define TABLE_TYPE_UNDERLAY_SET_DSCP "UNDERLAY_SET_DSCP" +#define TABLE_TYPE_UNDERLAY_SET_DSCPV6 "UNDERLAY_SET_DSCPV6" typedef enum { diff --git a/tests/dvslib/dvs_acl.py b/tests/dvslib/dvs_acl.py index 236ccaa0fc1..2197c034b8c 100644 --- a/tests/dvslib/dvs_acl.py +++ b/tests/dvslib/dvs_acl.py @@ -331,6 +331,30 @@ def verify_acl_table_action_list( for action in expected_action_list: assert action in action_list + def create_dscp_acl_rule( + self, + table_name: str, + rule_name: str, + qualifiers: Dict[str, str], + action: str, + priority: str = "2020" + ) -> None: + """Create a new DSCP ACL rule in the given table. + Args: + table_name: The name of the ACL table to add the rule to. + rule_name: The name of the ACL rule. + qualifiers: The list of qualifiers to add to the rule. + action: DSCP value. + priority: The priority of the rule. + """ + fvs = { + "priority": priority, + "DSCP_ACTION": action + } + + for k, v in qualifiers.items(): + fvs[k] = v + self.config_db.create_entry("ACL_RULE", "{}|{}".format(table_name, rule_name), fvs) def create_acl_rule( self, diff --git a/tests/test_acl_mark.py b/tests/test_acl_mark.py new file mode 100644 index 00000000000..aa5b5ec7ed6 --- /dev/null +++ b/tests/test_acl_mark.py @@ -0,0 +1,447 @@ +import pytest +from requests import request + +OVERLAY_TABLE_TYPE = "UNDERLAY_SET_DSCP" +OVERLAY_TABLE_NAME = "OVERLAY_MARK_META_TEST" +OVERLAY_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"] +OVERLAY_RULE_NAME = "OVERLAY_TEST_RULE" + +OVERLAY_TABLE_TYPE6 = "UNDERLAY_SET_DSCPV6" +OVERLAY_TABLE_NAME6 = "OVERLAY_MARK_META_TEST6" +OVERLAY_BIND_PORTS6 = ["Ethernet20", "Ethernet24", "Ethernet28", "Ethernet32"] +OVERLAY_RULE_NAME6 = "OVERLAY_TEST_RULE6" + +# tests for UNDERLAY_SET_DSCP table + + +class TestAclMarkMeta: + @pytest.fixture + def overlay_acl_table(self, dvs_acl): + try: + dvs_acl.create_acl_table(OVERLAY_TABLE_NAME, + OVERLAY_TABLE_TYPE, + OVERLAY_BIND_PORTS) + yield dvs_acl.get_acl_table_ids(2) + finally: + dvs_acl.remove_acl_table(OVERLAY_TABLE_NAME) + dvs_acl.verify_acl_table_count(0) + + @pytest.fixture + def overlay6_acl_table(self, dvs_acl): + try: + dvs_acl.create_acl_table(OVERLAY_TABLE_NAME6, + OVERLAY_TABLE_TYPE6, + OVERLAY_BIND_PORTS6) + yield dvs_acl.get_acl_table_ids(2) + finally: + dvs_acl.remove_acl_table(OVERLAY_TABLE_NAME6) + dvs_acl.verify_acl_table_count(0) + + def verify_acl_table_group_members_multitable(self, dvs_acl, acl_table_id, acl_table_group_ids, member_count): + members = dvs_acl.asic_db.wait_for_n_keys(dvs_acl.ADB_ACL_GROUP_MEMBER_TABLE_NAME, + member_count) + + member_groups = [] + table_member_map = {} + for member in members: + fvs = dvs_acl.asic_db.wait_for_entry(dvs_acl.ADB_ACL_GROUP_MEMBER_TABLE_NAME, member) + group_id = fvs.get("SAI_ACL_TABLE_GROUP_MEMBER_ATTR_ACL_TABLE_GROUP_ID") + table_id = fvs.get("SAI_ACL_TABLE_GROUP_MEMBER_ATTR_ACL_TABLE_ID") + + if group_id in acl_table_group_ids and table_id in acl_table_id: + member_groups.append(group_id) + if table_id not in table_member_map: + table_member_map[table_id] = [] + table_member_map[table_id].append(group_id) + + assert set(member_groups) == set(acl_table_group_ids) + return table_member_map + + def get_table_stage(self, dvs_acl, acl_table_id, v4_ports, v6_ports): + stages = [] + names = [] + ports = [] + for table in acl_table_id: + fvs = dvs_acl.asic_db.wait_for_entry(dvs_acl.ADB_ACL_TABLE_NAME, table) + stage = fvs.get("SAI_ACL_TABLE_ATTR_ACL_STAGE") + if stage == "SAI_ACL_STAGE_INGRESS": + stages.append("ingress") + elif stage == "SAI_ACL_STAGE_EGRESS": + stages.append("egress") + qual = fvs.get("SAI_ACL_TABLE_ATTR_FIELD_ACL_USER_META") + if qual == "true": + names.append("EGR_SET_DSCP") + ports.append(v4_ports+v6_ports) + qual = fvs.get("SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6") + if qual == "true": + names.append("MARK_META6") + ports.append(v6_ports) + qual = fvs.get("SAI_ACL_TABLE_ATTR_FIELD_DST_IP") + if qual == "true": + names.append("MARK_META") + ports.append(v4_ports) + return stages, names, ports + + def verify_acl_table_port_binding_multi(self, dvs_acl, table_member_map, bind_ports, stages, acl_table_id): + for i in range(0, len(stages)): + stage = stages[i] + table = acl_table_id[i] + port_groups = [] + for port in bind_ports[i]: + port_oid = dvs_acl.counters_db.get_entry("COUNTERS_PORT_NAME_MAP", "").get(port) + fvs = dvs_acl.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_PORT", port_oid) + acl_table_group_id = fvs.pop(dvs_acl.ADB_PORT_ATTR_LOOKUP[stage], None) + assert acl_table_group_id in table_member_map[table] + port_groups.append(acl_table_group_id) + + assert len(port_groups) == len(bind_ports[i]) + assert set(port_groups) == set(table_member_map[table]) + + + def get_acl_rules_with_action(self, dvs_acl, total_rules): + """Verify that there are N rules in the ASIC DB.""" + members = dvs_acl.asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", + total_rules) + + member_groups = [] + table_member_map = {} + for member in members: + fvs = dvs_acl.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY", member) + table_id = fvs.get("SAI_ACL_ENTRY_ATTR_TABLE_ID") + entry = {} + entry['id'] = member + action = fvs.get("SAI_ACL_ENTRY_ATTR_ACTION_SET_DSCP") + if action: + entry['action_type'] = "dscp" + entry['action_value'] = action + meta = fvs.get("SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META") + entry['match_meta'] = meta.split('&')[0] + action = fvs.get("SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA") + if action: + entry['action_type'] = "meta" + entry['action_value'] = action + + if table_id not in table_member_map: + table_member_map[table_id] = [] + table_member_map[table_id].append(entry) + return table_member_map + + def verify_acl_rules_with_action(self, table_names, acl_table_id, table_rules, meta, dscp): + for i in range(0, len(table_names)): + if acl_table_id[i] in table_rules: + for j in range(0, len(table_rules[acl_table_id[i]])): + if table_names[i] == "MARK_META" or table_names[i] == "MARK_META6": + assert table_rules[acl_table_id[i]][j]['action_type'] == "meta" + assert table_rules[acl_table_id[i]][j]['action_value'] in meta + else: + assert table_rules[acl_table_id[i]][j]['action_type'] == "dscp" + assert table_rules[acl_table_id[i]][j]['action_value'] in dscp + assert table_rules[acl_table_id[i]][j]['match_meta'] in meta + + def test_OverlayTableCreationDeletion(self, dvs_acl): + try: + dvs_acl.create_acl_table(OVERLAY_TABLE_NAME, OVERLAY_TABLE_TYPE, OVERLAY_BIND_PORTS) + # this should create 2 tables. MARK_META and EGR_SET_DSCP Verify the table count. + acl_table_id = dvs_acl.get_acl_table_ids(2) + stages, names, ports = self.get_table_stage(dvs_acl, acl_table_id, OVERLAY_BIND_PORTS, []) + + acl_table_group_ids = dvs_acl.get_acl_table_group_ids(len(OVERLAY_BIND_PORTS)*2) + table_member_map = self.verify_acl_table_group_members_multitable(dvs_acl, acl_table_id, acl_table_group_ids, 8) + + self.verify_acl_table_port_binding_multi(dvs_acl, table_member_map, ports, stages, acl_table_id) + + # Verify status is written into STATE_DB + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME, "Active") + finally: + dvs_acl.remove_acl_table(OVERLAY_TABLE_NAME) + dvs_acl.verify_acl_table_count(0) + # Verify the STATE_DB entry is removed + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME, None) + + def test_Overlay6TableCreationDeletion(self, dvs_acl): + try: + dvs_acl.create_acl_table(OVERLAY_TABLE_NAME6, OVERLAY_TABLE_TYPE6, OVERLAY_BIND_PORTS6) + # this should create 2 tables. MARK_META and EGR_SET_DSCP Verify the table count. + acl_table_id = dvs_acl.get_acl_table_ids(2) + stages, names, ports = self.get_table_stage(dvs_acl, acl_table_id, [], OVERLAY_BIND_PORTS6) + acl_table_group_ids = dvs_acl.get_acl_table_group_ids(len(OVERLAY_BIND_PORTS6)*2) + table_member_map = self.verify_acl_table_group_members_multitable(dvs_acl, acl_table_id, acl_table_group_ids, 8) + + self.verify_acl_table_port_binding_multi(dvs_acl, table_member_map, ports, stages, acl_table_id) + + # Verify status is written into STATE_DB + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME6, "Active") + finally: + dvs_acl.remove_acl_table(OVERLAY_TABLE_NAME6) + dvs_acl.verify_acl_table_count(0) + # Verify the STATE_DB entry is removed + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME6, None) + + def test_OverlayBothv4v6TableCreationDeletion(self, dvs_acl): + try: + dvs_acl.create_acl_table(OVERLAY_TABLE_NAME, OVERLAY_TABLE_TYPE, OVERLAY_BIND_PORTS) + dvs_acl.create_acl_table(OVERLAY_TABLE_NAME6, OVERLAY_TABLE_TYPE6, OVERLAY_BIND_PORTS6) + # this should create 2 tables. MARK_META and EGR_SET_DSCP Verify the table count. + acl_table_id = dvs_acl.get_acl_table_ids(3) + stages, names, ports = self.get_table_stage(dvs_acl, acl_table_id,OVERLAY_BIND_PORTS, OVERLAY_BIND_PORTS6) + acl_table_group_ids = dvs_acl.get_acl_table_group_ids(len(OVERLAY_BIND_PORTS6)*4) + table_member_map = self.verify_acl_table_group_members_multitable(dvs_acl, acl_table_id, acl_table_group_ids, 16) + + self.verify_acl_table_port_binding_multi(dvs_acl, table_member_map, ports, stages, acl_table_id) + + # Verify status is written into STATE_DB + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME, "Active") + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME6, "Active") + finally: + dvs_acl.remove_acl_table(OVERLAY_TABLE_NAME) + dvs_acl.verify_acl_table_count(2) + # Verify the STATE_DB entry is removed + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME, None) + acl_table_id = dvs_acl.get_acl_table_ids(2) + + stages, names, ports = self.get_table_stage(dvs_acl, acl_table_id, [], OVERLAY_BIND_PORTS6) + acl_table_group_ids = dvs_acl.get_acl_table_group_ids(len(OVERLAY_BIND_PORTS6)*2) + table_member_map = self.verify_acl_table_group_members_multitable(dvs_acl, acl_table_id, acl_table_group_ids, 8) + + dvs_acl.remove_acl_table(OVERLAY_TABLE_NAME6) + dvs_acl.verify_acl_table_count(0) + # Verify the STATE_DB entry is removed + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME6, None) + + def test_OverlayBothv4v6TableSameintfCreationDeletion(self, dvs_acl): + try: + dvs_acl.create_acl_table(OVERLAY_TABLE_NAME, OVERLAY_TABLE_TYPE, OVERLAY_BIND_PORTS) + dvs_acl.create_acl_table(OVERLAY_TABLE_NAME6, OVERLAY_TABLE_TYPE6, OVERLAY_BIND_PORTS) + # this should create 2 tables. MARK_META and EGR_SET_DSCP Verify the table count. + acl_table_id = dvs_acl.get_acl_table_ids(3) + stages, names, ports = self.get_table_stage(dvs_acl, acl_table_id,OVERLAY_BIND_PORTS, OVERLAY_BIND_PORTS) + acl_table_group_ids = dvs_acl.get_acl_table_group_ids(len(OVERLAY_BIND_PORTS)*2) + table_member_map = self.verify_acl_table_group_members_multitable(dvs_acl, acl_table_id, acl_table_group_ids, 12) + + self.verify_acl_table_port_binding_multi(dvs_acl, table_member_map, ports, stages, acl_table_id) + + # Verify status is written into STATE_DB + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME, "Active") + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME6, "Active") + finally: + dvs_acl.remove_acl_table(OVERLAY_TABLE_NAME) + dvs_acl.verify_acl_table_count(2) + # Verify the STATE_DB entry is removed + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME, None) + acl_table_id = dvs_acl.get_acl_table_ids(2) + + stages, names, ports = self.get_table_stage(dvs_acl, acl_table_id, [], OVERLAY_BIND_PORTS) + acl_table_group_ids = dvs_acl.get_acl_table_group_ids(len(OVERLAY_BIND_PORTS)*2) + table_member_map = self.verify_acl_table_group_members_multitable(dvs_acl, acl_table_id, acl_table_group_ids, 8) + + dvs_acl.remove_acl_table(OVERLAY_TABLE_NAME6) + dvs_acl.verify_acl_table_count(0) + # Verify the STATE_DB entry is removed + dvs_acl.verify_acl_table_status(OVERLAY_TABLE_NAME6, None) + + def test_OverlayEntryCreationDeletion(self, dvs_acl, overlay_acl_table): + config_qualifiers = {"DST_IP": "20.0.0.1/32", + "SRC_IP": "10.0.0.0/32"} + acl_table_id = dvs_acl.get_acl_table_ids(2) + _, names, _ = self.get_table_stage(dvs_acl, acl_table_id, [], OVERLAY_BIND_PORTS) + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME, "VALID_RULE", config_qualifiers,action="12") + # Verify status is written into STATE_DB + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "VALID_RULE", "Active") + table_rules = self.get_acl_rules_with_action(dvs_acl, 2) + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, ["1"], ["12"]) + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME, "VALID_RULE") + # Verify the STATE_DB entry is removed + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "VALID_RULE", None) + dvs_acl.verify_no_acl_rules() + + def test_OverlayEntryMultiRuleRef(self, dvs_acl, overlay_acl_table): + config_qualifiers = {"DST_IP": "20.0.0.1/32", + "SRC_IP": "10.0.0.0/32", + "DSCP": "1" + } + acl_table_id = dvs_acl.get_acl_table_ids(2) + _, names, _ = self.get_table_stage(dvs_acl, acl_table_id, [], OVERLAY_BIND_PORTS) + #create 1st Rule + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME, "1", config_qualifiers, action="12") + #create 2nd Rule + config_qualifiers["DSCP"] = "2" + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME, "2", config_qualifiers, action="12") + #create 3rd Rule + config_qualifiers["DSCP"] = "3" + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME, "3", config_qualifiers, action="12") + + #This should create 4 rules 3 for MARK_META and 1 for EGR_SET_DSCP + # Verify status is written into STATE_DB + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "1", "Active") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "2", "Active") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "3", "Active") + table_rules = self.get_acl_rules_with_action(dvs_acl, 4) + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, ["1"], ["12"]) + + # remove first rule. We should still have 3 rules, 2 for MARK_META and 1 for EGR_SET_DSCP + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME, "1") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "1", None) + table_rules = self.get_acl_rules_with_action(dvs_acl, 3) + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, ["1"], ["12"]) + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "1", None) + + # remove 2nd rule. We should still have 2 rules, 1 for MARK_META and 1 for EGR_SET_DSCP + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME, "2") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "2", None) + table_rules = self.get_acl_rules_with_action(dvs_acl, 2) + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, ["1"], ["12"]) + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "2", None) + + # Verify the STATE_DB entry is removed + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME, "3") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "3", None) + + dvs_acl.verify_no_acl_rules() + + def test_OverlayEntryMultiTableRules(self, dvs_acl): + config_qualifiers = {"DST_IP": "20.0.0.1/32", + "SRC_IP": "10.0.0.0/32", + "DSCP": "1"} + dvs_acl.create_acl_table(OVERLAY_TABLE_NAME, + OVERLAY_TABLE_TYPE, + OVERLAY_BIND_PORTS) + dvs_acl.create_acl_table(OVERLAY_TABLE_NAME6, + OVERLAY_TABLE_TYPE6, + OVERLAY_BIND_PORTS6) + acl_table_id = dvs_acl.get_acl_table_ids(3) + _, names, _ = self.get_table_stage(dvs_acl, acl_table_id, [], OVERLAY_BIND_PORTS) + #create 1st Rule + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME, "1", config_qualifiers, action="12") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "1", "Active") + + #create 2nd Rule ipv6 + config_qualifiers6 = {"SRC_IPV6": "2777::0/64", + "DST_IPV6": "2788::0/64", + "DSCP" : "1"}; + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME6, "1", config_qualifiers6, action="12") + + # Verify status of both rules. + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "1", "Active") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME6, "1", "Active") + table_rules = self.get_acl_rules_with_action(dvs_acl, 3) + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, ["1"], ["12"]) + + # remove first rule. We should still have 1 rule, 1 for MARK_META + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME6, "1") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME6, "1", None) + table_rules = self.get_acl_rules_with_action(dvs_acl, 2) + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, ["1"], ["12"]) + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "1", "Active") + + # remove 2nd rule. We should still have 2 rules, 1 for MARK_META and 1 for EGR_SET_DSCP + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME, "1") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "1", None) + dvs_acl.verify_no_acl_rules() + dvs_acl.remove_acl_table(OVERLAY_TABLE_NAME) + dvs_acl.remove_acl_table(OVERLAY_TABLE_NAME6) + dvs_acl.verify_acl_table_count(0) + + def test_OverlayEntryMultiMetaRule(self, dvs_acl, overlay_acl_table): + config_qualifiers = {"DST_IP": "20.0.0.1/32", + "SRC_IP": "10.0.0.0/32", + "DSCP": "1" + } + + acl_table_id = dvs_acl.get_acl_table_ids(2) + _, names, _ = self.get_table_stage(dvs_acl, acl_table_id, [], OVERLAY_BIND_PORTS) + #create 1st Rule + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME, "1", config_qualifiers, action="12") + #create 2nd Rule + config_qualifiers["DSCP"] = "2" + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME, "2", config_qualifiers, action="13") + #create 3rd Rule + config_qualifiers["DSCP"] = "3" + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME, "3", config_qualifiers, action="14") + + #This should create 4 rules 3 for MARK_META and 1 for EGR_SET_DSCP + # Verify status is written into STATE_DB + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "1", "Active") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "2", "Active") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "3", "Active") + table_rules = self.get_acl_rules_with_action(dvs_acl, 6) + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, ["1", "2", "3"], ["12", "13", "14"]) + + # remove first rule. We should still have 3 rules, 2 for MARK_META and 1 for EGR_SET_DSCP + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME, "1") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "1", None) + table_rules = self.get_acl_rules_with_action(dvs_acl, 4) + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, ["1", "2", "3"], ["12", "13", "14"]) + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "1", None) + + # remove 2nd rule. We should still have 2 rules, 1 for MARK_META and 1 for EGR_SET_DSCP + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME, "2") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "2", None) + table_rules = self.get_acl_rules_with_action(dvs_acl, 2) + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, ["1", "2", "3"], ["12", "13", "14"]) + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "2", None) + + # Verify the STATE_DB entry is removed + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME, "3") + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, "3", None) + + dvs_acl.verify_no_acl_rules() + + def test_OverlayEntryExhaustMeta(self, dvs_acl, overlay_acl_table): + config_qualifiers = {"DST_IP": "20.0.0.1/32", + "SRC_IP": "10.0.0.0/32", + "DSCP": "1" + } + acl_table_id = dvs_acl.get_acl_table_ids(2) + _, names, _ = self.get_table_stage(dvs_acl, acl_table_id, [], OVERLAY_BIND_PORTS) + #create 8 rules. 8th one should fail. + for i in range(1, 9): + config_qualifiers["DSCP"] = str(i) + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME, str(i), config_qualifiers, action=str(i+10)) + if i < 8: + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, str(i), "Active") + else: + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, str(i), None) + + table_rules = self.get_acl_rules_with_action(dvs_acl, 14) + meta = [str(i) for i in range(1, 8)] + dscps = [str(i) for i in range(11, 18)] + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, meta, dscps) + + for i in range(1, 9): + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME, str(i)) + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, str(i), None) + dvs_acl.verify_no_acl_rules() + + def test_OverlayEntryTestMetaDataMgr(self, dvs_acl, overlay_acl_table): + # allocate all 7 metadata values and free them multiple times. + # At the end there should be no rules allocated. + for i in range(1, 4): + config_qualifiers = {"DST_IP": "20.0.0.1/32", + "SRC_IP": "10.0.0.0/32", + "DSCP": "1" + } + acl_table_id = dvs_acl.get_acl_table_ids(2) + _, names, _ = self.get_table_stage(dvs_acl, acl_table_id, [], OVERLAY_BIND_PORTS) + #create 8 rules. 8th one should fail. + for i in range(1, 9): + config_qualifiers["DSCP"] = str(i) + dvs_acl.create_dscp_acl_rule(OVERLAY_TABLE_NAME, str(i), config_qualifiers, action=str(i+10)) + if i < 8: + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, str(i), "Active") + else: + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, str(i), None) + + table_rules = self.get_acl_rules_with_action(dvs_acl, 14) + meta = [str(i) for i in range(1, 8)] + dscps = [str(i) for i in range(11, 18)] + self.verify_acl_rules_with_action(names, acl_table_id, table_rules, meta, dscps) + + for i in range(1, 9): + dvs_acl.remove_acl_rule(OVERLAY_TABLE_NAME, str(i)) + dvs_acl.verify_acl_rule_status(OVERLAY_TABLE_NAME, str(i), None) + dvs_acl.verify_no_acl_rules() + + # Add Dummy always-pass test at end as workaroud +# for issue when Flaky fail on final test it invokes module tear-down before retrying +def test_nonflaky_dummy(): + pass \ No newline at end of file From 1843de47c7c71e6587a97c8c6ba6d94c76061687 Mon Sep 17 00:00:00 2001 From: Lawrence Lee Date: Tue, 19 Nov 2024 17:03:32 -0800 Subject: [PATCH 4/8] Ensure second route group OID is correct (#3380) What I did When grabbing the second route group OID, ensure that it is distinct from the first route group OID Why I did it The order in which DB keys are returned is not deterministic, so blindly taking the last key returned will sometimes lead to selecting the wrong OID --- tests/dash/test_dash_route_group.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/dash/test_dash_route_group.py b/tests/dash/test_dash_route_group.py index 459507aaa1f..dbc8f31eeba 100644 --- a/tests/dash/test_dash_route_group.py +++ b/tests/dash/test_dash_route_group.py @@ -44,7 +44,11 @@ def test_rebind_eni_route_group(dash_db: DashDB): dash_db.set_app_db_entry(APP_DASH_ROUTE_GROUP_TABLE_NAME, ROUTE_GROUP2, ROUTE_GROUP2_CONFIG) dash_db.set_app_db_entry(APP_DASH_ROUTE_TABLE_NAME, ROUTE_GROUP2, OUTBOUND_ROUTE_PREFIX1, ROUTE_VNET_CONFIG_UNDERLAY_SIP) - rg2_oid = dash_db.wait_for_asic_db_keys("ASIC_STATE:SAI_OBJECT_TYPE_OUTBOUND_ROUTING_GROUP", min_keys=2)[1] + oids = dash_db.wait_for_asic_db_keys("ASIC_STATE:SAI_OBJECT_TYPE_OUTBOUND_ROUTING_GROUP", min_keys=2) + for oid in oids: + if oid != rg1_oid: + rg2_oid = oid + break dash_db.set_app_db_entry(APP_DASH_ENI_ROUTE_TABLE_NAME, ENI_ID, ENI_ROUTE_GROUP1_CONFIG) From eda63a9b25abea80638322b5d28d9acc82d1e1b5 Mon Sep 17 00:00:00 2001 From: abdosi <58047199+abdosi@users.noreply.github.com> Date: Thu, 21 Nov 2024 13:18:02 -0800 Subject: [PATCH 5/8] Vlanmgrd handling of portchannel does not exist more gracefully. (#3367) What I did: Fixes sonic-net/sonic-buildimage#20684 Why I did: It is seen when doing config reload on multi-asic (multiple instances of swss/teamd) their might be race condition when teammrd might be getting SIGTERM and cleaning up the port-channel in kernel and vlanmgrd might be adding the port channel as member in kernel and this can cause the exception as this operation as being done in async. How I did: To handle the failure gracefully if we get an exception when portchannel as vlan member in kernel we check if Portchannel exists and if not we just retry to minimize the risk of failure due to above race-condition. --- cfgmgr/vlanmgr.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/cfgmgr/vlanmgr.cpp b/cfgmgr/vlanmgr.cpp index c23a23cf141..5e1c8239827 100644 --- a/cfgmgr/vlanmgr.cpp +++ b/cfgmgr/vlanmgr.cpp @@ -221,7 +221,17 @@ bool VlanMgr::addHostVlanMember(int vlan_id, const string &port_alias, const str cmds << BASH_CMD " -c " << shellquote(inner.str()); std::string res; - EXEC_WITH_ERROR_THROW(cmds.str(), res); + try + { + EXEC_WITH_ERROR_THROW(cmds.str(), res); + } + catch (const std::runtime_error& e) + { + if (!isMemberStateOk(port_alias)) + return false; + else + EXEC_WITH_ERROR_THROW(cmds.str(), res); + } return true; } @@ -632,6 +642,12 @@ void VlanMgr::doVlanMemberTask(Consumer &consumer) m_vlanMemberReplay.erase(kfvKey(t)); } + else + { + SWSS_LOG_INFO("Netdevice for %s not ready, delaying", kfvKey(t).c_str()); + it++; + continue; + } } else if (op == DEL_COMMAND) { From 3d5ac540f3ae14277f8dba9d3f7ba6361ed9144c Mon Sep 17 00:00:00 2001 From: Stephen Sun <5379172+stephenxs@users.noreply.github.com> Date: Sat, 23 Nov 2024 00:40:10 +0800 Subject: [PATCH 6/8] Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled (#3334) Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled *There is a configuration to optimize the buffer performance DEVICE_METADATA|localhost.create_only_config_db_buffers. Using this configuration the buffer counters are created only for the queues and PGs with buffer configured for performance optimization. However, to have counters for CPU tx queue, we have to configure it in BUFFER_QUEUE, mainly for data traffic, which is a bad coupling. Using this PR the counter will be created for CPU tx queue without data buffer configuration. --- orchagent/flexcounterorch.cpp | 7 +++++ orchagent/p4orch/tests/fake_portorch.cpp | 4 +-- orchagent/port.h | 2 ++ orchagent/portsorch.cpp | 40 ++++++++++++++++++++++-- orchagent/portsorch.h | 4 +-- tests/test_flex_counters.py | 36 +++++++++++---------- 6 files changed, 71 insertions(+), 22 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 705622bfa0c..b8918d0ac95 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -408,6 +408,13 @@ map FlexCounterOrch::getQueueConfigurations() { queuesStateVector.at(configPortName).enableQueueCounter(startIndex); } + + Port port; + gPortsOrch->getPort(configPortName, port); + if (port.m_host_tx_queue_configured && port.m_host_tx_queue <= maxQueueIndex) + { + queuesStateVector.at(configPortName).enableQueueCounter(port.m_host_tx_queue); + } } catch (std::invalid_argument const& e) { SWSS_LOG_ERROR("Invalid queue index [%s] for port [%s]", configPortQueues.c_str(), configPortName.c_str()); continue; diff --git a/orchagent/p4orch/tests/fake_portorch.cpp b/orchagent/p4orch/tests/fake_portorch.cpp index c3340e0cf30..8857721643f 100644 --- a/orchagent/p4orch/tests/fake_portorch.cpp +++ b/orchagent/p4orch/tests/fake_portorch.cpp @@ -191,11 +191,11 @@ void PortsOrch::generateQueueMapPerPort(const Port &port, FlexCounterQueueStates { } -void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues) +void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue) { } -void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues) +void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue) { } diff --git a/orchagent/port.h b/orchagent/port.h index 318a60a3764..1c7c34999f5 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -201,6 +201,8 @@ class Port sai_port_priority_flow_control_mode_t m_pfc_asym = SAI_PORT_PRIORITY_FLOW_CONTROL_MODE_COMBINED; uint8_t m_pfc_bitmask = 0; // PFC enable bit mask uint8_t m_pfcwd_sw_bitmask = 0; // PFC software watchdog enable + uint8_t m_host_tx_queue = 0; + bool m_host_tx_queue_configured = false; uint16_t m_tpid = DEFAULT_TPID; uint32_t m_nat_zone_id = 0; uint32_t m_vnid = VNID_NONE; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 2ce9b31b6f3..7e21f8c50c7 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -3570,6 +3570,12 @@ bool PortsOrch::initPort(const PortConfig &port) m_recircPortRole[alias] = role; } + // We have to test the size of m_queue_ids here since it isn't initialized on some platforms (like DPU) + if (p.m_host_tx_queue_configured && p.m_queue_ids.size() > p.m_host_tx_queue) + { + createPortBufferQueueCounters(p, to_string(p.m_host_tx_queue), false); + } + SWSS_LOG_NOTICE("Initialized port %s", alias.c_str()); } else @@ -3600,6 +3606,11 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id) return; } + if (p.m_host_tx_queue_configured && p.m_queue_ids.size() > p.m_host_tx_queue) + { + removePortBufferQueueCounters(p, to_string(p.m_host_tx_queue), false); + } + /* remove port from flex_counter_table for updating counters */ auto flex_counters_orch = gDirectory.get(); if ((flex_counters_orch->getPortCountersState())) @@ -6013,6 +6024,9 @@ bool PortsOrch::addHostIntfs(Port &port, string alias, sai_object_id_t &host_int attr.id = SAI_HOSTIF_ATTR_QUEUE; attr.value.u32 = DEFAULT_HOSTIF_TX_QUEUE; attrs.push_back(attr); + + port.m_host_tx_queue = DEFAULT_HOSTIF_TX_QUEUE; + port.m_host_tx_queue_configured = true; } sai_status_t status = sai_hostif_api->create_hostif(&host_intfs_id, gSwitchId, (uint32_t)attrs.size(), attrs.data()); @@ -7320,6 +7334,10 @@ void PortsOrch::generateQueueMap(map queuesState { flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1); } + else if (it.second.m_host_tx_queue_configured && it.second.m_host_tx_queue <= maxQueueNumber) + { + flexCounterQueueState.enableQueueCounters(it.second.m_host_tx_queue, it.second.m_host_tx_queue); + } queuesStateVector.insert(make_pair(it.second.m_alias, flexCounterQueueState)); } generateQueueMapPerPort(it.second, queuesStateVector.at(it.second.m_alias), false); @@ -7458,6 +7476,10 @@ void PortsOrch::addQueueFlexCounters(map queuesS { flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1); } + else if (it.second.m_host_tx_queue_configured && it.second.m_host_tx_queue <= maxQueueNumber) + { + flexCounterQueueState.enableQueueCounters(it.second.m_host_tx_queue, it.second.m_host_tx_queue); + } queuesStateVector.insert(make_pair(it.second.m_alias, flexCounterQueueState)); } addQueueFlexCountersPerPort(it.second, queuesStateVector.at(it.second.m_alias)); @@ -7539,6 +7561,10 @@ void PortsOrch::addQueueWatermarkFlexCounters(map queuesStateVector); uint32_t getNumberOfPortSupportedQueueCounters(string port); - void createPortBufferQueueCounters(const Port &port, string queues); - void removePortBufferQueueCounters(const Port &port, string queues); + void createPortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue=true); + void removePortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue=true); void addQueueFlexCounters(map queuesStateVector); void addQueueWatermarkFlexCounters(map queuesStateVector); diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index c664f0390e6..1963248c2f3 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -609,12 +609,12 @@ def remove_ip_address(self, interface, ip): def set_admin_status(self, interface, status): self.config_db.update_entry("PORT", interface, {"admin_status": status}) - @pytest.mark.parametrize('counter_type', [('queue_counter'), ('pg_drop_counter')]) - def test_create_only_config_db_buffers_false(self, dvs, counter_type): + @pytest.mark.parametrize('counter_type_id', [('queue_counter', '8'), ('pg_drop_counter', '7')]) + def test_create_only_config_db_buffers_false(self, dvs, counter_type_id): """ Test steps: 1. By default the configuration knob 'create_only_config_db_value' is missing. - 2. Get the counter OID for the interface 'Ethernet0:7' from the counters database. + 2. Get the counter OID for the interface 'Ethernet0', queue 8 or PG 7, from the counters database. 3. Perform assertions based on the 'create_only_config_db_value': - If 'create_only_config_db_value' is 'false' or does not exist, assert that the counter OID has a valid OID value. @@ -623,10 +623,11 @@ def test_create_only_config_db_buffers_false(self, dvs, counter_type): counter_type (str): The type of counter being tested """ self.setup_dbs(dvs) + counter_type, index = counter_type_id meta_data = counter_group_meta[counter_type] self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map']) - counter_oid = self.counters_db.db_connection.hget(meta_data['name_map'], 'Ethernet0:7') + counter_oid = self.counters_db.db_connection.hget(meta_data['name_map'], 'Ethernet0:' + index) assert counter_oid is not None, "Counter OID should have a valid OID value when create_only_config_db_value is 'false' or does not exist" def test_create_remove_buffer_pg_watermark_counter(self, dvs): @@ -658,12 +659,12 @@ def test_create_remove_buffer_pg_watermark_counter(self, dvs): self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '1', False) self.wait_for_id_list_remove(meta_data['group_name'], "Ethernet0", counter_oid) - @pytest.mark.parametrize('counter_type', [('queue_counter'), ('pg_drop_counter')]) - def test_create_only_config_db_buffers_true(self, dvs, counter_type): + @pytest.mark.parametrize('counter_type_id', [('queue_counter', '8'), ('pg_drop_counter', '7')]) + def test_create_only_config_db_buffers_true(self, dvs, counter_type_id): """ Test steps: 1. The 'create_only_config_db_buffers' was set to 'true' by previous test. - 2. Get the counter OID for the interface 'Ethernet0:7' from the counters database. + 2. Get the counter OID for the interface 'Ethernet0', queue 8 or PG 7, from the counters database. 3. Perform assertions based on the 'create_only_config_db_value': - If 'create_only_config_db_value' is 'true', assert that the counter OID is None. @@ -671,11 +672,12 @@ def test_create_only_config_db_buffers_true(self, dvs, counter_type): dvs (object): virtual switch object counter_type (str): The type of counter being tested """ + counter_type, index = counter_type_id self.setup_dbs(dvs) meta_data = counter_group_meta[counter_type] self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map']) - counter_oid = self.counters_db.db_connection.hget(meta_data['name_map'], 'Ethernet0:7') + counter_oid = self.counters_db.db_connection.hget(meta_data['name_map'], 'Ethernet0:' + index) assert counter_oid is None, "Counter OID should be None when create_only_config_db_value is 'true'" def test_create_remove_buffer_queue_counter(self, dvs): @@ -695,12 +697,12 @@ def test_create_remove_buffer_queue_counter(self, dvs): self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map']) - self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|7', {'profile': 'egress_lossless_profile'}) - counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', True) + self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|8', {'profile': 'egress_lossless_profile'}) + counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '8', True) self.wait_for_id_list(meta_data['group_name'], "Ethernet0", counter_oid) - self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|7') - self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', False) + self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|8') + self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '8', False) self.wait_for_id_list_remove(meta_data['group_name'], "Ethernet0", counter_oid) def test_create_remove_buffer_watermark_queue_pg_counter(self, dvs): @@ -723,16 +725,18 @@ def test_create_remove_buffer_watermark_queue_pg_counter(self, dvs): self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map']) self.config_db.update_entry('BUFFER_PG', 'Ethernet0|7', {'profile': 'ingress_lossy_profile'}) - self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|7', {'profile': 'egress_lossless_profile'}) + self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|8', {'profile': 'egress_lossless_profile'}) for counterpoll_type, meta_data in counter_group_meta.items(): if 'queue' in counterpoll_type or 'pg' in counterpoll_type: - counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', True) + index = '8' if 'queue' in counterpoll_type else '7' + counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', index, True) self.wait_for_id_list(meta_data['group_name'], "Ethernet0", counter_oid) - self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|7') + self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|8') self.config_db.delete_entry('BUFFER_PG', 'Ethernet0|7') for counterpoll_type, meta_data in counter_group_meta.items(): if 'queue' in counterpoll_type or 'pg' in counterpoll_type: - self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', False) + index = '8' if 'queue' in counterpoll_type else '7' + self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', index, False) self.wait_for_id_list_remove(meta_data['group_name'], "Ethernet0", counter_oid) From 1847195cf02a2baa478a664a671adc95c704a810 Mon Sep 17 00:00:00 2001 From: vincentpcng <129542523+vincentpcng@users.noreply.github.com> Date: Fri, 22 Nov 2024 10:01:49 -0800 Subject: [PATCH 7/8] Add port FEC BER feature swss, HLD#1829 (#3363) What I did This is to add the swss changes for the feature port FEC BER. It modify the port_rates.lua script to compute the BER and stored them in the DB . Two additonal PR(s) will address the cli ( sonic-utilities) and the sonic-mgmt changes Why I did it The HLD for this feature is HLD#1829 How I verified it We verify the counters internally (1) verify when link has correctable FEC errors, the BER were calcuated accordingly (2) verify 400G with 4x100 and the BER , four serdes with 100G rate (3) run a redis-script to polling the DB counters and verify the calculation (4) stop polling manually modify the uncorrectable counter and verify the cli display and calculation --- orchagent/port_rates.lua | 146 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 142 insertions(+), 4 deletions(-) diff --git a/orchagent/port_rates.lua b/orchagent/port_rates.lua index c29977d153b..d7b8b8e4b14 100644 --- a/orchagent/port_rates.lua +++ b/orchagent/port_rates.lua @@ -13,6 +13,9 @@ end local counters_db = ARGV[1] local counters_table_name = ARGV[2] local rates_table_name = "RATES" +local appl_db_port = "PORT_TABLE" +-- refer back to common/schema.h +local appl_db = "0" -- Get configuration redis.call('SELECT', counters_db) @@ -29,11 +32,117 @@ logit(alpha) logit(one_minus_alpha) logit(delta) +local port_interface_oid_map = redis.call('HGETALL', "COUNTERS_PORT_NAME_MAP") +local port_interface_oid_key_count = redis.call('HLEN', "COUNTERS_PORT_NAME_MAP") + +-- lookup interface name from port oid + +local function find_interface_name_from_oid(port) + + for i = 1, port_interface_oid_key_count do + local index = i * 2 - 1 + if port_interface_oid_map[index + 1] == port then + return port_interface_oid_map[index] + end + end + + return 0 +end + +-- calculate lanes and serdes speed from interface lane count & speed +-- return lane speed and serdes speed + +local function calculate_lane_and_serdes_speed(count, speed) + + local serdes = 0 + local lane_speed = 0 + + if count == 0 or speed == 0 then + logit("Invalid number of lanes or speed") + return 0, 0 + end + + -- check serdes_cnt if it is a multiple of speed + local serdes_cnt = math.fmod(speed, count) + + if serdes_cnt ~= 0 then + logit("Invalid speed and number of lanes combination") + return 0, 0 + end + + lane_speed = math.floor(speed / count) + + -- return value in bits + if lane_speed == 1000 then + serdes = 1.25e+9 + elseif lane_speed == 10000 then + serdes = 10.3125e+9 + elseif lane_speed == 25000 then + serdes = 25.78125e+9 + elseif lane_speed == 50000 then + serdes = 53.125e+9 + elseif lane_speed == 100000 then + serdes = 106.25e+9 + else + logit("Invalid serdes speed") + end + + return lane_speed, serdes +end + +-- look up interface lanes count, lanes speed & serdes speed +-- return lane count, lane speed, serdes speed + +local function find_lanes_and_serdes(interface_name) + -- get the port config from config db + local _ + local serdes, lane_speed, count = 0, 0, 0 + + -- Get the port configure + redis.call('SELECT', appl_db) + local lanes = redis.call('HGET', appl_db_port ..':'..interface_name, 'lanes') + + if lanes then + local speed = redis.call('HGET', appl_db_port ..':'..interface_name, 'speed') + + -- we were spliting it on ',' + _, count = string.gsub(lanes, ",", ",") + count = count + 1 + + lane_speed, serdes = calculate_lane_and_serdes_speed(count, speed) + + end + -- switch back to counter db + redis.call('SELECT', counters_db) + + return count, lane_speed, serdes +end + local function compute_rate(port) + local state_table = rates_table_name .. ':' .. port .. ':' .. 'PORT' local initialized = redis.call('HGET', state_table, 'INIT_DONE') logit(initialized) + -- FEC BER + local fec_corr_bits, fec_uncorr_frames + local fec_corr_bits_ber_new, fec_uncorr_bits_ber_new = -1, -1 + -- HLD review suggest to use the statistical average when calculate the post fec ber + local rs_average_frame_ber = 1e-8 + local lanes_speed, serdes_speed, lanes_count = 0, 0, 0 + + -- lookup interface name from oid + local interface_name = find_interface_name_from_oid(port) + if interface_name then + lanes_count, lanes_speed, serdes_speed = find_lanes_and_serdes(interface_name) + + if lanes_count and serdes_speed then + fec_corr_bits = redis.call('HGET', counters_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_IN_FEC_CORRECTED_BITS') + fec_uncorr_frames = redis.call('HGET', counters_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_IN_FEC_NOT_CORRECTABLE_FRAMES') + end + end + + -- Get new COUNTERS values local in_ucast_pkts = redis.call('HGET', counters_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_IN_UCAST_PKTS') local in_non_ucast_pkts = redis.call('HGET', counters_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS') @@ -58,10 +167,11 @@ local function compute_rate(port) local out_octets_last = redis.call('HGET', rates_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_OUT_OCTETS_last') -- Calculate new rates values - local rx_bps_new = (in_octets - in_octets_last) / delta * 1000 - local tx_bps_new = (out_octets - out_octets_last) / delta * 1000 - local rx_pps_new = ((in_ucast_pkts + in_non_ucast_pkts) - (in_ucast_pkts_last + in_non_ucast_pkts_last)) / delta * 1000 - local tx_pps_new = ((out_ucast_pkts + out_non_ucast_pkts) - (out_ucast_pkts_last + out_non_ucast_pkts_last)) / delta * 1000 + local scale_factor = 1000 / delta + local rx_bps_new = (in_octets - in_octets_last) * scale_factor + local tx_bps_new = (out_octets - out_octets_last) * scale_factor + local rx_pps_new = ((in_ucast_pkts + in_non_ucast_pkts) - (in_ucast_pkts_last + in_non_ucast_pkts_last)) * scale_factor + local tx_pps_new = ((out_ucast_pkts + out_non_ucast_pkts) - (out_ucast_pkts_last + out_non_ucast_pkts_last)) * scale_factor if initialized == "DONE" then -- Get old rates values @@ -83,6 +193,21 @@ local function compute_rate(port) redis.call('HSET', rates_table_name .. ':' .. port, 'TX_PPS', tx_pps_new) redis.call('HSET', state_table, 'INIT_DONE', 'DONE') end + + -- only do the calculation when all info present + + if fec_corr_bits and fec_uncorr_frames and lanes_count and serdes_speed then + local fec_corr_bits_last = redis.call('HGET', rates_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_FEC_CORRECTED_BITS_last') + local fec_uncorr_frames_last = redis.call('HGET', rates_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_FEC_NOT_CORRECTABLE_FARMES_last') + + local serdes_rate_total = lanes_count * serdes_speed * delta / 1000 + + fec_corr_bits_ber_new = (fec_corr_bits - fec_corr_bits_last) / serdes_rate_total + fec_uncorr_bits_ber_new = (fec_uncorr_frames - fec_uncorr_frames_last) * rs_average_frame_ber / serdes_rate_total + else + logit("FEC counters or lane info not found on " .. port) + end + else redis.call('HSET', state_table, 'INIT_DONE', 'COUNTERS_LAST') end @@ -94,6 +219,19 @@ local function compute_rate(port) redis.call('HSET', rates_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_OUT_NON_UCAST_PKTS_last', out_non_ucast_pkts) redis.call('HSET', rates_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_IN_OCTETS_last', in_octets) redis.call('HSET', rates_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_OUT_OCTETS_last', out_octets) + + -- do not update FEC related stat if we dont have it + + if not fec_corr_bits or not fec_uncorr_frames or not fec_corr_bits_ber_new or + not fec_uncorr_bits_ber_new then + logit("FEC counters not found on " .. port) + return + end + -- Set BER values + redis.call('HSET', rates_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_FEC_CORRECTED_BITS_last', fec_corr_bits) + redis.call('HSET', rates_table_name .. ':' .. port, 'SAI_PORT_STAT_IF_FEC_NOT_CORRECTABLE_FARMES_last', fec_uncorr_frames) + redis.call('HSET', rates_table_name .. ':' .. port, 'FEC_PRE_BER', fec_corr_bits_ber_new) + redis.call('HSET', rates_table_name .. ':' .. port, 'FEC_POST_BER', fec_uncorr_bits_ber_new) end local n = table.getn(KEYS) From 04255e069d1dd337b945bc7c68a13498d128f611 Mon Sep 17 00:00:00 2001 From: Lawrence Lee Date: Mon, 25 Nov 2024 17:09:10 -0800 Subject: [PATCH 8/8] Skip route group immutable test (#3393) What I did Skip VS test dash/test_dash_route_group.py::test_bound_route_group_immutable Why I did it This test causes orchagent to crash since the VS SAI has not been updated to include implicit deletion of high-volume DASH objects (https://github.com/sonic-net/SONiC/blob/master/doc/dash/dash-sonic-hld.md#18-implicit-deletion-of-sai-objects) --- tests/dash/test_dash_route_group.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/dash/test_dash_route_group.py b/tests/dash/test_dash_route_group.py index dbc8f31eeba..f2051a9598f 100644 --- a/tests/dash/test_dash_route_group.py +++ b/tests/dash/test_dash_route_group.py @@ -15,6 +15,7 @@ APP_DASH_ROUTE_GROUP_TABLE_NAME, ) + @pytest.fixture(autouse=True) def common_setup_teardown(dash_db: DashDB): dash_db.set_app_db_entry(APP_DASH_APPLIANCE_TABLE_NAME, APPLIANCE_ID, APPLIANCE_CONFIG) @@ -37,6 +38,7 @@ def common_setup_teardown(dash_db: DashDB): dash_db.remove_app_db_entry(APP_DASH_ROUTING_TYPE_TABLE_NAME, PRIVATELINK) dash_db.remove_app_db_entry(APP_DASH_APPLIANCE_TABLE_NAME, APPLIANCE_ID) + def test_rebind_eni_route_group(dash_db: DashDB): dash_db.set_app_db_entry(APP_DASH_ROUTE_GROUP_TABLE_NAME, ROUTE_GROUP1, ROUTE_GROUP1_CONFIG) dash_db.set_app_db_entry(APP_DASH_ROUTE_TABLE_NAME, ROUTE_GROUP1, OUTBOUND_ROUTE_PREFIX1, ROUTE_VNET_CONFIG) @@ -58,6 +60,7 @@ def test_rebind_eni_route_group(dash_db: DashDB): dash_db.set_app_db_entry(APP_DASH_ENI_ROUTE_TABLE_NAME, ENI_ID, ENI_ROUTE_GROUP2_CONFIG) dash_db.wait_for_asic_db_field("ASIC_STATE:SAI_OBJECT_TYPE_ENI", eni_key, SAI_ENI_ATTR_OUTBOUND_ROUTING_GROUP_ID, rg2_oid) + def test_duplicate_eni_route_group(dash_db: DashDB): dash_db.set_app_db_entry(APP_DASH_ROUTE_GROUP_TABLE_NAME, ROUTE_GROUP1, ROUTE_GROUP1_CONFIG) dash_db.set_app_db_entry(APP_DASH_ROUTE_TABLE_NAME, ROUTE_GROUP1, OUTBOUND_ROUTE_PREFIX1, ROUTE_VNET_CONFIG) @@ -71,6 +74,8 @@ def test_duplicate_eni_route_group(dash_db: DashDB): dash_db.set_app_db_entry(APP_DASH_ENI_ROUTE_TABLE_NAME, ENI_ID, ENI_ROUTE_GROUP1_CONFIG) dash_db.wait_for_asic_db_field("ASIC_STATE:SAI_OBJECT_TYPE_ENI", eni_key, SAI_ENI_ATTR_OUTBOUND_ROUTING_GROUP_ID, rg1_oid) + +@pytest.mark.skip(reason="Test will crash orchagent until VS SAI is updated with implicit deletion for DASH objects") def test_bound_route_group_immutable(dash_db: DashDB): dash_db.set_app_db_entry(APP_DASH_ROUTE_GROUP_TABLE_NAME, ROUTE_GROUP1, ROUTE_GROUP1_CONFIG) num_route_groups = len(dash_db.wait_for_asic_db_keys("ASIC_STATE:SAI_OBJECT_TYPE_OUTBOUND_ROUTING_GROUP"))